Closed Bug 1216349 Opened 5 years ago Closed 5 years ago

Black boxes since 2014-04-11 with e10s off and D3D9 OMTC HWA


(Core :: Graphics, defect)

32 Branch
Not set



Tracking Status
e10s - ---
firefox43 + verified
firefox44 + verified


(Reporter: Oriol, Assigned: bas.schouten)



(Keywords: regression)


(5 files)

Attached file testcase
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20151019030227

Steps to reproduce:

I occasionally see black boxes, specially when scrolling.
Finally I have been able to create a reproducible testcase.

Just make sure multi-process is disabled and OMTC hardware acceleration is enabled. I can reproduce with both D3D9 and D3D11. Then open the testcase.

Actual results:

JS will scroll automatically, and some black boxes will appear.

Expected results:

There should be no black boxes.

Last good nightly: 2014-05-20 cb9f34f73ebe
First bad nightly: 2014-05-21 9d8d16695f6a

Sadly it seems it's too late and the inbound builds have been removed.
See Also: → 1211615
Component: Untriaged → Graphics
OS: Unspecified → Windows
Product: Firefox → Core
Version: 44 Branch → 32 Branch
Bas, want to fix?
Flags: needinfo?(bas)
OMTC being enabled on Windows is in that range. Conveniently, I filed bug 1019909 around that time as well, but it fizzled out due to difficulties reproducing and eventually the move to Treeherder.
Blocks: 899785
Ever confirmed: true
Keywords: regression
See Also: → 1019909
See Also: → 1217215
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #2)
> OMTC being enabled on Windows is in that range.

True, enabling layers.offmainthreadcomposition.enabled manually produced this range for D3D9:

Last good nightly: 2014-04-10 690c810c8e3e
First bad nightly: 2014-04-11 d8c1b10c3a3d

Since D3D11 has another range, I have filed bug 1217215.
Summary: Black boxes since 2014-05-21 with e10s off, HWA on, OMTC on → Black boxes since 2014-04-11 with e10s off and D3D9 OMTC HWA
Assignee: nobody → bas
Flags: needinfo?(bas)
Here's a couple of interesting observations:

1. Scroll the test case down to below the vertical line's start, and scroll back up, the black areas are now permanently gone.
2. Wait for the layers to become inactive, the black areas are now permanently gone.
3. Switch tabs away and back, black areas are now permanently gone.
Attached image Reduced case - Frame 1
Attached image Reduced case - Frame 2
Attached image Reduced case - Frame 3
The transition from the second to the third frame here is interesting. Notice some odd stuff going on in the state in the second frame. Note they share the same 375x615 TextureHost. In the second frame almost the entire TextureHost is considered Valid, even though that area isn't in the visible region at all. In the subsequent third frame we see that that area considered valid in the second frame, but that is not in the visible region, is black. That makes complete sense.

So current observations suggest, that perhaps counterintuitively, thinks go wrong in the -second- frame where there are no artifacts yet, by overestimating the validity of the TextureHost.
The problem here is that we create a new texture host when the visible region changes between frame 1 and 2, we then copy the valid contents of the old one into the new one. When we send the updated region to the compositor though, that only contains the valid region that is also in the visible region. Which means that part of the new texture host is never uploaded.

This patch makes it so that when a new texture host is received by a content host, it will make sure to upload the entire old valid region regardless of whether it's in the current updated region.
Attachment #8678142 - Flags: review?(nical.bugzilla)
Should we uplift this?
Flags: needinfo?(bas)
Attachment #8678142 - Flags: review?(nical.bugzilla) → review+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
(In reply to Milan Sreckovic [:milan] from comment #10)
> Should we uplift this?

Yes, probably, but I would like to wait a couple of days to make sure this doesn't cause any issues.
Flags: needinfo?(bas)
The patch seems to fix bug 1217215 too. Maybe I shouldn't have separated it into a new bug? It had a different regression range, though.
I think you did the right thing given the different range. Feel free to dupe it over or close it out marked as depending on this bug, though.
Duplicate of this bug: 1217215
(In reply to Bas Schouten (:bas.schouten) from comment #13)
> I would like to wait a couple of days to make sure this
> doesn't cause any issues.

Is it OK to uplift it now?
Comment on attachment 8678142 [details] [diff] [review]
Upload the old valid region as well if our texture host changed

Approval Request Comment
[Feature/regressing bug #]: OMTC
[User impact if declined]: Black areas on websites
[Describe test coverage new/current, TreeHerder]: Nightly coverage
[Risks and why]: Low, tiny perf regression possibility
[String/UUID change made/needed]: None
Attachment #8678142 - Flags: approval-mozilla-beta?
Tracking since this is a regression and marking affected for 43.
Comment on attachment 8678142 [details] [diff] [review]
Upload the old valid region as well if our texture host changed

Fix for another OMTC regression. OK to uplift to beta.
Attachment #8678142 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Reproduced the initial issue using an old Nightly build (2015-10-24), verified that the issue is fixed using Firefox 43 beta 4 and latest Firefox Developer Edition 44.0a2 on Windows XP 32-bit.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.