Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of line rendering #6230

Merged
merged 6 commits into from Aug 17, 2023

Conversation

davepagurek
Copy link
Contributor

Addresses #5975

While the architectural changes in #6162 might be a bit more difficult to weigh as they are not always better across the board for all browsers, there are some changes that can be pulled out to improve the performance of the existing line rendering architecture without actually changing the way lines are represented.

Changes

  • No longer checks the state of backface culling; instead, it makes the vertex ordering consistent so that we don't need to disable culling when drawing lines
  • Removes the need for flattening line data before rendering; instead, it is put into the arrays in a pre-flattened state
  • Stores line buffer data in a Float32Array from the beginning so that it can be reused without constantly reallocating new arrays
  • Changes registerEnabled to a Set (not really a perf update, but makes it easier to loop over enabled registers in the future)

Performance

Test sketch: https://editor.p5js.org/davepagurek/sketches/7jnWpXBq_

Many lines

Browser1.6.0This PR
Chrome60fps60fps
Firefox15fps60fps
Safari5fps5fps
Mobile Chrome6fps8fps

Large curve

Browser1.6.0This PR
Chrome24fps28fps
Firefox11fps20fps
Safari20fps27fps
Mobile Chrome9fps14fps

Retained shapes

Browser1.6.0This PR
Chrome60fps60fps
Firefox60fps60fps
Safari60fps60fps
Mobile Chrome60fps60fps

PR Checklist

  • npm run lint passes
  • [Inline documentation] is included / updated
  • [Unit tests] are included / updated

* Like a C++ vector, its fixed-length Float32Array backing its contents will
* double in size when it goes over its capacity.
*/
p5.DataVector = class DataVector {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you're referencing c++ with the vector name, but I feel like within the context of p5 it means something different. Is there a clearer name for this class? DataArray?

*
* Like a C++ vector, its fixed-length Float32Array backing its contents will
* double in size when it goes over its capacity.
*/
Copy link
Contributor

@aferriss aferriss Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this class have use outside of p5 rendering internals? Worth exposing more broadly or at least making a inline doc example for future developers here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably only useful for things that need a Flaot32Array, which for now is just rendering. An example for devs can't hurt!

@@ -195,12 +195,12 @@ p5.RendererGL.prototype.endShape = function(
this._processVertices(...arguments);
this.isProcessingVertices = false;
if (this._doFill) {
if (this.immediateMode.geometry.vertices.length > 1) {
if (this.immediateMode.geometry.vertices.length >= 3) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guessing this is because anything less than a triangle can't have a fill right? Just want to make sure there isn't some weird reason why this was originally set to 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! I noticed that in that example where we're drawing a bunch of lines, it was still trying to bind fill buffers. Lines don't have fills, and points are drawn using a separate _drawPoints() method, so I think this should be safe.

Copy link
Contributor

@aferriss aferriss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! Thanks so much for taking another look at this. I feel much better about these changes knowing it's wins across the board.

Was backface culling actually making a difference? When trying your test sketch I couldn't notice a performance change while turning it on and off.

I left some small comments throughout, but overall I think this is great!

@davepagurek
Copy link
Contributor Author

Backface culling isn't a performance gain on its own, but since some users turn it on occasionally, we used to have some code to temporarily turn it off and then restore its state when drawing lines. It was that saving/restoring bit that was slow, which is gone now 🙂 The culling toggle in the sketch is to help check that lines don't visually break when culling is turned on.

@davepagurek davepagurek merged commit 8baa00e into processing:main Aug 17, 2023
2 checks passed
@davepagurek davepagurek deleted the feat/faster-lines branch August 17, 2023 13:25
AmrikSD pushed a commit to Potato-Blood/pogo that referenced this pull request Nov 10, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [p5](https://togithub.com/processing/p5.js) | devDependencies | minor
| [`1.7.0` -> `1.8.0`](https://renovatebot.com/diffs/npm/p5/1.7.0/1.8.0)
|
|
[@types/p5](https://togithub.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/p5)
([source](https://togithub.com/DefinitelyTyped/DefinitelyTyped)) |
devDependencies | patch | [`1.7.0` ->
`1.7.3`](https://renovatebot.com/diffs/npm/@types%2fp5/1.7.0/1.7.3) |

---

### Release Notes

<details>
<summary>processing/p5.js (p5)</summary>

###
[`v1.8.0`](https://togithub.com/processing/p5.js/releases/tag/v1.8.0)

[Compare
Source](https://togithub.com/processing/p5.js/compare/v1.7.0...v1.8.0)

<!-- Release notes generated using configuration in .github/release.yml
at v1.8.0 -->

#### What's Changed 🎊

##### WebGL

In this release, p5.js added some new WebGL mode tools. Filters now run
in shaders for extra speed, and you can now run custom filter shaders,
even on 2D canvases. You can now cut holes in shapes with
`beginContour()` and apply vector masks with `beginClip()`. You can
reuse shapes more efficiently with `buildGeometry()` and instanced
rendering. Finally, we have also fixed a number of bugs. *- Summary
written by [@&#8203;davepagurek](https://togithub.com/davepagurek) ✨*

- Add support for beginContour() and endContour() in Webgl mode by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6297
- Fix stroke rendering when drawing to framebuffers by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6304
- Adds createFilterShader() and custom shader support to the webGL
filter() function by
[@&#8203;wong-justin](https://togithub.com/wong-justin) in
[processing/p5.js#6237
- Fix WebGL text not rendering when rotated 90 degrees by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6316
- Fix reading between nested active framebuffers by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6314
- Add methods to construct p5.Geometry from other p5 drawing functions
by [@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6287
- Handle missing exact edge vertices in buildGeometry by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6320
- Fix strokes on framebuffers with different aspect ratios by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6339
- Fix freed geometry leaving attributes in a broken state by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6323
- Improve performance of line rendering by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6230
- Add support for webGL instancing by
[@&#8203;RandomGamingDev](https://togithub.com/RandomGamingDev) in
[processing/p5.js#6276
- Add shaders for filter() constants, and use them by default in P2D by
[@&#8203;wong-justin](https://togithub.com/wong-justin) in
[processing/p5.js#6324
- Fix clip() on both the main canvas and framebuffers by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6376
- fixed texture filtering bug in p5.Framebuffer by
[@&#8203;KeyboardSounds](https://togithub.com/KeyboardSounds) in
[processing/p5.js#6420
- Fix clear() on framebuffers on Intel macs by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6429
- Fix textureMode(IMAGE) + beginShape(TESS) by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6366
- fixed issue
[#&#8203;6440](https://togithub.com/processing/p5.js/issues/6440) by
[@&#8203;Gaurav-1306](https://togithub.com/Gaurav-1306) in
[processing/p5.js#6446
- Erode, dilate, threshold shader filters match closer to CPU filters by
[@&#8203;wong-justin](https://togithub.com/wong-justin) in
[processing/p5.js#6405
- Update WebGL blur filter to match CPU blur more by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6460
- Fix camera flipping on framebuffers between push/pop calls by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6471
- Setuniform by [@&#8203;Gaurav-1306](https://togithub.com/Gaurav-1306)
in
[processing/p5.js#6474
- resolved issue
[#&#8203;6399](https://togithub.com/processing/p5.js/issues/6399) by
[@&#8203;Gaurav-1306](https://togithub.com/Gaurav-1306) in
[processing/p5.js#6480
- Auto-bind filter shaders to the filter graphic by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6482
- new PR for issue
[#&#8203;6383](https://togithub.com/processing/p5.js/issues/6383)(Problem
for diagonal) by
[@&#8203;perminder-17](https://togithub.com/perminder-17) in
[processing/p5.js#6488

##### Friendly Error System (FES)

- Add Hindi translation to FES by
[@&#8203;Ayush23Dash](https://togithub.com/Ayush23Dash) in
[processing/p5.js#6272
- Re-worded lines 413 and 446 of FES Developer Notes by
[@&#8203;OnexiMedina](https://togithub.com/OnexiMedina) in
[processing/p5.js#6307
- Reference FES Contributor Docs inside FES Directory along with a
diagram to understand usages of FES functions by
[@&#8203;Ayush23Dash](https://togithub.com/Ayush23Dash) in
[processing/p5.js#6335
- Fixed typing errors in fes_core.js documentation by
[@&#8203;Garima3110](https://togithub.com/Garima3110) in
[processing/p5.js#6478
- Update friendly_error_system.md by
[@&#8203;Garima3110](https://togithub.com/Garima3110) in
[processing/p5.js#6481
- Update fes_reference_dev_notes.md by
[@&#8203;Garima3110](https://togithub.com/Garima3110) in
[processing/p5.js#6486

##### Reference Documentation Update

We updated a group of p5.js Reference pages as part of 2023 Season of
Docs (SoD) program, with a goal to make them more accessible and
beginner-friendly. Thanks to the SoD technical writer
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) ✨.

- Edit docs for math functions by
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) in
[processing/p5.js#6281
- docs(typography): fix typos in example for textFont by
[@&#8203;meezwhite](https://togithub.com/meezwhite) in
[processing/p5.js#6401
- Edit docs for p5.Vector by
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) in
[processing/p5.js#6340
- Edit docs for pixels functions by
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) in
[processing/p5.js#6390
- Edit docs for loading & displaying images by
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) in
[processing/p5.js#6425
- Update docs for p5.Image by
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) in
[processing/p5.js#6434
- Edit docs for p5.Font by
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) in
[processing/p5.js#6453
- Edit docs for image by
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) in
[processing/p5.js#6424
- Edit docs for typography load and display by
[@&#8203;nickmcintyre](https://togithub.com/nickmcintyre) in
[processing/p5.js#6450

##### Google Summer of Code (GSoC) 2023 Wrap up

- 🌸 Added GSoC wrap up! by
[@&#8203;dewanshDT](https://togithub.com/dewanshDT) in
[processing/p5.js#6403
- Gsoc 23 Wrapup post by
[@&#8203;Ayush23Dash](https://togithub.com/Ayush23Dash) in
[processing/p5.js#6415
- add GSoC'23 wrapup post for Justin Wong by
[@&#8203;wong-justin](https://togithub.com/wong-justin) in
[processing/p5.js#6418
- Create lichlyter_gsoc\_2023.md by
[@&#8203;katlich112358](https://togithub.com/katlich112358) in
[processing/p5.js#6455
- Create munusshih_gsoc\_2023.md by
[@&#8203;munusshih](https://togithub.com/munusshih) in
[processing/p5.js#6461

##### Other Code Update

- Ask to disable printing when print() called with no arguments by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6253
- fix textWidth() and textToPoints() by
[@&#8203;munusshih](https://togithub.com/munusshih) in
[processing/p5.js#6184
- Fix issue where nf with 0 'right' parameter returns undefined in
string by [@&#8203;limzykenneth](https://togithub.com/limzykenneth) in
[processing/p5.js#6291
- Update environment.js with fix for frameRate description by
[@&#8203;quinton-ashley](https://togithub.com/quinton-ashley) in
[processing/p5.js#6269
- Implement clip() to shapes by
[@&#8203;davepagurek](https://togithub.com/davepagurek) in
[processing/p5.js#6306
- Clarified workflow for contributing documentation by
[@&#8203;thatguyseven](https://togithub.com/thatguyseven) in
[processing/p5.js#6312
- Clears MediaElement canvas at the beginning of every frame by
[@&#8203;donaldzhu](https://togithub.com/donaldzhu) in
[processing/p5.js#6309
- Clean up gruntfile release related steps by
[@&#8203;Qianqianye](https://togithub.com/Qianqianye) in
[processing/p5.js#6321
- fix-return-type by
[@&#8203;asukaminato0721](https://togithub.com/asukaminato0721) in
[processing/p5.js#6326
- fix HALF_FLOAT by
[@&#8203;asukaminato0721](https://togithub.com/asukaminato0721) in
[processing/p5.js#6330
- Added .gitattributes to Increase compatability with Window users and
line endings by [@&#8203;SilasVM](https://togithub.com/SilasVM) in
[processing/p5.js#6317
- update all contributors setup by
[@&#8203;gr2m](https://togithub.com/gr2m) in
[processing/p5.js#6341
- refine canvas' type by
[@&#8203;asukaminato0721](https://togithub.com/asukaminato0721) in
[processing/p5.js#6328
- MouseEvent, WheelEvent and KeyboardEvent type by
[@&#8203;asukaminato0721](https://togithub.com/asukaminato0721) in
[processing/p5.js#6329
- fixed-wrong-capture-size-and-freeze-issue by
[@&#8203;Prateek93a](https://togithub.com/Prateek93a) in
[processing/p5.js#5159
- add more event type by
[@&#8203;asukaminato0721](https://togithub.com/asukaminato0721) in
[processing/p5.js#6379
- Main by [@&#8203;j-adel](https://togithub.com/j-adel) in
[processing/p5.js#6374
- Update labeler Github Action by
[@&#8203;stampyzfanz](https://togithub.com/stampyzfanz) in
[processing/p5.js#6395
- add unregisterMethod function by
[@&#8203;capGoblin](https://togithub.com/capGoblin) in
[processing/p5.js#6426
- add before/after preload and setup by
[@&#8203;capGoblin](https://togithub.com/capGoblin) in
[processing/p5.js#6433
- Fix: Misleading error message when NaN passed by
[@&#8203;capGoblin](https://togithub.com/capGoblin) in
[processing/p5.js#6464
- Support pixel density on p5.Image (fixes issue
[#&#8203;6114](https://togithub.com/processing/p5.js/issues/6114)) by
[@&#8203;Gaurav-1306](https://togithub.com/Gaurav-1306) in
[processing/p5.js#6447
- Fix orphan canvas when sketch is removed before canvas creation by
[@&#8203;limzykenneth](https://togithub.com/limzykenneth) in
[processing/p5.js#6355

##### Other Documentation Update

- Fixed GitHub capitalization typo in contributor_docs by
[@&#8203;SilasVM](https://togithub.com/SilasVM) in
[processing/p5.js#6284
- Fixing typo in "What are issues?" by
[@&#8203;snwarner22](https://togithub.com/snwarner22) in
[processing/p5.js#6288
- Fixed GitHub spelling in CONTRIBUTING.md by
[@&#8203;SilasVM](https://togithub.com/SilasVM) in
[processing/p5.js#6295
- Fixed grammatical errors in contributor_guidelines.md by
[@&#8203;thatguyseven](https://togithub.com/thatguyseven) in
[processing/p5.js#6296
- Update documentation_style_guide.md with new guideline by
[@&#8203;zelf0](https://togithub.com/zelf0) in
[processing/p5.js#6334
- add missing code contributors to all contributors in README and
`.all-contributors.rc` file by [@&#8203;gr2m](https://togithub.com/gr2m)
in
[processing/p5.js#6349
- docs(all-contributors): remove
[@&#8203;stellartux](https://togithub.com/stellartux) as requested by
[@&#8203;gr2m](https://togithub.com/gr2m) in
[processing/p5.js#6368
- docs(src/utilities): Use `describe()` instead of `@alt` by
[@&#8203;Zearin](https://togithub.com/Zearin) in
[processing/p5.js#5598
- Fix typo in export path to fix dev mode by
[@&#8203;mykongee](https://togithub.com/mykongee) in
[processing/p5.js#6373
- Improve Readme for future Contributors to codebase by
[@&#8203;Ayush23Dash](https://togithub.com/Ayush23Dash) in
[processing/p5.js#6260
- Fixed mousePressed() Example Error by
[@&#8203;Utkarsh3128](https://togithub.com/Utkarsh3128) in
[processing/p5.js#6413
- Update README.md by
[@&#8203;katlich112358](https://togithub.com/katlich112358) in
[processing/p5.js#6458
- Fixed typing errors in validate_params.js file's documentation by
[@&#8203;Garima3110](https://togithub.com/Garima3110) in
[processing/p5.js#6473
- typo and unused variable from core by
[@&#8203;benschac](https://togithub.com/benschac) in
[processing/p5.js#6476

#### New Contributors 💗

- [@&#8203;munusshih](https://togithub.com/munusshih) made their first
contribution in
[processing/p5.js#6184
- [@&#8203;SilasVM](https://togithub.com/SilasVM) made their first
contribution in
[processing/p5.js#6284
- [@&#8203;snwarner22](https://togithub.com/snwarner22) made their first
contribution in
[processing/p5.js#6288
- [@&#8203;thatguyseven](https://togithub.com/thatguyseven) made their
first contribution in
[processing/p5.js#6296
- [@&#8203;OnexiMedina](https://togithub.com/OnexiMedina) made their
first contribution in
[processing/p5.js#6307
- [@&#8203;donaldzhu](https://togithub.com/donaldzhu) made their first
contribution in
[processing/p5.js#6309
- [@&#8203;gr2m](https://togithub.com/gr2m) made their first
contribution in
[processing/p5.js#6341
- [@&#8203;RandomGamingDev](https://togithub.com/RandomGamingDev) made
their first contribution in
[processing/p5.js#6276
- [@&#8203;mykongee](https://togithub.com/mykongee) made their first
contribution in
[processing/p5.js#6373
- [@&#8203;j-adel](https://togithub.com/j-adel) made their first
contribution in
[processing/p5.js#6374
- [@&#8203;meezwhite](https://togithub.com/meezwhite) made their first
contribution in
[processing/p5.js#6401
- [@&#8203;dewanshDT](https://togithub.com/dewanshDT) made their first
contribution in
[processing/p5.js#6403
- [@&#8203;Utkarsh3128](https://togithub.com/Utkarsh3128) made their
first contribution in
[processing/p5.js#6413
- [@&#8203;KeyboardSounds](https://togithub.com/KeyboardSounds) made
their first contribution in
[processing/p5.js#6420
- [@&#8203;capGoblin](https://togithub.com/capGoblin) made their first
contribution in
[processing/p5.js#6426
- [@&#8203;Gaurav-1306](https://togithub.com/Gaurav-1306) made their
first contribution in
[processing/p5.js#6446
- [@&#8203;katlich112358](https://togithub.com/katlich112358) made their
first contribution in
[processing/p5.js#6455
- [@&#8203;Garima3110](https://togithub.com/Garima3110) made their first
contribution in
[processing/p5.js#6473
- [@&#8203;benschac](https://togithub.com/benschac) made their first
contribution in
[processing/p5.js#6476
- [@&#8203;perminder-17](https://togithub.com/perminder-17) made their
first contribution in
[processing/p5.js#6488
- [@&#8203;lakshay451](https://togithub.com/lakshay451) made their first
contribution in
[processing/p5.js#6493

**Full Changelog**:
processing/p5.js@v1.7.0...v1.8.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMS4wIiwidXBkYXRlZEluVmVyIjoiMzcuMzUuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: Renovate Bot <bot@renovateapp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants