[webkit-reviews] review granted: [Bug 60008] [chromium] Use mapTexSubImage2D for tile uploads if available : [Attachment 92905] Nits.

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted Nat Duca
<nduca at chromium.org>'s request for review:
Bug 60008: [chromium] Use mapTexSubImage2D for tile uploads if available
https://bugs.webkit.org/show_bug.cgi?id=60008

Attachment 92905: Nits.
https://bugs.webkit.org/attachment.cgi?id=92905&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92905&action=review

See if you can consolidate the extension query+enable, but otherwise this looks
fine.

> Source/WebCore/platform/graphics/chromium/LayerTilerChromium.cpp:61
> +    m_useMapSubForUploads =
layerRendererContext()->getExtensions()->supports("GL_CHROMIUM_map_sub");
> +    if (m_useMapSubForUploads)
> +	  
layerRendererContext()->getExtensions()->ensureEnabled("GL_CHROMIUM_map_sub");

Rather than querying+enabling this once per LayerTilerChromium can we do this
on the LRC to just do it once per compositor context?  It seems awfully
wasteful to have to do this once per layer.

> Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplay.cpp:76
> +    m_useMapSubForUploads =
m_layerRenderer->context()->getExtensions()->supports("GL_CHROMIUM_map_sub");
> +    if (m_useMapSubForUploads)
> +	  
m_layerRenderer->context()->getExtensions()->ensureEnabled("GL_CHROMIUM_map_sub
");

This could reuse the bit on LRC if it existed there.