Closed Bug 1333329 Opened 3 years ago Closed 3 years ago

Fix painting during device reset


(Core :: Graphics: Layers, defect)

Not set



Tracking Status
firefox54 --- fixed


(Reporter: dvander, Assigned: dvander)




(4 files)

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.
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)
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)
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)
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)
Attachment #8829773 - Flags: review?(rhunt) → review+
Attachment #8829774 - Flags: review?(rhunt) → review+
Attachment #8829777 - Flags: review?(rhunt) → review+
Attachment #8829778 - Flags: review?(rhunt) → review+
These look great. Thanks David!
Pushed by
Use a simpler mechanism for tracking which ref layers need device resets. (bug 1333329 part 1, r=rhunt)
Remove the device counter mechanism from gfxPlatform. (bug 1333329 part 2, r=rhunt)
Ignore paints when a content device reset has not yet been acknowledged. (bug 1333329 part 3, r=rhunt)
Fix bogus check in OnRenderingDeviceReset. (bug 1333329 part 4, r=rhunt)
You need to log in before you can comment on or make changes to this bug.