-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
performance: use clip-path css instead of width #1983
Conversation
this prevents browsers from continuously making layout engine passes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea!
Could you replace string concatenation with template strings? And the remaining double quotes with single ones.
Tried in Chrome/Windows and it works correctly. I have not Safari, so I cannot see the performance there. But this improvement seems to have sense, so I approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs a changelog entry, but other than that, LGTM.
ok, all fixed. @katspaugh is this part of the officially required style for the project (template strings over concatenation, single quotes")? could this be in es-lint rather than a round-trip on every PR? |
Thanks, @osheroff! |
this killed off the cursor functionality. we're using the right-hand-border of the progressWave to show a "audio cursor" element.
* use a clip-path css mask instead of changing width this prevents browsers from continuously making layout engine passes. * code review fixes; code style, comments * add changelog entry * add PR number to changelog Co-authored-by: Thijs Triemstra <info@collab.nl>
…)" (katspaugh#1994) this killed off the cursor functionality. we're using the right-hand-border of the progressWave to show a "audio cursor" element.
constantly changing the width of the multi-canvas
progressWave
element triggers a layout pass in the browser. This doesn't seem to be a huge problem for Chrome, but in Safari on mac it's worth quite a hunk of performance.This is Safari pre-patch (I played the whole file on /example/html-init/ for these tests):
Safari post-patch:
Chrome seems to jitter more and benefit less, but note the ~10% of rendering that we seem to save:
pre-patch:
![Screen Shot 2020-06-23 at 9 28 19 PM](https://user-images.githubusercontent.com/260084/85501310-f7308f80-b599-11ea-80d2-11baa51167a1.png)
![Screen Shot 2020-06-23 at 9 29 08 PM](https://user-images.githubusercontent.com/260084/85501313-f7c92600-b599-11ea-95f2-7380f20433c7.png)
post-patch:
![Screen Shot 2020-06-23 at 9 30 35 PM](https://user-images.githubusercontent.com/260084/85501296-ee3fbe00-b599-11ea-93ea-711ffc7c2895.png)
![Screen Shot 2020-06-23 at 9 31 30 PM](https://user-images.githubusercontent.com/260084/85501298-eed85480-b599-11ea-8c37-174d5cf057b2.png)
would love to know if I'm missing anything with the logic here or if other people can run benchmarks and see if this feels like a win.