1554502 - Webrender + Tree Style Tab: Visual artifact with text behind transparent div

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Okarin]

Attachment: testcase.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

On Windows + Nvidia and Linux + Intel with nightly and Webrender enabled.

I can reproduce by following these steps:

• Create a new profile
• Make sure Webrender is used (about:support)
• Install Tree Style Tab (3.0.14) : https://addons.mozilla.org/en-US/firefox/addon/tree-style-tab/
• Open the attached file (testcase.html)
• Move your cursor over the "+" button in the Tree Style Tab sidebar.

Actual results:

The text of the "content" div is rendered over the "header" div.

Expected results:

The text from the "content" div should be rendered behind the "header" div. It should still be visible as the header is transparent (opacity: 0.9).

Comment 1

[Okarin]

Attachment: testcase.png

Comment 2

[Okarin]

Attachment: youtube.png

This also affects real websites like YouTube on video pages (https://www.youtube.com/watch?v=BCUwP2csmBM)

Comment 3

[Okarin]

mozregression:

app_name: firefox
build_date: 2019-02-07 16:25:33.242000
build_type: inbound
build_url: https://queue.taskcluster.net/v1/task/CdAUon1lRhu-ruqgT837IQ/runs/0/artifacts/public%2Fbuild%2Ftarget.zip
changeset: 0258dc6318a821647ec61519590de275b6663ad6
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0258dc6318a821647ec61519590de275b6663ad6&tochange=f526887aa3ae0087bbb9553627453eb9ecdec871
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: CdAUon1lRhu-ruqgT837IQ

Differential Revision: https://phabricator.services.mozilla.com/D18917

Comment 4

[Alice0775 White]

I can reproduce the issue on Nightly69.0a1 Windows10.

Comment 5

[Daniel Bodea [:danibodea]]

Setting flags based on the regressor's pushed changeset.

Comment 6

[Dzmitry Malyshau [:kvark]]

Thank you for the test cases!
I narrowed it down to our batching code by making the lookback size configurable. If we stop trying to merge with previous batches, the re-ordering stops occurring. The fact that it occurs tells me that our bounding rect is incorrect.

Comment 7

[Dzmitry Malyshau [:kvark]]

Attachment: Configurable lookback count

We've had a constant of 10 hard-coded there since early days.
Turning it into a configurable number allows us to easier tune it and
debug related issues.

Comment 8

[Ryan Hunt [:rhunt]]

From a conversation with Dzmitry on irc, it sounded like the issue is that there were multiple display lists with cache_tiles=true. This was being caused by having multiple top-level content documents rendering to the same window.

If this is accurate, we should be able to use the 'primary' attribute that is set on <browser>'s in order to make a unique cache_tiles decision.

Here's a try run of a patch that implements this [1]. I tried to keep it as simple as possible so that this could be uplifted.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=590172ab92118c2c0ad1ec6ba0ae113f88ef0169

Comment 9

[Ryan Hunt [:rhunt]]

Hmm, a bit odd. I can still reproduce the issue with that test case with those patchers.

I added some logging and confirmed that only the main content area is setting cache_tiles=true. The tree-style tabs panel is setting cache_tiles=false.

Dzmitry, did I misunderstand the issue? And if not, would you be able to test out the patches from the try run or take a look at them?

Comment 10

[Dzmitry Malyshau [:kvark]]

Ryan, thank you for producing and testing the patch!
I observed the following:

1. The main DL has 2 iframes, both of which contain a stacking context with "cache_tiles=true"
2. Setting them to false leads to fixing the bug in this case

From that, and the fact we know that tile caching is only expected to be enabled on at most a single stacking context (today, at least), I concluded that (1) is triggering the issue. If it's still happening, I need to look once more into it, with your patch applied.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53ac7ce00afa
Configurable lookback count r=gw

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/53ac7ce00afa

Comment 13

[Dzmitry Malyshau [:kvark]]

I figured out the problem, at last. Filed bug 1556053 separately for the cache flag changes.

Comment 14

[Dzmitry Malyshau [:kvark]]

Attachment: Track the reference frame of batched bounding boxes and enforce it in WR

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05bbf91cf528
Track the reference frame of batched bounding boxes and enforce it in WR r=gw

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/05bbf91cf528

Comment 17

[Julien Cristau [:jcristau]]

Should this ride the trains or do we want it in 68?

Comment 18

[Dzmitry Malyshau [:kvark]]

I believe this is safe to uplift to 68, if there is still a room to do so. Both patches are needed.

Comment 19

[Dzmitry Malyshau [:kvark]]

Comment on attachment 9067880 [details]
Configurable lookback count

Beta/Release Uplift Approval Request

• User impact if declined: Visual artifacts when WebRender is enabled on some page/extension combinations, including the Tree Style Tab
• Is this code covered by automated tests?: No
• 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: Low
• Why is the change risky/not risky? (and alternatives if risky): Non-risky, since it just makes batching more conservative.
• String changes made/needed:

Comment 20

[Julien Cristau [:jcristau]]

Comment on attachment 9067880 [details]
Configurable lookback count

webrender fix for 68.0b9

Comment 21

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/4ab0defb9012
https://hg.mozilla.org/releases/mozilla-beta/rev/e4b071df2c1c

Comment 22

[Oana Pop-Rus]

Backed out 2 changesets (bug 1554502) for causing build bustages.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&resultStatus=superseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&revision=121bb815438386f3f7b31a39da9aa3f29bc96f18&selectedJob=250918187

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=250918187&repo=mozilla-beta&lineNumber=15734

Backout: https://hg.mozilla.org/releases/mozilla-beta/rev/73ae511ea89559d9c1b848a8d27cfe1fb6b2e034

Comment 23

[Dzmitry Malyshau [:kvark]]

I'll make a combined change that builds and works from these on Beta.

Comment 24

[Dzmitry Malyshau [:kvark]]

Attachment: wr-tree-tabs-beta.patch

The automatic conflict merging got confused in this case because of the specific of other changes that came in before. Attaching here a combined patch that I resolved manually when cherry-picking.

Small try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a80f75fcec039b319d5ced675142a07fc841f2fd

Would you be able to uplift it to Beta? there is only a few character difference from the patch that failed :)

Comment 25

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/016c1fb63aaf