[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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #66041|review?                     |review-
               Flag|                            |




--- Comment #33 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-08-31 13:14:33 PST ---
(From update of attachment 66041)
> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:337
> +    OwnPtr<unsigned char> lineTemp(new unsigned char[rowBytes]);
nit: use OwnArrayPtr here

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:339
> +
nit: remove this extra blank line

> WebCore/platform/graphics/chromium/LayerRendererChromium.h:94
> +    int rootLayerTextureWidth() const { return m_rootLayerTextureWidth; }
nit: this should probably return IntSize:

  IntSize rootLayerTextureSize() const { ... }

There was a thread recently on webkit-dev about how we should be using IntSize,
IntPoint, etc. more to help clean up the codebase.

> WebCore/platform/graphics/chromium/LayerRendererChromium.h:96
> +    void getFramebufferPixels(void *pixels, const int width, const int height);
nit: can this function take an IntSize parameter?

  void getFramebufferPixels(void* pixels, const IntSize&);

> WebKit/chromium/src/WebViewImpl.cpp:982
> +        if (canvas) {
nit: this function is now clearly way too long.  please break it up into
helper functions: one for SKIA and one for CG.

> WebKit/chromium/src/WebViewImpl.cpp:987
> +
nit: remove this blank line

> WebKit/chromium/src/WebViewImpl.cpp:1002
> +                    OwnPtr<skia::PlatformCanvas> canvasResize = OwnPtr<skia::PlatformCanvas>(new skia::PlatformCanvas());
why does this PlatformCanvas need to be heap allocated?

> WebKit/chromium/src/WebViewImpl.cpp:1010
> +                        canvas->drawBitmapRect(bitmapResize, &srcRect, dstRect, 0);
in ImageSkia.cpp, i see similar code, but it tends to enable filtering
when doing linear re-sampling.

have you considered replacing all of this code with some code that
uses GraphicsContext and BitmapImage?  save for the GraphicsContext
construction, i bet you could write some portable code for this if
you used those classes.  WebFrameImpl::paint shows how to setup the
GraphicsContext.

One more comment:  is it really best to implement this using HW readback
instead of just engaging the software painting path?

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