LayerChromium-derived classes should not have direct access to LayerRendererChromium, as it lives on the compositor thread. So, refactor it so it doesn't need it. This also requires updating all of the texture updaters to not cache memeber pointers to GraphicsContext3D and instead take them as function args when needed.
Created attachment 104571 [details] Patch
Comment on attachment 104571 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104571&action=review I gotta run, but I still owe you this review. > Source/WebCore/platform/graphics/chromium/ImageLayerChromium.cpp:97 > + ImageLayerTextureUpdater(bool useMapTexSubImage) guess what keyword this constructor now gets!
(In reply to comment #2) > (From update of attachment 104571 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104571&action=review > > I gotta run, but I still owe you this review. > > > Source/WebCore/platform/graphics/chromium/ImageLayerChromium.cpp:97 > > + ImageLayerTextureUpdater(bool useMapTexSubImage) > > guess what keyword this constructor now gets! I need to figure out how to hook this up in the style bot.
Created attachment 104721 [details] Patch
(In reply to comment #4) > Created an attachment (id=104721) [details] > Patch Now with more 'explicit'.
Comment on attachment 104721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104721&action=review > Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:132 > + m_textureUpdater = LayerTextureUpdaterBitmap::create(ContentLayerPainter::create(m_owner), host->contextSupportsMapSub()); contextSuportsMapSub is moving to the LayerRendererSide. Is there any way to not need this at construction time?
(In reply to comment #6) > (From update of attachment 104721 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104721&action=review > > > Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:132 > > + m_textureUpdater = LayerTextureUpdaterBitmap::create(ContentLayerPainter::create(m_owner), host->contextSupportsMapSub()); > > contextSuportsMapSub is moving to the LayerRendererSide. Is there any way to not need this at construction time? My expectation was that we would have an explicit init phase where we queried the graphics context for things like max texture sizes and support for features. So, we would have those settings on both sides. You're going to need to know those things before the first commit anyway. Is this not feasible?
We don't need to know if we have mapSub or not until updateCompositorResources()
Comment on attachment 104721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104721&action=review Looks like a step in the right direction. For max texture size it makes sense to grab that information during initialization and stash it. To be truly exhaustive, we should be refetching this on every lost context, since the new context could have a different limit (if the user changed their gfx driver on win7, or they switched to some other output device or whatever). For mapSub support and bestTextureFormat, we shouldn't need that until upload time so I wonder if we can do something fancier. No great harm in stashing it for now, though. > Source/WebCore/platform/graphics/chromium/LayerTextureUpdaterCanvas.h:55 > + LayerTextureUpdaterCanvas(PassOwnPtr<LayerPainterChromium>); harro one-arg constructor~~~~~~
Committed r93615: <http://trac.webkit.org/changeset/93615>
(In reply to comment #9) > (From update of attachment 104721 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104721&action=review > > Looks like a step in the right direction. > > For max texture size it makes sense to grab that information during initialization and stash it. To be truly exhaustive, we should be refetching this on every lost context, since the new context could have a different limit (if the user changed their gfx driver on win7, or they switched to some other output device or whatever). For mapSub support and bestTextureFormat, we shouldn't need that until upload time so I wonder if we can do something fancier. No great harm in stashing it for now, though. I'd categorize these with scroll offset, as properties we can pull from the compositor thread when requesting a frame. And, if any of these properties change, we can just invalidate the world. > > Source/WebCore/platform/graphics/chromium/LayerTextureUpdaterCanvas.h:55 > > + LayerTextureUpdaterCanvas(PassOwnPtr<LayerPainterChromium>); > > harro one-arg constructor~~~~~~ /me mumbles.