Closed Bug 1694059 Opened 3 years ago Closed 3 years ago

Use WebRender to render most non-native-theme widgets when possible.

Categories

(Core :: Widget, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
88 Branch
Fission Milestone M7
Tracking Status
firefox88 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

(Whiteboard: fission-nnt)

Attachments

(4 files)

No description provided.

And remove some useless argument names. This is just preparation for the next
patch and shouldn't change behavior.

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

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

(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.

(But sure :))

Keywords: leave-open
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

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!

As per feedback in D105931.

Fission Milestone: --- → M7
Whiteboard: fission-nnt

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).

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c2ae38a2744
Rename various nnt prefs. r=spohl,mstange
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
Flags: needinfo?(emilio)
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
Backout by smolnar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e7dbba6f04b3
Backed out changeset 960cb2cf2009 for causing reftest failures.

Wrong failure/back out link? What was failing?

Flags: needinfo?(emilio)
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
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
Attachment #9204649 - Attachment description: Bug 1694059 - Use WebRender to render non-native range inputs and checkboxes. r=mstange,spohl → Bug 1694059 - Use WebRender to render non-native range inputs and checkboxes. r=mstange

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.

Flags: needinfo?(emilio) → needinfo?(mstange.moz)
Attachment #9204649 - Attachment description: Bug 1694059 - Use WebRender to render non-native range inputs and checkboxes. r=mstange → Bug 1694059 - Use WebRender to render non-native range inputs and radio buttons. r=mstange
Keywords: leave-open

(Markus replied in phab, and I addressed his feedback. Thanks Markus!)

Flags: needinfo?(mstange.moz)
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
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch

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% reddit 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% reddit ContentfulSpeedIndex linux64-shippable-qr cold nocondprof webrender 798.75 -> 744.42
6% reddit SpeedIndex linux64-shippable-qr cold nocondprof webrender 572.75 -> 536.00
5% reddit 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

Regressions: 1695189
Regressions: 1696709
Regressions: 1700206
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: