1431582 - Visual artifacts on http://thewebrocks.com/demos/mozilla-monument-name-finder/

Status: RESOLVED FIXED

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

Description

[Darkspirit]

Attachment: 2018-01-19_03-33-55.mp4

This is a spin-off from bug 1420748.
http://thewebrocks.com/demos/3D-css-tester/ looks good/fixed.

But http://thewebrocks.com/demos/mozilla-monument-name-finder/ got only a bit better. This spin-off is to track that.

Attached video:
ESR 52.5.0, 20180107220443, 20180110221942, 20180118220148 @ Debian Testing (KDE, Radeon RX480)

Comment 1

[Kartikaya Gupta (email:[e-mail])]

I don't see much of a difference between the ESR and the 20180118220148 ones, can you describe a little which artifacts you're referring to? On OS X latest nightly when I open the URL it looks ok to me.

Comment 2

[Darkspirit]

Attachment: 2018-01-19_17-31-33.mp4

ESR shows everything just perfectly.
a) Sides are firmly connected and
b) there are no gray areas flickering on the upper corners.

Attached video = today's 20180119103852
* to a: focus on the outer right. Orange slides into blue, they are not connected. The outer left is not perfect, too.
* to b: those text snippets from the bottom appear in the corners on the top.
* the number "2" is cut-off on the left for a moment everytime it is at the front.

Comment 3

[Darkspirit]

(A fresh non-WR profile shows it perfectly like ESR.)

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Thanks! Locally on OS X I see issue (a) now, but not issue (b). I see both issues in your video.

Comment 5

[Darkspirit]

Nightly 59 x64 20180122100120 de_DE @ Windows 10 1709 (Radeon RX480)
fresh profile: gfx.webrender.all
It looks exactly the same as on Debian.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

I see both on Windows Nvidia.

Comment 7

[Darkspirit]

(bug 1436058) https://github.com/servo/webrender/compare/30f2cd5e648bbcadbe8200d479de3f7d76089d0b...e7c95ae9fb6b7f8d973ae5106a1ba8be6adea007

Fixed: Those grey text snippets from the bottom do not longer appear in the top corners.

Still broken:
1) "focus on the outer right. Orange slides into blue, they are not connected. The outer left is not perfect, too."
2) "the number "2" is cut-off on the left for a moment everytime it is at the front." (and "4" on the right hand side)

Comment 8

[Mayank Bansal]

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #7)
> (bug 1436058)
> https://github.com/servo/webrender/compare/
> 30f2cd5e648bbcadbe8200d479de3f7d76089d0b...
> e7c95ae9fb6b7f8d973ae5106a1ba8be6adea007
> 
> Fixed: Those grey text snippets from the bottom do not longer appear in the
> top corners.
> 
> Still broken:
> 1) "focus on the outer right. Orange slides into blue, they are not
> connected. The outer left is not perfect, too."
> 2) "the number "2" is cut-off on the left for a moment everytime it is at
> the front." (and "4" on the right hand side)

Point 1 appears to be fixed
Regarding Point2: "2" and "4" are now visible properly, but they flicker when they come into view.

Comment 9

[Darkspirit]

Could the flickering be a variant of bug 1454706 comment 8?

Comment 10

[Aria Beingessner [:Gankra]]

This looks like Z-fighting, and therefore probably the same bug as https://bugzilla.mozilla.org/show_bug.cgi?id=1491088 ?

Comment 11

[Dzmitry Malyshau [:kvark]]

It's not actually Z-fighting here, since the surfaces that aren't sorted correctly are distanced quite a bit from each other. But it is a plane-splitting/sorting issue for sure, I looked at it a bit.

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Reduced test case, v1

Comment 13

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Andrew Osmond [:aosmond] from comment #12)

Created attachment 9039196 [details]
Reduced test case, v1

Point of interest: It bends when you change the cube class rotation from -61 degrees (correct, nice and straight) to -62 degrees (bad, pulled up in the middle).

Comment 14

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Reduced test case, v2 -- text is bent

Simplified the transforms further.

Comment 15

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Reduced test case, v2 -- text is straight

As as the failure case, but it is rotated one fewer degree.

Comment 16

[Andrew Osmond [:aosmond] (he/him)]

Looks like we are tripping:

https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/gfx/wr/webrender/src/picture.rs#2433-2440

If I disable this, the test case passes and we no longer distort the original page as it turns. However there is still some minor flickering that is presumably unrelated.

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Guess this is bug 1521656 then?

Comment 18

[Andrew Osmond [:aosmond] (he/him)]

Attachment: WR/RenderDoc captures for the reduced test case

Produced off from https://hg.mozilla.org/mozilla-central/rev/5d9c5aa981a5b824e6132c923c2f201b3fa49397

Comment 19

[Dzmitry Malyshau [:kvark]]

Attachment: WR support non-locally-rasterized split planes

Now that we no longer guarantee that a picture with perspective transform is rasterized in local space, we need to ensure that the shaders don't apply perspective correction to the texture coordinates twice.
For that to be the case, we pass an extra flag to the plane splitting shader, and un-do the perspective correction if it's not enabled.

Comment 20

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1431582 - Add reduced test case for 3d transform bugs.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4eadfbd77cb5
Add reduced test case for 3d transform bugs. r=kvark

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4eadfbd77cb5

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/131a5437e7d3
WR support non-locally-rasterized split planes r=gw

Comment 24

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=131a5437e7d31985f2a73d09fd76f8a1e4fdf648&tochange=494cb7f6b6764588f0d334e5dd1423249a58c849&selectedJob=224740129

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=224740129&repo=autoland&lineNumber=2974

Backout link: https://hg.mozilla.org/integration/autoland/rev/494cb7f6b6764588f0d334e5dd1423249a58c849

Comment 25

[Dzmitry Malyshau [:kvark]]

I learned now how to fire up try pushes that include WR reftests.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/653f85bc0bf8
WR support non-locally-rasterized split planes r=gw

Comment 27

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/653f85bc0bf8

Comment 28

[Dzmitry Malyshau [:kvark]]

Comment on attachment 9039607 [details]
WR support non-locally-rasterized split planes

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1505934

User impact if declined

Visual glitches in pages that involve filters, 3D transforms, and preserve-3D stacking contexts.

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Medium

Why is the change risky/not risky? (and alternatives if risky)

In the worst case, we'll have more glitches, but nothing will explode.

String changes made/needed

Comment 29

[Liz Henry (:lizzard)]

Are you also going to land the reduced test case?

Comment 30

[Liz Henry (:lizzard)]

Comment on attachment 9039607 [details]
WR support non-locally-rasterized split planes

Fix to support some issues with 3d transforms, includes adding a test back in.
OK to uplift for beta 4.

Comment 31

[Dzmitry Malyshau [:kvark]]

Liz,

Yes, thanks for spotting this. My change relies on https://bugzilla.mozilla.org/attachment.cgi?id=9039632 being merged as well.

Comment 32

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/8f2ea35d4601
https://hg.mozilla.org/releases/mozilla-beta/rev/dc3e444e4a2c

Comment 33

[Cornel Ionce [:noni]]

(In reply to Dzmitry Malyshau [:kvark] from comment #28)

Is this code covered by automated tests?

Yes

Needs manual test from QE?

No

Marking as qe- per comment 28.

Comment 34

[Darkspirit]

No difference to non-WR anymore. Thanks!