Closed Bug 969458 Opened 6 years ago Closed 6 years ago

Menu Panel Grid Items Have Disabled Subpixel Anti-aliasing

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 --- verified
firefox30 --- verified

People

(Reporter: shorlander, Assigned: dao)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P2])

Attachments

(2 files)

The labels of the buttons in the Menu Panel no longer have subpixel anti-aliasing after the landing of bug 897496.

Happens on all platforms, but is particularly noticeable on OS X on standard resolution displays.
Whiteboard: [Australis:M?] → [Australis:P2]
So... the alternative here is to fix the background colors we use to not be transparent (in any of the normal, hover or active states). We can do that on OS X and I think also on Windows, where I believe the background colors for the palette are known and not dependent on OS theme. Mike, is that right? Is it true on Linux, too?
Flags: needinfo?(mdeboer)
Well, yes for platforms other than OSX. Sub-pixel rendering is not disabled when opacity is applied on the background on OSX.

I'm more interested in why the regression could be caused by bug 897496!
Blocks: 897496
Flags: needinfo?(mdeboer)
(In reply to Mike de Boer [:mikedeboer] from comment #3)
> Well, yes for platforms other than OSX. Sub-pixel rendering is not disabled
> when opacity is applied on the background on OSX.
> 
> I'm more interested in why the regression could be caused by bug 897496!

So the regression is caused by the combination of the fading SVG mask and a semi-transparent background color.

Unless we want to get rid of the mask again, the only fix I know of is making the background color not be transparent. That would include the hover/active states, because otherwise you'd see a noticeable transition. On top of that, if we used a fixed background color for the non-hover state, and the current transparent ones for the non-hover state, I suspect that'd make the transition from one to the other look wonky. Assuming the background color of the panel is known, we can compute/measure the correct colors given the hsla() background colors we use now, and fix them.

After discussing with Mike, unfortunately it seems we use -moz-field as the background color on Windows, and -moz-dialog on Linux. Those colors depend on the system theme, so we can't fix this there.

An additional complication is customize mode, because when dragging the nodes obviously you wouldn't want the non-transparent background color to still show up.

Put together, that looks to be quite a lot of work to just fix the text rendering on OS X non-retina... :-\

Markus, is there some way we could force subpixel AA on despite the non-opaque background + SVG mask, or can it just not work correctly in these circumstances or something?
Flags: needinfo?(mstange)
(In reply to :Gijs Kruitbosch from comment #4)
> After discussing with Mike, unfortunately it seems we use -moz-field as the
> background color on Windows, and -moz-dialog on Linux. Those colors depend
> on the system theme, so we can't fix this there.

Well, you can put multiple gradients on top of each other in the background of one element, so you could use solid-color gradients with the colors you want to mix... That wouldn't be the nicest code, though, and I'm not sure how big the perf impact of those gradients would be on Windows/D2D.

> Put together, that looks to be quite a lot of work to just fix the text
> rendering on OS X non-retina... :-\

I agree.

> Markus, is there some way we could force subpixel AA on despite the
> non-opaque background + SVG mask, or can it just not work correctly in these
> circumstances or something?

With really much work, maybe... but I don't think any of the people who could do it have the time for it. What I'm thinking of is to make SVG masks use the PushGroupAndCopyBackground approach that we already use for opacity in BasicLayers. But then again, if I replace the mask in the panel with opacity:0.95, we don't get full subpixel AA either, so that approach apparently doesn't work in the panel - and fixing that part probably means even more work.
Flags: needinfo?(mstange)
(In reply to Markus Stange [:mstange] from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > After discussing with Mike, unfortunately it seems we use -moz-field as the
> > background color on Windows, and -moz-dialog on Linux. Those colors depend
> > on the system theme, so we can't fix this there.
> 
> Well, you can put multiple gradients on top of each other in the background
> of one element, so you could use solid-color gradients with the colors you
> want to mix... That wouldn't be the nicest code, though, and I'm not sure
> how big the perf impact of those gradients would be on Windows/D2D.

Does it not optimize single-color gradients? Or, alternatively, could we use an optimized data URI png with the flat hsla values? I guess that still leaves the -moz-field or -moz-dialog background...

Or we could just always fix the foreground/background colors. :-\
(In reply to Markus Stange [:mstange] from comment #5)
> > Markus, is there some way we could force subpixel AA on despite the
> > non-opaque background + SVG mask, or can it just not work correctly in these
> > circumstances or something?
> 
> With really much work, maybe... but I don't think any of the people who
> could do it have the time for it. What I'm thinking of is to make SVG masks
> use the PushGroupAndCopyBackground approach that we already use for opacity
> in BasicLayers. But then again, if I replace the mask in the panel with
> opacity:0.95, we don't get full subpixel AA either, so that approach
> apparently doesn't work in the panel - and fixing that part probably means
> even more work.

This almost sounds like implementing bug 877294 would be easier. Although, perhaps that'd have the same impact in terms of disabling AA?
Needs an owner to figure out what to do here, over to Dao.
Assignee: nobody → dao
jrmuizel and I are looking at attempting to _disable_ subpixel AA on transparent surfaces with d2d enabled in bug 974607, as a potential perf win for the customization transition.

I could be wrong, but it sounds like these two bugs conflict.
(In reply to Mike Conley (:mconley) from comment #9)
> jrmuizel and I are looking at attempting to _disable_ subpixel AA on
> transparent surfaces with d2d enabled in bug 974607, as a potential perf win
> for the customization transition.
> 
> I could be wrong, but it sounds like these two bugs conflict.

Not really, we should just move away from using transparency at all on these button states.
Dao, have an update on this?
Flags: needinfo?(dao)
I have an idea how to fix this, still need to try it out...
Flags: needinfo?(dao)
Attached patch patchSplinter Review
This applies the SVG mask only to toolbarbuttons that need it. Since most toolbarbuttons won't need it, they'll get sub-pixel AA again...
Attachment #8387610 - Flags: review?(jaws)
Comment on attachment 8387610 [details] [diff] [review]
patch

Review of attachment 8387610 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, I can see a noticeable perf improvement when opening the menu panel with this patch attached. It's still not perfect, but this patch made it better.
Attachment #8387610 - Flags: review?(jaws) → review+
Whiteboard: [Australis:P2] → [Australis:P2][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/cc50c0d103cb
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P2][fixed-in-fx-team] → [Australis:P2]
Target Milestone: --- → Firefox 30
Comment on attachment 8387610 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 897496
User impact if declined: subpar text quality
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #8387610 - Flags: approval-mozilla-aurora?
Attachment #8387610 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
Verified as fixed on the latest Firefox 29 beta and Firefox 30 aurora builds on Windows 7 64bit, Mac OS X 10.9.2 and Ubuntu 13.04 32bit.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.