Closed Bug 1431582 Opened 2 years ago Closed 7 months ago

Categories

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

x86_64
All
defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 --- disabled
firefox60 --- disabled
firefox61 --- disabled
firefox65 --- wontfix
firefox66 --- fixed
firefox67 --- fixed

People

(Reporter: darkspirit, Assigned: kvark)

References

(Depends on 1 open bug, Blocks 2 open bugs, )

Details

(Keywords: nightly-community)

Attachments

(7 files, 1 obsolete file)

Attached video 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)
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.
Attached video 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.
(A fresh non-WR profile shows it perfectly like ESR.)
Thanks! Locally on OS X I see issue (a) now, but not issue (b). I see both issues in your video.
Nightly 59 x64 20180122100120 de_DE @ Windows 10 1709 (Radeon RX480)
fresh profile: gfx.webrender.all
It looks exactly the same as on Debian.
OS: Linux → All
I see both on Windows Nvidia.
Flags: needinfo?(milan)
(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)
See Also: → 1454706
See Also: → 1486113
(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.
QA Whiteboard: [flicker]
Could the flickering be a variant of bug 1454706 comment 8?
Flags: needinfo?(milaninbugzilla)
This looks like Z-fighting, and therefore probably the same bug as https://bugzilla.mozilla.org/show_bug.cgi?id=1491088 ?
Flags: needinfo?(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.
Flags: needinfo?(kvark)
Priority: P2 → P3
Assignee: nobody → aosmond
Attached file Reduced test case, v1 (obsolete) —

(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).

Simplified the transforms further.

Attachment #9039196 - Attachment is obsolete: true

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

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.

Guess this is bug 1521656 then?

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.

Keywords: leave-open
Pushed by aosmond@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4eadfbd77cb5
Add reduced test case for 3d transform bugs. r=kvark
Assignee: aosmond → dmalyshau
Pushed by dmalyshau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/131a5437e7d3
WR support non-locally-rasterized split planes r=gw

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

Flags: needinfo?(dmalyshau)
Pushed by dmalyshau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/653f85bc0bf8
WR support non-locally-rasterized split planes r=gw

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

Attachment #9039607 - Flags: approval-mozilla-beta?

Are you also going to land the reduced test case?

Flags: needinfo?(dmalyshau)

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.

Attachment #9039607 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Liz,

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

Flags: needinfo?(dmalyshau)

(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.

Flags: qe-verify-

No difference to non-WR anymore. Thanks!

Status: NEW → RESOLVED
Closed: 7 months ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.