Closed Bug 1122526 Opened 9 years ago Closed 9 years ago

[OMTA] Australis doorhanger menu looks pixelated during its opacity fade-in animation

Categories

(Core :: Layout, defect)

41 Branch
x86_64
Windows 7
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla42
Tracking Status
firefox39 --- unaffected
firefox40 + unaffected
firefox41 + verified
firefox42 + verified

People

(Reporter: Virtual, Assigned: dbaron)

References

Details

(Keywords: nightly-community, regression)

Attachments

(12 files)

16.19 KB, image/png
Details
19.46 KB, image/png
Details
20.74 KB, image/png
Details
26.70 KB, image/png
Details
601.75 KB, video/mp4
Details
45.89 KB, image/png
Details
553 bytes, text/html
Details
2.70 KB, text/plain
Details
4.46 KB, patch
roc
: review+
Details | Diff | Splinter Review
5.66 KB, patch
roc
: review+
Details | Diff | Splinter Review
3.62 KB, patch
roc
: review+
Details | Diff | Splinter Review
2.49 KB, patch
roc
: review+
Details | Diff | Splinter Review
Animation quality of the hamburger icon is awful and terrible.
Just look on attached screenshots.
Is it the same issue what you were seeing in bug #872536?
Flags: needinfo?(ehsan)
I don't remember.  Sorry.
Flags: needinfo?(ehsan)
Still, thank you for very fast looking into this.

Aris pointed me it can be caused by "layers.offmainthreadcomposition.enabled" and it's true.

