Closed Bug 1460389 Opened 6 years ago Closed 6 years ago

Emoticon bubble on Github disappears for a short moment

Categories

(Core :: Graphics: WebRender, defect, P1)

x86_64
All
defect

Tracking

()

VERIFIED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- unaffected
firefox62 --- disabled

People

(Reporter: jan, Assigned: hiro)

References

(Blocks 1 open bug, )

Details

(Keywords: nightly-community, regression)

Attachments

(7 files)

Attached video 2018-05-09_20-35-12.mp4 (deleted) —
Debian Testing, KDE, Radeon RX480 Win10 1803, Nvidia GTX1060 2560x1440 (Dell U2515H) mozregression --good 2018-05-07 --bad 9294f67b3f3bd4a3dd898961148cecd8bfc1ce9c --pref gfx.webrender.all:true startup.homepage_welcome_url:'https://github.com/carllerche/h2/pull/273' > 9:39.38 INFO: Last good revision: 3b84be9ee9e6a19d8e49aa86b71c9d60b5eaec8b > 9:39.38 INFO: First bad revision: 1a51d479e4404608fea41fc4a8e3d5b21b1850a7 > 9:39.38 INFO: Pushlog: > https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3b84be9ee9e6a19d8e49aa86b71c9d60b5eaec8b&tochange=1a51d479e4404608fea41fc4a8e3d5b21b1850a7 > 1a51d479e440 Hiroyuki Ikezoe — Bug 1456679 - Enable tests in test_animations_omta.html on WebRender. r=kats > 217999de6c88 Hiroyuki Ikezoe — Bug 1456679 - Don't set non-animated values as AnimatedValue in delay phase. r=kats > a7074da0c89c Hiroyuki Ikezoe — Bug 1456679 - Update the previous timestamp with the current timestamp even if there are only delayed phase animations. r=kats > 8e9fc4d911cf Hiroyuki Ikezoe — Bug 1456679 - Make SampleAnimations return boolean to tell there is any animations even if the animation in delay phase. r=kats
It seems animation on pseudo element. We may use a parent nsIFrame somewhere. Anyway I need to debug it, I'd want devtools to stop this kind of hover triggered animation. :)
Flags: needinfo?(hikezoe)
Attached file A simplified test case (deleted) —
Ok, now I think I understand what's going on there. From WebRenderBridgeParent::CompositeToTarget <https://hg.mozilla.org/mozilla-central/file/96b37ba12225/gfx/layers/wr/WebRenderBridgeParent.cpp#l1328>; // We do this even if the arrays are empty, because it will clear out any // previous properties store on the WR side, which is desirable. txn.UpdateDynamicProperties(opacityArray, transformArray); I've read this comment before, but missed somehow. Though I don't have any ideas to solve this, assigning to myself for now.
Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
Flags: needinfo?(hikezoe)
A solution I can think of is not to override static opacity/transform value if the array is empty, but I am not sure we can do it, not I am reading webrender code.
Here is a try with an attempt at very early stage. https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c3beb632516940cca34384006cb710cccdc22eb This try fixes the original case, but doesn't fix transform animation case yet. And also I noticed that animations with positive end delay case doesn't originally work either (i.e. before bug 1456679). So we have to figure out a way to fix these bunch of the issues.
I've filed for the end delay issue in another bug (bug 1460804) since the issue also happens on non-WebRender.
Here is another try that makes webrender changes as separate commits. https://treeherder.mozilla.org/#/jobs?repo=try&revision=849d2cc1cd9189f37d324615fb5b6978731848fe I will create a PR to webrender repo.
Attached file The PR to webrender (deleted) —
Comment on attachment 8974916 [details] Bug 1460389 - Remove TransactionBuilder::AppendTransformProperties. https://reviewboard.mozilla.org/r/243294/#review249148
Attachment #8974916 - Flags: review?(bugmail) → review+
Comment on attachment 8974917 [details] Bug 1460389 - Constify arguments for StackingContextHelper ctor. https://reviewboard.mozilla.org/r/243296/#review249150
Attachment #8974917 - Flags: review?(bugmail) → review+
Comment on attachment 8974918 [details] Bug 1460389 - Use non-animated (static) value if the animation is in not in-effect. https://reviewboard.mozilla.org/r/243298/#review249156 LGTM. Because of the way our WR update process works I might end up landing this patch as part of bug 1459935 or whatever bug ends up landing the PR you made.
Attachment #8974918 - Flags: review?(bugmail) → review+
Comment on attachment 8974918 [details] Bug 1460389 - Use non-animated (static) value if the animation is in not in-effect. https://reviewboard.mozilla.org/r/243298/#review249486 ::: commit-message-ada5f:1 (Diff revision 1) > +Bug 1460389 - Use non-animated (static) value if the animation is in delay phase. r?kats,birtles Nit: I think this is not strictly about the delay phase but about any case where the animation is not in-effect. ::: gfx/webrender_bindings/src/bindings.rs:1626 (Diff revision 1) > + // the value for the case where the animation is > + // in delay phase. s/is in delay phase/is in not in-effect (e.g. in the delay phase with no corresponding fill mode)/
Attachment #8974918 - Flags: review?(bbirtles) → review+
Comment on attachment 8974919 [details] Bug 1460389 - Reftests for opacity and transform animations that are sent to the compositor in their delay phase. https://reviewboard.mozilla.org/r/243300/#review249488
Attachment #8974919 - Flags: review?(bbirtles) → review+
Depends on: 1460861
Bug 1460861 is on autoland and looks like it will stick. So part 3 of this patchset will need to be rebased on top of that (should be pretty trivial - just keep the new version of the code in your patch) and then this should be good to land.
checkin-needed?
Pushed a final try now, I will land them later. (I didn't know that we can land patches for WebRender separately)
Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/137e5960bb28 Remove TransactionBuilder::AppendTransformProperties. r=kats https://hg.mozilla.org/integration/autoland/rev/8e9820ac53ce Constify arguments for StackingContextHelper ctor. r=kats https://hg.mozilla.org/integration/autoland/rev/bfc8fa6e75a2 Use non-animated (static) value if the animation is in not in-effect. r=birtles,kats https://hg.mozilla.org/integration/autoland/rev/63988e7c891f Reftests for opacity and transform animations that are sent to the compositor in their delay phase. r=birtles
Verified fixed in Nightly 62 x64 20180516100125 de_DE @ Debian Testing (Radeon RX480). Thanks! Emoticon bubble on Github and Gmail's loading animation are fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: