[chromium] Ubercomp: clean up CCRenderer interface
Created attachment 152873 [details] Patch
This is an small cleanup to reduce boilerplate in the ubercomp implementation. When CCHeadsUpDisplay is better integrated into LayerRendererChromium, we should make private finishDrawingFrame() as well.
Comment on attachment 152873 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152873&action=review > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:368 > + renderPass->targetSurface()->damageTracker()->didDrawDamagedArea(); This won't be able to stay here obviously, once targetSurface() is gone. So if we are going to do this, we have to guarantee that every render pass given to drawFrame() is actually drawn. I'm not sure that this is going to make our lives easier.. I'm currently adding another piece to the code in CCLTHI::drawLayers() that would then go here. But it really belongs there. CCLTHI manages the RenderPasses, LRC just draws them. So overall, I feel like this is not going to help us and isn't worth removing one method from the API. :/
For ubercomp, I was going to have to stash away pointers to the renderpasses as they came in, reconstruct the render pass list, and then do the actual work in finishDrawingFrame(). It seems to me that the one-at-a-time model is an implementation detail of OpenGL. Why shouldn't we make sure that the only renderpasses sent to CCRenderer will be actually drawn; that seems like a good idea anyway? We can still do some work on the side in CCLTHI; I'm fine with moving the didDrawDamaged area back there in a loop over the render passes.
Created attachment 153082 [details] Patch Move didDrawDamagedArea back to LTHI
Comment on attachment 153082 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153082&action=review R=me. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:555 > + m_layerRenderer->drawFrame(frame.renderPasses, m_rootScissorRect); > + if (m_headsUpDisplay->enabled(settings())) > + m_headsUpDisplay->draw(this); > + m_layerRenderer->finishDrawingFrame(); It's a little weird to me to have this asymmetric finish function without an exposed begin, but I understand that you need to draw some extra stuff in the frame here. The heads up display is a little bit of a wart, and I'm not sure if it should be generating quads or be completely contained within LRC or something else. Refactoring for the future, I guess.
Comment on attachment 153082 [details] Patch Clearing flags on attachment: 153082 Committed r123033: <http://trac.webkit.org/changeset/123033>
All reviewed patches have been landed. Closing bug.
Reverted r123036 for reason: Triggers some DCHECKs in Chromium browser_tests Committed r123120: <http://trac.webkit.org/changeset/123120>
Comment on attachment 153082 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153082&action=review >> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:555 >> + m_layerRenderer->finishDrawingFrame(); > > It's a little weird to me to have this asymmetric finish function without an exposed begin, but I understand that you need to draw some extra stuff in the frame here. The heads up display is a little bit of a wart, and I'm not sure if it should be generating quads or be completely contained within LRC or something else. Refactoring for the future, I guess. Making quads I think so a child renderer can display a hud.
(In reply to comment #9) > Reverted r123036 for reason: > > Triggers some DCHECKs in Chromium browser_tests > > Committed r123120: <http://trac.webkit.org/changeset/123120> The ChangeLog got mangled, it wasn't this bug that got reverted.