My Graphics section from about:support
Adapter Description	NVIDIA GeForce GTX 460 v2
Adapter Drivers	nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Adapter RAM	1024
Device ID	0x1205
Direct2D Enabled	true
DirectWrite Enabled	true (6.2.9200.16571)
Driver Date	12-13-2014
Driver Version	9.18.13.4709
GPU #2 Active	false
GPU Accelerated Windows	1/1 Direct3D 11 (OMTC)
Subsys ID	00000000
Vendor ID	0x10de
WebGL Renderer	Google Inc. -- ANGLE (NVIDIA GeForce GTX 460 v2 Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote	true
AzureCanvasBackend	direct2d 1.1
AzureContentBackend	direct2d 1.1
AzureFallbackCanvasBackend	cairo
AzureSkiaAccelerated	0
Component: Menus → Graphics: Layers
Keywords: regression
Product: Firefox → Core
Summary: Awful and terrible animation quality of the hamburger icon → Awful and terrible animation quality of the hamburger icon with layers.offmainthreadcomposition.enabled=true (default setting)
Based on the fact it's related to layers.offmainthreadcomposition.enabled and it's an animation, I'm guessing we're using OMTA for this animation, and not rendering it at a high enough resolution in layout at the start of the animation. The code that determines that is in layout, so moving components.
Component: Graphics: Layers → Layout
Also, :Virtual, you tagged this as a regression in comment 7. If it's a regression, can you find a regression window?
Flags: needinfo?(BernesB)
I tagged it as regression per default OMTC enabled, as in the past it was disabled. But I will try find a decent regression range, as I don't remember it occurring before.
Flags: needinfo?(BernesB)
I'm not able to reproduce it anymore in latest Nightly,
even when using Firefox Portable versions (39.0a1, 38.0a2, 37.0b1, 36.0 and 31.5.0 ESR).
Very odd...
No longer blocks: enable-omt-animations
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: regression
Resolution: --- → WORKSFORME
See Also: 963928
Still broken for me on latest nightly on Windows 8.
Status: VERIFIED → REOPENED
Keywords: regression
Resolution: WORKSFORME → ---
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> Based on the fact it's related to layers.offmainthreadcomposition.enabled
> and it's an animation, I'm guessing we're using OMTA for this animation, and

Though OMT Animations aren't currently enabled, so I don't think that's true.
(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #13)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> > Based on the fact it's related to layers.offmainthreadcomposition.enabled
> > and it's an animation, I'm guessing we're using OMTA for this animation, and
> 
> Though OMT Animations aren't currently enabled, so I don't think that's true.

Sorry, I was confusing this with the poor animation performance when OMTA is switched on. I filed bug 1149865 for that.

Restoring the previous resolution.
No longer blocks: enable-omt-animations
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → WORKSFORME
[Tracking Requested - why for this release]: Regression

I'm reopening this, as the issue was mainly with "layers.offmainthreadcomposition.async-animations" set to true.
Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
Summary: Awful and terrible animation quality of the hamburger icon with layers.offmainthreadcomposition.enabled=true (default setting) → Awful and terrible animation quality of the hamburger icon with layers.offmainthreadcomposition.async-animations=true (default setting)
How does this bug differ from bug 947753?
Just noticed that if you click on the hamburger icon and hold the pointer there the animation is fine.  It's when you click on it and then release the mouse button you get the distortion.
(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #16)
> How does this bug differ from bug 947753?

This bug is about poor visual quality,
bug #947753 is about poor performance,
but I don't mind duping this into one bug.
Status: REOPENED → NEW
Oh, this is that it looks pixelated during the animation?
Summary: Awful and terrible animation quality of the hamburger icon with layers.offmainthreadcomposition.async-animations=true (default setting) → Australis hamburger menu looks pixelated during its opacity fade-in animation (with off-main-thread animations enabled, now default setting)
This is a screenrecording of this bug using the try-build of bug 947753 comment 20.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> Based on the fact it's related to layers.offmainthreadcomposition.enabled
> and it's an animation, I'm guessing we're using OMTA for this animation, and
> not rendering it at a high enough resolution in layout at the start of the
> animation. The code that determines that is in layout, so moving components.

Even with OMTC/OMTA, shouldn't we be able to detect that the resolution is too low and rerasterize? Is the issue that we're doing this, but the latency is just too great to help for this short animation?
Jet, is this something for your team? This should be fixed before we get to aurora or we may need to turn off OMTA.
Flags: needinfo?(bugs)
(In reply to Liz Henry (:lizzard) from comment #23)
> Jet, is this something for your team? 

Yes.

> This should be fixed before we get to aurora or we may need to turn off OMTA.

We'll take a closer look. Thanks!
Flags: needinfo?(bugs)
Flags: needinfo?(bugs)
Fixed by patch in bug 947753, although we should really file a layers followup that things get this bad when we have an extra layer.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Flags: needinfo?(bugs)
I can't reproduce it in latest Nightly (2015-05-03) with patch from bug #947753,
so I'm marking this as FIXED,
but I would like someone to check and confirm this, if it's really true.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Depends on: 947753
Resolution: --- → FIXED
Whiteboard: [fixed by patch from bug #947753]
(In reply to Virtual_ManPL [:Virtual] from comment #26)
> I can't reproduce it in latest Nightly (2015-05-03) with patch from bug
> #947753,
> so I'm marking this as FIXED,
> but I would like someone to check and confirm this, if it's really true.

Same here, fixed in 05-03 for me.
Thank you very much for confirmation.
I already make 2 mistakes here for locking this bug too fast,
so now it's better to make sure, that I won't make mistake by 3rd time.
Status: RESOLVED → VERIFIED
I just landed bug 1161049 on mozilla-inbound, which changes how bug 947753 is fixed -- probably to a way that wouldn't have fixed this bug on its own.  However, bug 1153539 has landed since, and I think that should handle this case.

So once bug 1161049 hits mozilla-central and then hits nightly, it might be worth retesting this to confirm that it's still fixed.
Flags: needinfo?(BernesB)
Target Milestone: --- → mozilla40
(In reply to David Baron [:dbaron] ⏰UTC+2 (busy, returning May 21) from comment #29)
> I just landed bug 1161049 on mozilla-inbound, which changes how bug 947753
> is fixed -- probably to a way that wouldn't have fixed this bug on its own. 
> However, bug 1153539 has landed since, and I think that should handle this
> case.
> 
> So once bug 1161049 hits mozilla-central and then hits nightly, it might be
> worth retesting this to confirm that it's still fixed.

I'm running inbound with this patch and the animation when opening the Hamburger icon is very bad again.  It was working fine.
(In reply to David Baron [:dbaron] ⏰UTC+2 (busy, returning May 21) from comment #31)
> Very bad in what way?

It's the same as what was reported in this bug. The fade in aspect is not fluid anymore.
Is it a low frame rate or a pixelated appearance?
When the panel first appears the icons and text are very dark and then they change to their normal appearance.  I tried to capture this happening but I guess it's happening to fast to capture.  Using no animation is fine.
(In reply to David Baron [:dbaron] ⏰UTC+2 (busy, returning May 21) from comment #29)
> So once bug 1161049 hits mozilla-central and then hits nightly, it might be
> worth retesting this to confirm that it's still fixed.

It's pixelated again, but I think the pixelation is smoother than it was before. Think antialiased.

The animation itself is still smooth, though.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Whiteboard: [fixed by patch from bug #947753]
Status: REOPENED → ASSIGNED
No longer depends on: 947753
Summary: Australis hamburger menu looks pixelated during its opacity fade-in animation (with off-main-thread animations enabled, now default setting) → [OMTA] Australis hamburger menu looks pixelated during its opacity fade-in animation
this affects the bookmarks panel as well
It looks like all of the doorhanger UIs are affected, not just the hamburger and bookmarks menus.

I checked, and the downloads menu, Site Identity menu (Larry), per-site permissions menu (plugin permissions, etc.), and doorhanger UIs from add-ons like uBlock Origin are all impacted.

I'm on yesterday's Aurora 40.0a2 build (2015-05-17) with Direct3D 11 (OMTC) acceleration, and all of the various "offmainthread" options set to their defaults:

html5.offmainthread;true
layers.offmainthreadcomposition.async-animations;true
layers.offmainthreadcomposition.enabled;true
layers.offmainthreadcomposition.force-basic;false
layers.offmainthreadcomposition.frame-rate;-1
layers.offmainthreadcomposition.log-animations;false
layers.offmainthreadcomposition.testing.enabled;false

If my graphics info helps, it's:

Adapter Description	Intel(R) HD Graphics 3000
Adapter Drivers	igdumd64 igd10umd64 igd10umd64 igdumd32 igd10umd32 igd10umd32
Adapter RAM	Unknown
Asynchronous Pan/Zoom	none
Device ID	0x0126
Direct2D Enabled	true
DirectWrite Enabled	true (6.2.9200.16571)
Driver Date	1-30-2015
Driver Version	9.17.10.4101
GPU #2 Active	false
GPU Accelerated Windows	15/15 Direct3D 11 (OMTC)
Subsys ID	21ce17aa
Supports Hardware H264 Decoding	false
Vendor ID	0x8086
WebGL Renderer	Google Inc. -- ANGLE (Intel(R) HD Graphics 3000 Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote	true
AzureCanvasBackend	direct2d 1.1
AzureContentBackend	direct2d 1.1
AzureFallbackCanvasBackend	cairo
AzureSkiaAccelerated	0
Summary: [OMTA] Australis hamburger menu looks pixelated during its opacity fade-in animation → [OMTA] Australis doorhanger menu looks pixelated during its opacity fade-in animation
We should fix this for firefox41. Adding a tracking flag.
OK, I stuck some printfs into nsLayoutUtils::ComputeSuitableScaleForAnimation when this was happening with the hamburger menu, and:
 * maxScale is (0.4, 0.4)
 * minScale is (0, 0)
 * aDisplaySize is the size of the browser window (not the popup)
 * aVisibleSize is the size of the popup (roughly)

The later two mean that displayVisibleRatio is *usually* greater than 1, although not always, especially in the vertical direction (when the popup is taller than the window).  That's not great, but it's not the source of the main problem here.

The main problem is that maxScale is 0.4 -- if I just manually change the 0.4 to 1.0, then the pixelation goes away, as long as the browser window is tall enough, at least.

So I think there are probably two problems:
 (1) [ the main problem ] we're not considering all the transforms we should be, or something like that

 (2) we're using the main window's size instead of the popup's
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d77a934407d (although I know the new reftest will fail on at least some platforms -- I'd like to find out if it's all or some)
(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #42)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d77a934407d
> (although I know the new reftest will fail on at least some platforms -- I'd
> like to find out if it's all or some)

The test fails locally for me, but in automation only fails on Win7.  (It actually fails on any platforms where we have subpixel antialiasing -- I should perhaps just rewrite it to use something other than text.)


The bigger problem is that while the patch does fix the testcase, it doesn't fix the actual bug.  mIncomingScale when the actual bug happens is sometimes 1, sometimes substantially smaller.  I'm not currently sure why.
Attached file scales-output
With https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/fc72291dd1df/omta-scale-ancestor , when testing this bug, we end up hitting the modified code from three different stacks with three different values for aIncomingScale, as shown in this attachment.  So that patch doesn't seem right, despite fixing the simple testcase.
I'm hoping that the build at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-184933115453/try-win32/ fixes this, although I haven't gotten access to a Windows machine today to test.
(In reply to David Baron [:dbaron] ⏰UTC-7 (mostly busy, back June 29) from comment #47)
> I'm hoping that the build at
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.
> com-184933115453/try-win32/ fixes this, although I haven't gotten access to
> a Windows machine today to test.

Still broken.
This fixes the scaling choice when we have more than one layer for the
same element, e.g., because it animates both transform and opacity.
Attachment #8626762 - Flags: review?(roc)
The patch works by not handling transform:none specially at all, which
will lead to a scale of 1 (instead of the current 0).
Attachment #8626763 - Flags: review?(roc)
With the new patch 3, the bug here is actually fixed (and I've tested that).

Another try run to check that my latest reftest adjustments work across platforms:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=de8a626094a5
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-de8a626094a5/try-win32/

With that build, the icons and text-strings have some minor movement at the end of the animation.
http://elbart.bplaced.net/async/async_on.mp4 (slowed down to 1/6th realtime)

For comparison with async-animations off:
http://elbart.bplaced.net/async/async_off.mp4 (slowed down to 1/6th realtime)
So the less than smooth end of the animation mentioned and demonstrated in comment 56 is happening as intended?
(In reply to Elbart from comment #60)
> So the less than smooth end of the animation mentioned and demonstrated in
> comment 56 is happening as intended?

That should be a separate bug report.
Comment on attachment 8626761 [details] [diff] [review]
patch 1 - Factor in the scale from ancestors when computing scale for layer with OMT animation of transform

Approval Request Comment
[Feature/regressing bug #]: OMT Animations
[User impact if declined]: some animations of transform either appear pixelated or use too much memory
[Describe test coverage new/current, TreeHerder]: has reftest coverage
[Risks and why]: reasonably low risk; the main risk is that it might have been covering up other bugs
[String/UUID change made/needed]: no
Attachment #8626761 - Flags: approval-mozilla-aurora?
Comment on attachment 8626762 [details] [diff] [review]
patch 2 - Only do OMTA transform scale choosing for layers that are for transform display items

Approval Request Comment
same as patch 1
Attachment #8626762 - Flags: approval-mozilla-aurora?
Comment on attachment 8626763 [details] [diff] [review]
patch 3 - Correctly account for transform:none when finding maximum scale for a transform animation

Approval Request Comment
same as patch 1 (except only effect here is pixelation, no possibility of excessive memory use)
Attachment #8626763 - Flags: approval-mozilla-aurora?
Comment on attachment 8626764 [details] [diff] [review]
patch 4 - Use the nearest widget size as the maximum size for an animated layer, in case it's a popup larger than the toplevel window

Approval Request Comment
[Feature/regressing bug #]: OMT Animations
[User impact if declined]: animations of transform in a popup that's larger than its parent window will appear pixelated
[Describe test coverage new/current, TreeHerder]: none; it's a pain to test
[Risks and why]: low risk, though there's a chance it might uncover some other bug
[String/UUID change made/needed]: no
Attachment #8626764 - Flags: approval-mozilla-aurora?
(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #61)
> (In reply to Elbart from comment #60)
> > So the less than smooth end of the animation mentioned and demonstrated in
> > comment 56 is happening as intended?
> 
> That should be a separate bug report.

Done - Bug #1180509



I think we also should uplift these patches to Firefox 40 (Beta), as it's also affected.
Any plans for this or should we disable OMTA there?
Comment on attachment 8626761 [details] [diff] [review]
patch 1 - Factor in the scale from ancestors when computing scale for layer with OMT animation of transform

Approving for uplift to Aurora, low risk, it has been in Nightly for approximately a week and is 'Verified'.
Attachment #8626761 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8626762 [details] [diff] [review]
patch 2 - Only do OMTA transform scale choosing for layers that are for transform display items

Approving for uplift to Aurora, low risk, it has been in Nightly for approximately a week and is 'Verified'.
Attachment #8626762 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8626763 [details] [diff] [review]
patch 3 - Correctly account for transform:none when finding maximum scale for a transform animation

Approving for uplift to Aurora, low risk, it has been in Nightly for approximately a week and is 'Verified'.
Attachment #8626763 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8626764 [details] [diff] [review]
patch 4 - Use the nearest widget size as the maximum size for an animated layer, in case it's a popup larger than the toplevel window

Approving for uplift to Aurora, low risk, it has been in Nightly for approximately a week and is 'Verified'.
Attachment #8626764 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 66(In reply to Virtual_ManPL [:Virtual] from comment #66)
> I think we also should uplift these patches to Firefox 40 (Beta), as it's
> also affected.
> Any plans for this or should we disable OMTA there?
Flags: needinfo?(kglazko)
Flags: needinfo?(dbaron)
(In reply to Virtual_ManPL [:Virtual] from comment #72)
> Comment 66(In reply to Virtual_ManPL [:Virtual] from comment #66)
> > I think we also should uplift these patches to Firefox 40 (Beta), as it's
> > also affected.
> > Any plans for this or should we disable OMTA there?

It's already disabled there, since the enabling there was conditional on #ifdef RELEASE_BUILD.
Flags: needinfo?(kglazko)
Flags: needinfo?(dbaron)
Ah, that's why.
Thank you very much for detailed explanation.
So I'm marking Firefox 40 as unaffected.
Depends on: 1215180
You need to log in before you can comment on or make changes to this bug.