Closed Bug 1423528 Opened 2 years ago Closed 2 years ago

CSS animation bug in the Firefox for Windows only

Categories

(Core :: Graphics: Layers, defect, P3)

57 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- verified
firefox60 --- verified

People

(Reporter: etherreals777, Assigned: mattwoodrow)

References

(Depends on 1 open bug)

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171128222554

Steps to reproduce:

If I add transform scale to the inner element and transform rotate to the wrapper element, it'll rotate with artifacts on Mozilla Firefox
The issue occurs on Firefox for Windows.
I've tested it 47.0.2 and 57.0.1 versions.
Firefox 57.0.1 works good for Ubuntu
Here is the code https://plnkr.co/edit/uX0V8QYiJI7e6Rae1niS?p=preview



Actual results:

I see artifacts
The screenshot attached


Expected results:

It must animate without artifacts
Not a security issue that needs to stay hidden.
Group: firefox-core-security
Component: Untriaged → Layout
Product: Firefox → Core
I can reproduce.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=958d2a5d10091401fd5e900e8e063d21940c137e&tochange=7f894f791cdf170d788507d0eff30024ce699523


Via local build,
Last good: build from cset 1561522b1179 + patch 7f894f791cdf
First bad: build from cset 68932b1a5b71 + patch 7f894f791cdf


Regressed by: Matt Woodrow — Bug 1361970 - Make PostProcessLayers occlusion culling work against the surface we will draw to rather than the parent layer. r=mstange
Blocks: 1361970
Component: Layout → Graphics: Layers
Flags: needinfo?(matt.woodrow)
Whiteboard: [gfx-noted]
This is bug 1381753 again, except for Advanced Layers this time. Getting reftests for async animations would be nice so that we don't break this a third time.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Attachment #8951043 - Flags: review?(mstange)
Attachment #8951043 - Flags: review?(mstange) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32bf2817fb7a
Recompute shadow visible rect for ContainerLayerMLGPU in case it has been changed by async animations. r=mstange
https://hg.mozilla.org/mozilla-central/rev/32bf2817fb7a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
We're late in the cycle and the lack of tests for this makes me nervous. That said, it grafts cleanly to Beta as-landed. Should we consider this for Beta uplift or let it ride the trains?
Flags: needinfo?(matt.woodrow)
The lack of tests make me nervous too.

That said, this is just a conversion of the existing fix to be used on advanced layers, so the risk should be very low.

I think we should go for uplift!
Flags: needinfo?(matt.woodrow)
Comment on attachment 8951043 [details] [diff] [review]
Recompute visible rect for ContainerLayerMLGPU

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1361970
[User impact if declined]: Incorrect invalidation (trail left behind) for some async transform animations on Windows. Affects google calendar.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just takes the existing fix, and enables it for advanced layers.
[String changes made/needed]: None.
Attachment #8951043 - Flags: approval-mozilla-beta?
Duplicate of this bug: 1436751
Duplicate of this bug: 1424846
Comment on attachment 8951043 [details] [diff] [review]
Recompute visible rect for ContainerLayerMLGPU

Let's get this into tomorrow's 59b12 build so we can get it tested as much as possible before we ship 59. Marking qe-verify+ as well to get some manual verification.
Attachment #8951043 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Comment on attachment 8951043 [details] [diff] [review]
Recompute visible rect for ContainerLayerMLGPU

Not a new regression (it's from 56) but let's give this a try in 59. 

Matt, maybe you could file a followup bug for writing reftests ?

Aleksandr, would you mind verifying the fix in nightly (60) or in 59 beta 12 later this week?
Flags: needinfo?(etherreals777)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #13)

> Matt, maybe you could file a followup bug for writing reftests ?

Unfortunately this codepath is in code that we can't hit using reftests. It's specific to async animations, and we don't have any infrastructure currently that can test the rendering of those.

We have bug 1248828 filed for getting that support, but it hasn't been finished yet.
Depends on: 1248828
I've managed to reproduce this bug using STR from comment 0, on an affected Nightly build (2017-12-06).

I can no longer see the issue on latest Nightly 60.0a1 under Win 10 x64, Win 8.1 x86 and Win 7 x64.

Ni? myself as a reminder to verify this on 59.0b12 as well.
Status: RESOLVED → VERIFIED
Flags: needinfo?(ciprian.georgiu)
This bug is also verified fixed on Beta 59.0b12 (20180222170353) across the same platforms mentioned in the above comment.
Flags: qe-verify+
Flags: needinfo?(ciprian.georgiu)
Duplicate of this bug: 1435395
Flags: needinfo?(etherreals777)
You need to log in before you can comment on or make changes to this bug.