[Webkit-unassigned] [Bug 44127] [chromium] Thumbnails not generated for GPU Rendered Pages

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44127





--- Comment #13 from Vangelis Kokkevis <vangelis at chromium.org>  2010-08-19 15:09:05 PST ---
(In reply to comment #12)
> (In reply to comment #11)
> > 
> > WebKit/chromium/src/WebViewImpl.cpp:981
> >  +              ASSERT(pixels);
> > This ASSERT could move outside the #if clause so that it runs for both Skia and CG
> 
> Sure, done.
> 
> > WebKit/chromium/src/WebViewImpl.cpp:963
> >  +          if (canvas) {
> > This code will probably need to move after the call to m_layerRenderer->drawLayers() so that we get fresh results. I guess the problem is that drawLayers calls swap buffers so it will be too late. A possible solution would be to split the call to swap buffers out to a different function (say: LayerRendererChromium::present()) and call that from WebViewImpl, after drawLayers and readFramebufferPixels.
> 
> For thumbnail generation, the logic that invokes WebViewImpl::paint() and provides a non-Null canvas is guaranteed to make that call *after* the page has been rendered (at least) once. Thus far I haven't seen any issues with thumbnails coming back blank, and it would be nice to avoid re-rendering the page just to grab the thumbnail pixels.
> 
> It's possible to to do something like LayerRendererChromium::present(), but if we're trying to avoid re-rendering in the first place then is it needed?

I think that since you opted for a more general solution of integrating the readback in the paint method (which is a good thing), you cannot make assumptions on how this method will be called and whether the compositor will have rendered something before.  I think splitting out the swapbuffers call is worth it here, and is a fairly small change.

> 
> > WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:518
> >  +      glReadPixels(0, 0, width, height,
> > Are there any guarantees that the width and height passed in match those of the framebuffer? You should probably at a minimum put an assert there and/or clamp to the size of the framebuffer.
> 
> Good idea - should I compare/clamp against m_rootLayerTextureWidth/Height ?

yes, those are good values to clamp against.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.