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

Rotated text rendering has huge performance cost #6427

Open
2 of 17 tasks
quinton-ashley opened this issue Sep 20, 2023 · 22 comments
Open
2 of 17 tasks

Rotated text rendering has huge performance cost #6427

quinton-ashley opened this issue Sep 20, 2023 · 22 comments

Comments

@quinton-ashley
Copy link
Contributor

quinton-ashley commented Sep 20, 2023

Increasing Access

Better performance could allow people to do more with less powerful hardware.

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build Process
  • Unit Testing
  • Internalization
  • Friendly Errors
  • Other (specify if possible)

Feature enhancement details

I'm making this feature request because the text caching I've implemented in q5.js and p5play (with help from @davepagurek) provides a ~90x performance increase compared to p5.js. Because the difference is so significant, I'd like for this feature to be implemented in p5.js as well.

This issue first came to my attention when a user in the p5play discord server complained that adding text in their game caused performance lag.

I was able to narrow down the problem. It's not just an issue of drawing a lot of text to the screen. Every modern web browser's HTML5 canvas seems optimized to handle repeatedly drawing text to the canvas, even if the text is always changing position.
https://editor.p5js.org/quinton-ashley/sketches/aZksvlMxR

But rotated text seems to re-render on every draw call, which requires very high gpu usage. Here's a stress test that even causes fps to drop on my M1 MacBook.
https://editor.p5js.org/quinton-ashley/sketches/AVjHaOeJ_

I also created a more practical example in p5play (old v3.13) that can cause fps to drop on less capable hardware.
https://editor.p5js.org/quinton-ashley/sketches/vu_6SLKdQ

Screenshot 2023-09-15 at 08 41 31

I fixed this issue in q5 (v1.4) and in p5play (v3.14):

https://editor.p5js.org/quinton-ashley/sketches/A-9VoxCG7
https://editor.p5js.org/quinton-ashley/sketches/20SkWm0fM

Screenshot 2023-09-15 at 08 41 38

But if the typical p5.js user encountered this issue, they'd simply think p5 is incapable of rendering so much text. My first thought as a user would not be to download a library specifically to fix this issue.

The least sophisticated way for p5.js users to solve the problem would be to create a Graphics object, draw text on it, then display the graphic instead of the text using image. Not too difficult but I don't think it should be something p5.js users should need to implement themselves.

I'm not interested in implementing this in p5.js, but in case anyone would, take a look at my implementation here:
https://github.com/quinton-ashley/q5.js/blob/1.4.4/q5.js#L1564

In p5play if sprites are labeled using sprite.text it's super common for the labels to get rotated, but perhaps it's not a common enough occurrence for general p5.js sketches to warrant text caching be implemented in p5.js itself.

Please keep this issue open though so if anyone is experiencing this issue with p5.js they can just use p5play.

@dhowe
Copy link
Contributor

dhowe commented Sep 22, 2023

@quinton-ashley
Copy link
Contributor Author

@dhowe ah whoops, just fixed that: https://github.com/quinton-ashley/q5.js/blob/1.4.4/q5.js#L1564

@limzykenneth
Copy link
Member

I'm not sure I fully understand this. From my test on my M1 MacBook Pro I don't see significant performance difference between the two versions, there might be some difference if I measure the FPS but at least visually I'm not sure I can tell for sure (maybe this is more visible on Chome? I've only done most tests on Firefox).

Also I don't see performance difference between just moving text vs rotating text, and profiling I've done on the two showed the two have similar profile with most of the time spent in CanvasRenderingContext2D.fillText. The cached version spent most of the time in CanvasRenderingContext2D.drawImage.

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Sep 25, 2023

EDIT: problem exists in EVERY web browser

Try testing with Chrome, Safari, or Edge.

Chrome has the best overall HTML5 canvas performance, so it's the best web browser for using p5.js.

Firefox is pretty much irrelevant considering it only has 2% of global web browser users.

@davepagurek
Copy link
Contributor

For what it's worth I see the effects on only Chrome on my Intel Macbook and not a newer M1 macbook. One of the reasons why I think something like this best exists as a library is because it's an optimization that may or may not be relevant depending on your hardware setup (and the drivers and implementation may change over time too.) We saw something similar trying to optimize WebGL line rendering where something that was a huge speedup on one platform was slightly slower on another--trying to optimize something across the board can be a big time sink.

@limzykenneth
Copy link
Member

If it only exist on Chrome, I would say that's a Chrome problem and not for us to solve. I also don't recommend the practice of ignoring browsers just because some statistic put them lower than Chromium based browser too, but that's a different discussion.

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Sep 25, 2023

The problem affects every browser I tested (Chrome, Safari, Edge on Windows 11). I will test Firefox too to confirm that the problem exists in Firefox too.

Maybe I added too many links in my original post.

This example shows the problem: https://editor.p5js.org/quinton-ashley/sketches/AVjHaOeJ_

This example shows the solution results: https://editor.p5js.org/quinton-ashley/sketches/A-9VoxCG7

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Sep 25, 2023

Use browser performance testing tools to see the results. You might not be able to see the slight fps drop visually since M1 macs are so powerful, but on an older/cheaper computer the visual difference is super obvious, p5.js has a major fps drop.

@davepagurek
Copy link
Contributor

I don't see it so much as a Chrome problem but rather that Chrome uniquely adds an optimization for a specific case. Adding image caching of text would be something similar: optimizing for the case where you are drawing a lot of text, where the text doesn't change other properties over time, and where the memory cost of caching isn't a concern.

