Fix painting during device reset

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(4 attachments)

This came out of bug 1160157 and bug 1322741. The problem is this:
 1. Parent process triggers a device reset on window A.
 2. Compositor sends new TextureFactoryIdentifiers to tabs 1, 2, and 3.
 3. Message for tab 1 arrives. Reset content device, flush layers.
 4. Message for tab 2 arrives. Flush layers.
 5. Tab 3 attempts to paint; has stale devices.
 6. Message for tab 3 arrives. Flush layers.

Since the messages to update the TextureFactoryIdentifier aren't atomic, we can paint in between handling the device reset.

To fix this, we'd have to first reset all of the compositors for all windows, then send a single message per content process issuing a device reset and asking all TabChildren to request a new TextureFactoryIdentifier synchronously.

A simpler option might be to just fix ClientLayerManager so that it does not paint if it knows about a pending reset.

Note: None of this applies to UI layers since we handle them differently.
Created attachment 8829773 [details] [diff] [review]
part 1, simplify update acks

The compositor uses a counter to see if the number of device reset acknowledgements is balanced. Instead, we can just compare it to the most recent sequence number.
Attachment #8829773 - Flags: review?(rhunt)
Created attachment 8829774 [details] [diff] [review]
part 2, rm the device counter

Remove the device counter mechanism. It doesn't really do anything. The parent always resets atomically, and this doesn't stop the child from painting again with a bad TextureFactoryIdentifier.
Attachment #8829774 - Flags: review?(rhunt)
Created attachment 8829777 [details] [diff] [review]
part 3, fix the bug

This caches the most recent device reset sequence number in each ClientLayerManager. CLMs then refuse to paint if the sequence number doesn't match. It can only be updated by updating the TextureFactoryIdentifier, which we assume was preceded by invalidating layers.

For this to work, we're relying on the fact that all windows in the UI are reset, and for each one, all attached PBrowsers are also reset. That's true except for basic compositors - we never reset them, so there's a special case. (Maybe it'd be better to just remove all that special casing...)
Attachment #8829777 - Flags: review?(rhunt)
Created attachment 8829778 [details] [diff] [review]
part 4, fix think-o

This fixes a bug in OnRenderingDeviceReset. If for some reason the widget shouldn't use acceleration, but does anyway, we don't treat that as a reset.
Attachment #8829778 - Flags: review?(rhunt)

Updated

2 years ago
Attachment #8829773 - Flags: review?(rhunt) → review+

Updated

2 years ago
Attachment #8829774 - Flags: review?(rhunt) → review+

Updated

2 years ago
Attachment #8829777 - Flags: review?(rhunt) → review+

Updated

2 years ago
Attachment #8829778 - Flags: review?(rhunt) → review+

Comment 5

2 years ago
These look great. Thanks David!

Comment 6

2 years ago
Pushed by danderson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb5a00c7d799
Use a simpler mechanism for tracking which ref layers need device resets. (bug 1333329 part 1, r=rhunt)
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b4b15be0453
Remove the device counter mechanism from gfxPlatform. (bug 1333329 part 2, r=rhunt)
https://hg.mozilla.org/integration/mozilla-inbound/rev/4da258d42dee
Ignore paints when a content device reset has not yet been acknowledged. (bug 1333329 part 3, r=rhunt)
https://hg.mozilla.org/integration/mozilla-inbound/rev/f603c9fd66d4
Fix bogus check in OnRenderingDeviceReset. (bug 1333329 part 4, r=rhunt)

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bb5a00c7d799
https://hg.mozilla.org/mozilla-central/rev/1b4b15be0453
https://hg.mozilla.org/mozilla-central/rev/4da258d42dee
https://hg.mozilla.org/mozilla-central/rev/f603c9fd66d4
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.