Closed Bug 1425243 Opened 2 years ago Closed 2 years ago

CompositorOGL rendering bug with mask applied to content with animated transform

Categories

(Core :: Graphics, defect, P3)

58 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
firefox60 --- verified
firefox61 --- verified

People

(Reporter: camden.narzt, Assigned: botond)

References

Details

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

Attachments

(4 files)

Attached video bug.mov
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171211020921

Steps to reproduce:

reset firefox
load https://codepen.io/anon/pen/RxPozq
hover mouse over the logo img


Actual results:

really weird graphical glitch (see attached video if it doesn't reproduce for you)


Expected results:

no graphical glitches
Work for me in Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0 ID:20171211020921.
Component: Untriaged → Layout
Keywords: testcase
Product: Firefox → Core
WFM too, probably a graphics issue from the video.
Component: Layout → Graphics
This is animated PNG?
Flags: needinfo?(aosmond)
Priority: -- → P3
Whiteboard: [gfx-noted]
It's not an animated png. It's a normal png being rotated behind an svg (the svg acts as a mask).
Yes static PNG and SVG, it is a CSS animation. Additionally, the drawing glitches are outside the bounds of the image.
Flags: needinfo?(aosmond)
Any chance for a mozregression run to help us pinpoint the cause?
I can't repro on Linux. Perhaps this is a Mac-specific regression?
Alexis could reproduce this on Mac, but only with WebRender disabled. The problem goes away with WebRender enabled.

Cam, could you check whether enabling WebRender (in about:config, set "gfx.webrender.enabled" to true and then restart) fixes the problem for you as well?
Flags: needinfo?(camden.narzt)
Setting that option in about:config and restarting (and refreshing the page for good measure) made no difference to the bug for me, but I'm on dev-edition 58.0b15 (updated today) so I'm not sure if webrender is truly enableable (last I heard it was nightly only).
Flags: needinfo?(camden.narzt)
You're right, WebRender is only supported on the Nightly channel. Never mind then.
Actually, I can reproduce this on Linux with hardware acceleration force-enabled. That suggests the issue may be related to CompositorOGL.
Dev Edition just bumped up to v59 does anyone know if WebRender is now enableable? I'll retest with it on if so.
No, WebRender remains nightly-only.
I've been investigating this, and it does indeed look like a bug in CompositorOGL. I'm exploring an idea for a fix.
Assignee: nobody → botond
Summary: animation glitch → CompositorOGL rendering bug with mask applied to content with animated transform
The attached patch intersects the (transformed) bounds of the mask (if there is one) into the clip rect used in CompositorOGL::DrawGeometry().

It fixes the testcase for me locally.

Try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=880aab99bcc27edd96dd69e741e0360bcb361570
Attachment #8947301 - Flags: review?(mstange)
I wrote a reftest for this. Going to make sure it's passing on all platforms before posting for review:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dae45b1cb28a57ef221240548743dbad53880eee

I had to use requestAnimationFrame + reftest-wait to trigger the incorrect rendering (as in the original STR, it only happens while animating, not when the image is still), and I had to fuzz it generously to account for what appears to be a snapping-related issue. However, without the fix in place, the reftest fails with a max difference value of 255, well outside the fuzz boundary, so it still seems useful.
(In reply to Botond Ballo [:botond] from comment #18)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=dae45b1cb28a57ef221240548743dbad53880eee

In trying to verify that my new reftest passes on all platforms (it does, I just need to adjust the fuzzing a bit), I discovered that my *fix* breaks two existing reftests on Android:

  layout/reftests/async-scrolling/offscreen-clipped-blendmode-1.html 
  layout/reftests/bugs/1169331-1.html
Attachment #8947301 - Flags: review?(mstange)
Dropping review flag until we sort out the test failures.
The issue appears to be that the clip rect is relative to the render target, while the (post-transform) mask bounds is relative to [whatever the render target offset is relative to], so the two can't be intersected without adjusting by the render target offset.

Try push for revised patch:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=491d23ce46621444aef522ece8342473e0316226
Now with a second reftest to catch the coordinate systems problem on desktop (thanks to Markus for the testcase!):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=664197df033262543adb1778487794975d05b16a
The first reftest randomly fails with WebRender, with or without my patch, and it's not a fuzzing problem, so for now I've marked it random-if(webrender). I'll file a follow-up to track investigating and enabling it.

I also haven't quite got the fuzzing right yet for the second reftest: on Windows 7, it has "max difference: 1, number of differing pixels: 31219", while on Windows 10, it has "max difference: 55, number of differing pixels: 777". Do you know what's the proper way to annotate that?
Latest Try run, for reference:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1e10d849eb80b047bb3e65aed6e3e30819ceed4

This is just before I added the random-if(webrender) annotation for the first reftest.
(In reply to Botond Ballo [:botond] from comment #26)
> I also haven't quite got the fuzzing right yet for the second reftest: on
> Windows 7, it has "max difference: 1, number of differing pixels: 31219",
> while on Windows 10, it has "max difference: 55, number of differing pixels:
> 777". Do you know what's the proper way to annotate that?

Bas tells me the difference is that the Windows 10 machines use D2D.

Let's see if this Try gets all the annotations right:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b8e8f2d1b10d75f8e8537b902d3fdddd718b8ef
Looking good! Posting updated patches for review.
Comment on attachment 8947301 [details]
Bug 1425243 - Intersect the clip rect in CompositorOGL::DrawGeometry() with the mask bounds if there is a mask.

https://reviewboard.mozilla.org/r/217030/#review224916

::: gfx/layers/opengl/CompositorOGL.cpp:1083
(Diff revision 2)
> +    const gfx::Matrix4x4& maskTransform = effectMask->mMaskTransform;
> +    NS_ASSERTION(maskTransform.Is2D(), "How did we end up with a 3D transform here?!");
> +    maskBounds = Rect(Point(), Size(maskSize));
> +    maskBounds = maskTransform.As2D().TransformBounds(maskBounds);
> +
> +    clipRect = clipRect.Intersect(RoundedOut(maskBounds) - mCurrentRenderTarget->GetOrigin());

You can use offset instead of mCurrentRenderTarget->GetOrigin()
Attachment #8947301 - Flags: review?(mstange) → review+
Comment on attachment 8949278 [details]
Bug 1425243 - Reftest.

https://reviewboard.mozilla.org/r/218646/#review224918

There are two commented-out "background-image:..." declarations that you may want to remove.
Attachment #8949278 - Flags: review?(mstange) → review+
Comment on attachment 8949279 [details]
Bug 1425243 - Second reftest to catch the coordinate problem with the first fix attempt on desktop.

https://reviewboard.mozilla.org/r/218648/#review224922
Attachment #8949279 - Flags: review?(mstange) → review+
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1866dea08fcb
Intersect the clip rect in CompositorOGL::DrawGeometry() with the mask bounds if there is a mask. r=mstange
https://hg.mozilla.org/integration/autoland/rev/498f5e1caf9c
Reftest. r=mstange
https://hg.mozilla.org/integration/autoland/rev/794dc13908e5
Second reftest to catch the coordinate problem with the first fix attempt on desktop. r=mstange
Addressed review comments and landed.
(In reply to Botond Ballo [:botond] from comment #26)
> The first reftest randomly fails with WebRender, with or without my patch,
> and it's not a fuzzing problem, so for now I've marked it
> random-if(webrender). I'll file a follow-up to track investigating and
> enabling it.

Filed bug 1437608 for this.
QA Whiteboard: [good first verify]
I successfully reproduced this issue on Firefox 59.0a1 (2017-12-11) under macOS 12.16 using the STR and link attached in Comment 0.

The issue is not reproducible on Firefox Beta 60.0b9 and latest Nightly 61.0a1 (2018-04-02) under Windows 10 (x64), Ubuntu 16.04 (x64) and macOS 10.12.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.