I think that's a worthwhile tradeoff for many cases, but also adds maintenance cost that we have to weigh when considering making something a core feature. Core features should come with a certain level of across-the-board support for other p5 features, which I think would be hard to support for this particular case. A library, meanwhile, is able to be clear about the tradeoffs and let users opt-in, which is why I think that might be a better fit here.

In an ideal world, we could support these sorts of workarounds everywhere, but with a limited core team, to me it seems like better value to put our time into enabling things like this to exist as libraries. Does that seem right to you @limzykenneth?

@quinton-ashley
Copy link
Contributor Author

@limzykenneth The reality is Chrome, Safari, Edge, and Opera combined have ~92% of worldwide desktop users. Chrome on it's own has a near super majority, at ~64%.

Any problem that affects both Chrome and Safari users ought to be considered a problem for ALL users.

Also I just tested it and the problem does exist in Firefox too. Firefox unsurprisingly performed the worst out of every browser I tested.

@davepagurek
Copy link
Contributor

Any problem that affects both Chrome and Safari users ought to be considered a problem for ALL users.

I agree, although if it's a rendering bug that will likely get fixed in the next version (e.g. last year when Chrome broke the MULTIPLY blend mode for everyone) then that would be a scenario where we just wait it out rather than try to make a workaround. I think that's more what was being suggested rather than just ignoring Chrome. (I don't think that's what's going on in this case, but I don't think anyone's arguing to just ignore Chrome in general.)

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Sep 25, 2023

@limzykenneth Chrome and Safari just drop a few frames at peak load in the problem example on my M1. But Firefox churns out frames at ~150ms avg frame rendering time which is only ~7fps.

Screenshot 2023-09-25 at 09 47 17

@quinton-ashley
Copy link
Contributor Author

@davepagurek It's not a "rendering bug in Chrome" and why count on a future fix that might not be implemented for months or ever when the problem could be solved now?

@davepagurek
Copy link
Contributor

Right, I agree it's not a rendering bug, but instead a question of whether the optimization for this case is common enough to be worth the maintenance cost. The reason not to would be that we're also in parallel trying to think of ways to consolidate our text rendering code (#6391) to make it easier to maintain, easier to add features to like variable fonts, and easier to work consistently across feature sets. Another form of rendering text helps speed but increases dev time on those other axes, so that's what we'd be weighing off here.

@limzykenneth
Copy link
Member

The position I have is I don't recommend optimising for specific browser. Unless the standard says otherwise, following it should give the best performance and result, it is up to browser vendors to follow the standard and optimise for it. Putting in browser specific optimisation just increase maintenance cost and make our own optimisation brittle, not to mention creating self-perpetuating excuse for browser vendors to not address potential issue.

The days of optimising for IE and Safari already gave enough headache, a popularity of a browser is not something that I'd use to consider going back down that path really.

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Sep 25, 2023

@limzykenneth What are you talking about? I took the time to provide you evidence that the problem is NOT browser based and you're just talking past it.

@quinton-ashley
Copy link
Contributor Author

@davepagurek Yes and I agree the text rendering implementation in p5.js is unnecessarily complicated.

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Sep 25, 2023

If the problem could be fixed in every web browser's HTML5 canvas implementation that would be great but it's wishful thinking at this point. Has any such requests been made to web browser devs yet? Not that I know of.

@limzykenneth
Copy link
Member

@quinton-ashley If all browsers have this issue then yes it is potentially something we can optimise for, my comment is mainly in response to the quote of Chrome having ~64% market while Firefox being irrelevant because of only having 2%, which for me is not a point of consideration for whether we address anything or not.

We are all trying to help and I'm just stating my position re browser usage stats being a reason for a solution to be implemented. From my test I'm not seeing significant difference between the sketches in any browser, and as you noted, it might be because it is not very visible on an M1 machine, so I cannot comment on how it performs on different setup at the moment as I'm not very available. I'm sorry if it come across like I'm ignoring your point that it applies to Firefox as well (I can't fully verify at this point), I'm only concerned that perculiar issue with Chrome be optimised for simply because they are popular which I don't think is appropriate.

Also if this is a problem with existing browser's implementation, it is very likely we will see this issue already brought up in their bug tracker (in this form or some other form that may be harder to find), again I've not have the availability to do this search but it would be good to know whether this is a known problem already or not (someone else may have already come across this somewhere else?).

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Sep 25, 2023

@limzykenneth I specifically didn't mention Firefox when making this performance based issue report because I assumed everyone working on p5.js would already know how bad Firefox' HTML5 canvas performance is. Can we agree that while Firefox users might find such terrible performance acceptable, the vast majority of internet users will not? That's why talking about Firefox in this context IS irrelevant, even IF this specific problem did not affect Firefox. Again the irony is that the problem affects Firefox way more than every other browser, not less.

If the difference between <8fps and 60fps is not apparent to you, then I don't even know what to say. Seems like making this issue report was pointless.

@limzykenneth
Copy link
Member

I'm not sure where you get the impression that canvas performance on Firefox is bad and I'm afraid that's not something I can agree on. And I have to say:

Can we agree that while Firefox users might find such terrible performance acceptable, the vast majority of internet users will not?

I find that comment not an acceptable generalisation and not appriopriate nor constructive behaviour. I don't think I would be able to continue in this conversation if that is the position you take which I find borderline not inclusive. I will unsubscribe from this thread.

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Sep 25, 2023

I get "the impression" Firefox' canvas performance is worse because it's an indisputable fact that you can empirically test yourself. Don't worry about unsubscribing, I'm not continuing this ridiculous conversation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants