This only happens in accelerated drawing path.
http://trac.webkit.org/changeset/93360 might fix this? It make the root and non-root layer paths much more similar.
Created attachment 104417 [details] proposed patch
Comment on attachment 104417 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=104417&action=review > Source/WebCore/platform/graphics/chromium/LayerTilerChromium.cpp:664 > + GLC(context, context->bindTexture(GraphicsContext3D::TEXTURE_2D, 0)); I'm not sure I understand why this is here - is the problem here that skia is assuming a certain texture binding, but that isn't true here?
Comment on attachment 104417 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=104417&action=review >> Source/WebCore/platform/graphics/chromium/LayerTilerChromium.cpp:664 >> + GLC(context, context->bindTexture(GraphicsContext3D::TEXTURE_2D, 0)); > > I'm not sure I understand why this is here - is the problem here that skia is assuming a certain texture binding, but that isn't true here? It is not needed to fix this particular bug. This bug was yet another case of state management issue between skia and compositor, so I took a pass through the accelerated code path and fixed the functions that left GL state in a non-default state. I think in general it is a good practice to restore the GL state if it is not too expensive.
ping!
Comment on attachment 104417 [details] proposed patch The flush seems fine but I'm not really a fan of making unrelated changes that don't do anything. We have not been trying to keep all GL state on the compositor in any particular state (nor do I think that we would want to), so restoring some bits of state in an ad-hoc fashion seems to just be adding complexity for no benefit. If we do need a given GL state to be maintained on the compositor context then we should design, test, and be rigorous about it.
Comment on attachment 104417 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=104417&action=review > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:791 > // FIXME: These calls can be made once, when the compositor context is initialized. I think this FIXME should be removed. Now that the compositor's context is shared with non-compositor code, it will be impossible to assume that any given state will be stick across frames.
(In reply to comment #6) > (From update of attachment 104417 [details]) > The flush seems fine but I'm not really a fan of making unrelated changes that don't do anything. We have not been trying to keep all GL state on the compositor in any particular state (nor do I think that we would want to), so restoring some bits of state in an ad-hoc fashion seems to just be adding complexity for no benefit. If we do need a given GL state to be maintained on the compositor context then we should design, test, and be rigorous about it. That particular change is not restoring the state in any ad-hoc manner. It is just restoring it the default state. We do that in multiple places. For example LayerRendererChromium::drawLayersInternal() restores STENCIL_TEST and BLEND state. LayerTextureUpdaterSkPicture::updateTextureRect() rebinds default frame-buffer. Anyways I have removed it from this patch because it did not have any effect. We need to start thinking about state management as the compositor context is increasingly used by skia for content rendering.
Comment on attachment 104417 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=104417&action=review >> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:791 >> // FIXME: These calls can be made once, when the compositor context is initialized. > > I think this FIXME should be removed. Now that the compositor's context is shared with non-compositor code, it will be impossible to assume that any given state will be stick across frames. DONE
I meant ad-hoc in the sense that there are other places where the compositor sets GL state and does not restore it. I don't know if we have decided whether skia will continue to use the compositor context or not long-term.
Created attachment 105030 [details] proposed patch Removed the change to restore bound texture and removed FIXME.
Comment on attachment 105030 [details] proposed patch Looks good.
Comment on attachment 105030 [details] proposed patch Clearing flags on attachment: 105030 Committed r93727: <http://trac.webkit.org/changeset/93727>
All reviewed patches have been landed. Closing bug.