p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

Permit zero-temporary-value buffer binding

Open rbtcollins opened this issue 3 years ago • 3 comments
trafficstars

When using custom 3D renderers, p5 can support much larger models, but this particular bit of code showed up as a hotspot: I was passing in a Float32 already, and it was getting cloned.

Without this change, my particular workload runs at 7-12 fps. With it 45-50fps.

In principle p5 can make use of this itself, though having webGL ready representations as the source of truth is a much larger change - but even so, taking steps in that direction seems broadly useful.

Resolves #[Add issue number here]

Changes:

Screenshots of the change:

PR Checklist

Fixes: https://github.com/processing/p5.js/issues/5739

rbtcollins avatar Jul 28 '22 21:07 rbtcollins

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

welcome[bot] avatar Jul 28 '22 21:07 welcome[bot]

Thank you @rbtcollins ! As a reminder, an issue needs to be opened before a pull request is opened, then you can tag the issue in the pull request. You can read more about it in the GitHub Issue Flow. This is necessary for tracking development and keeping discussion clear. I will close this PR for now for organizational purpose, and please feel free to open a new PR after an issue is opened. Thanks!

Qianqianye avatar Jul 29 '22 02:07 Qianqianye

@Qianqianye hoop jumped through, you should be able to re-open this PR now.

rbtcollins avatar Jul 29 '22 08:07 rbtcollins

I'm going to close this PR for now because in the time since, we've started adding some tools to p5 to achieve something similar via p5.DataArray (and I think the else branch below also would let you do the workaround you mentioned where you manually replaced an array with a Float32Array): https://github.com/processing/p5.js/blob/c06e12aa921a1a44ddb411584c1718b61cc6408b/src/webgl/p5.RendererGL.js#L1833-L1837

I'm going to leave your issue open because we currently only internally use p5.DataArray for line data, which gets automatically created, so there's still some discussion to be had about the best way to do the same for developer-facing arrays like the vertices of a p5.Geometry. We'd love your feedback that if you have any ideas or opinions!

davepagurek avatar Aug 20 '23 12:08 davepagurek