I get flickering on mac with basic layers. Appears to be when scrolling and happens frequently in the Youtube playback controls: https://www.youtube.com/watch?v=nJEOAP-iWY8&feature=youtu.be

Regression windows points to bug 1176011.

A better test case is pan and resize on youtube.com

Quick update, as I've been silent thus far. I am currently working on this. It looks similar to https://bugzilla.mozilla.org/show_bug.cgi?id=1265873 and I've confirmed it's a regression from my patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1176011. I thought I had a solution, but it turned out to not work :( Back to the code I go...

Also, for posterity; this affects Safe Mode for OS X so it's more important than it would seem to a casual observer.

It was easier to reproduce the problem when e10s was disabled.

On tiled layer, ReadUnlock() in BufferTextureHost::MaybeUpload() seems to break the locking logic. When I commented out it, the flicker seems to be addressed.

(In reply to Sotaro Ikeda [:sotaro PTO & offline 10/Aug-15/Aug] from comment #6) > On tiled layer, ReadUnlock() in BufferTextureHost::MaybeUpload() seems to > break the locking logic. > When I commented out it, the flicker seems to be addressed. https://dxr.mozilla.org/mozilla-central/source/gfx/layers/composite/TextureHost.cpp#813

:nical, can you comment to comment 6?

HasIntermediateBuffer should be false. With basic compositor, BufferTextureData has non-IntermediateBuffer by Bug 1250500. But it HasIntermediateBuffer was true.

When shared pool was used, aAllocator->AsCompositableForwarder() was nullptr. Then hasIntermediateBuffer became true.

Hmm, when BufferTextureData was allocated by CompositorBridgeChild. we do not know the compositor backend type. It could be different on each compositor.

(In reply to Sotaro Ikeda [:sotaro] from comment #11) > Hmm, when BufferTextureData was allocated by CompositorBridgeChild. we do > not know the compositor backend type. It could be different on each > compositor. This is a problem. We should make it possible by design to use the same TextureClient/Host with multiple compositor Backend. It could kind of work with buffer texture (but in fact it doesn't work), and it clearly does not work with more complicated texture types. The recycling logic should take the compositor backend into account in all cases, but this also an issue more generally for things that allocate textures without knowing about the compositor backend (I think that some of the video code paths have this issue). It's very bug-prone and probably mostly work by chance.

Comment on attachment 8781648 [details] [diff] [review] 0001-Bug-1293908-Specify-the-LayersBackend-to-be-used-whe.patch Review of attachment 8781648 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/BufferTexture.cpp @@ +483,5 @@ > } > > auto fwd = aAllocator ? aAllocator->AsCompositableForwarder() : nullptr; > + bool hasIntermediateBuffer; > + if (aLayersBackend == LayersBackend::LAYERS_NONE) { Is there a way to write this in a way that aLayersBackend is never LAYERS_NONE? Creating a texture without knowing the compositor backend is not a good thing, so we should be able to change this into an assertion that fwd->GetCompositorBackendType() == aLayersBackend, rather than pick whichever of the two gives us something interesting. Or at least in the short term make it so we can assume that if aLayersBackend is NONE, then aFwd is null because we should have passed that information higher up the stack.

Pushed by gwright@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9b4c4fd433a9 Specify the LayersBackend to be used when creating Textures r=nical

Backed out for blowing up Linux64 tests. https://treeherder.mozilla.org/logviewer.html#?job_id=34363386&repo=mozilla-inbound#L2338 https://hg.mozilla.org/integration/mozilla-inbound/rev/ca51bc087957

Heh, it looks like what happened here is that between my try run and landing, Andrew landed a change to turn on GL layers by default in Nightly. Apparently my patch broke GL layers with Linux. So glad we have tests for that now! :)

Pushed by gwright@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f9dfb9c04270 Specify the LayersBackend to be used when creating Textures r=nical

Nical, I opted for option 2) in your review where we assert if aLayersBackend is NONE, then aFwd is null.

George, this is in the list of beta50 regressions (fixed in 51), want to request an uplift?

Comment on attachment 8781648 [details] [diff] [review] 0001-Bug-1293908-Specify-the-LayersBackend-to-be-used-whe.patch Approval Request Comment [Feature/regressing bug #]: bug 1176011. [User impact if declined]: flickering when scrolling on OS X when basic layers is active (mostly impacts safe moode). [Describe test coverage new/current, TreeHerder]: it's been in Nightly for a month now. [Risks and why]: hard to assess, but I'd say low-to-medium risk as the changes are fairly straightforward, and it's been fine on Nightly for a long time. [String/UUID change made/needed]: none

Hello Ben, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment on attachment 8781648 [details] [diff] [review] 0001-Bug-1293908-Specify-the-LayersBackend-to-be-used-whe.patch This fix has stabilized on Nightly51 for a month, seems stable to uplift to Beta50.

I didn't managed to reproduce this bug entirely, on Mac OS X 10.11.6 using an old Nightly build. I verified this issue on 50.0b7 (2016-10-13) and 51.0a2 (2016-10-14) and tried different scenarios, like Bennoit said, in comment 0 and comment 2. I think is safe to say that this issue is verified fixed, due the fact that I didn't encountered or seen flickering issues anymore on Youtube. The test were performed on Mac OS X 10.11.6. If you have any concerns please let me know.