Use WebRender to render most non-native-theme widgets when possible.
Categories
(Core :: Widget, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox88 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
(Whiteboard: fission-nnt)
Attachments
(4 files)
Assignee | ||
Comment 1•3 years ago
|
||
And remove some useless argument names. This is just preparation for the next
patch and shouldn't change behavior.
Assignee | ||
Comment 2•3 years ago
|
||
We basically use a couple primitives to draw these
(PaintRoundedRectWithRadius, FillRect), so making the code a bit generic
implementing stuff with WebRender seems straight-forward.
I've kept using the fallback codepath for the bits that draw complex
paths like arrows and such, but the rest of the things should work with
this patch.
A thing I'm not too happy about is the scrollbar painting setup (requires a lot
of boilerplate), but modulo template hacks make nsNativeBasicTheme a template
that receives its super class as a parameter or something) it seems hard to do
better.
Depends on D105930
Comment 3•3 years ago
|
||
I'm not sure I'll get to the reviews tonight, but even if Markus does, please hold off on landing this until we enabled the non-native theme on macOS in bug 1690842. My plan is to land bug 1690842 and all dependent patches tomorrow when we switch to 88 in Nightly. Thank you
Assignee | ||
Comment 4•3 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #3)
I'm not sure I'll get to the reviews tonight, but even if Markus does, please hold off on landing this until we enabled the non-native theme on macOS in bug 1690842. My plan is to land bug 1690842 and all dependent patches tomorrow when we switch to 88 in Nightly. Thank you
We could land this with the pref off if you prefer.
Assignee | ||
Comment 5•3 years ago
|
||
(But sure :))
Assignee | ||
Updated•3 years ago
|
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/14c6128706a5 Pass draw target by reference in non-native widget code. r=mstange
Comment 7•3 years ago
|
||
Bug 1690842 just landed and appears to stick, so please feel free to land once reviewed. I was mostly concerned that this might introduce new test failures that would prevent bug 1690842 from landing, but I don't think we need to pref this off if the tree is green. I will also defer to Markus on the review here since he has already taken a first pass. Thanks again!
Assignee | ||
Comment 8•3 years ago
|
||
As per feedback in D105931.
Updated•3 years ago
|
Assignee | ||
Comment 9•3 years ago
|
||
The only thing missing now are things that draw arrows / checkmarks.
Make the disabled range thumb opaque, to avoid dealing with clipping
(also matches all other browsers, fwiw).
Comment 10•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9c2ae38a2744 Rename various nnt prefs. r=spohl,mstange
Comment 11•3 years ago
|
||
bugherder |
Comment 12•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4436a8bb02fd Use WebRender to render most non-native-theme widgets when possible. r=mstange
Comment 13•3 years ago
|
||
Backed out for causing reftest failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/a744d499061c33da50d707ebe3b35835a333c3ff
Assignee | ||
Updated•3 years ago
|
Comment 14•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/960cb2cf2009 Use WebRender to render most non-native-theme widgets when possible. r=mstange
Comment 15•3 years ago
|
||
bugherder |
Comment 16•3 years ago
|
||
Backed out for causing reftest failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/916497e295fe6c3b5e8082045add889fd555dff6
Comment 17•3 years ago
|
||
Backout by smolnar@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e7dbba6f04b3 Backed out changeset 960cb2cf2009 for causing reftest failures.
Assignee | ||
Comment 18•3 years ago
|
||
Wrong failure/back out link? What was failing?
Comment 19•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/287db0596e0b Use WebRender to render most non-native-theme widgets when possible. r=mstange
Comment 20•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3bf5d4016ea7 Use WebRender to render non-native range inputs and checkboxes. r=mstange
Comment 21•3 years ago
|
||
Backout link is https://hg.mozilla.org/integration/autoland/rev/e7dbba6f04b37effda7473abbb0589e4c62490a1
And looks like the failures were: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=success%2Cpending%2Crunning%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=reftest&revision=960cb2cf200922ad714350fbeac546aadb25e5c6&group_state=expanded&selectedTaskRun=WE5dfRPpSFeKLlfZ0xVKGQ.0
Failure log: https://treeherder.mozilla.org/logviewer?job_id=330851912&repo=autoland
Comment 22•3 years ago
|
||
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/autoland/rev/2a637141548e Green up swgl tests.
Comment 23•3 years ago
|
||
Backed out for input range reftest failures.
Failure log: https://treeherder.mozilla.org/logviewer?job_id=330893920&repo=autoland
https://treeherder.mozilla.org/logviewer?job_id=330894183&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/0595464ceb8f5a901c044e19c4b373ba3a7a5501
Updated•3 years ago
|
Assignee | ||
Comment 24•3 years ago
|
||
So I don't know why yesterday's try run was green, but I can repro those failures with webrender disabled.
Markus, I think the overflow rect for the range is not accounting for the shadow offset:
OverflowRect: (x=4, y=4, w=172, h=32)
ThumbRect: (x=115, y=10, w=20, h=20), Inflation: (6 x 6), ShadowRect: (x=109, y=6, w=32, h=32)
(the thumb shadow rect is clearly going outside the overflow rect)
Fine to just do something like this? The cliprect changes are not really needed but no reason we need to clip to a bigger clip than necessary. I think that should also properly fix bug 1693346.
Comment 25•3 years ago
|
||
bugherder |
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 26•3 years ago
|
||
(Markus replied in phab, and I addressed his feedback. Thanks Markus!)
Comment 27•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2d0e67c944f2 Use WebRender to render non-native range inputs and radio buttons. r=mstange
Comment 28•3 years ago
|
||
bugherder |
Comment 29•3 years ago
•
|
||
Thanks to this, we gained following performance improvements! 🎉
== Change summary for alert #28911 (as of Wed, 24 Feb 2021 18:11:08 GMT) ==
Improvements:
Ratio | Suite | Test | Platform | Options | Absolute values (old vs new) |
---|---|---|---|---|---|
13% | FirstVisualChange | linux64-shippable-qr | cold nocondprof webrender | 284.14 -> 246.67 | |
8% | fandom | FirstVisualChange | linux64-shippable-qr | cold nocondprof webrender | 566.67 -> 523.33 |
7% | yahoo-mail | ContentfulSpeedIndex | linux64-shippable-qr | cold nocondprof webrender | 472.33 -> 437.33 |
7% | ContentfulSpeedIndex | linux64-shippable-qr | cold nocondprof webrender | 798.75 -> 744.42 | |
6% | SpeedIndex | linux64-shippable-qr | cold nocondprof webrender | 572.75 -> 536.00 | |
5% | PerceptualSpeedIndex | linux64-shippable-qr | cold nocondprof webrender | 685.67 -> 651.58 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=28911
Description
•