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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 3 10:09:52 PDT 2010


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


Vangelis Kokkevis <vangelis at chromium.org> changed:

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




--- Comment #48 from Vangelis Kokkevis <vangelis at chromium.org>  2010-09-03 10:09:51 PST ---
(From update of attachment 66505)
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 44b15f411d30c6fef6233acc6ae7a7eede7eb10f..9926e8d0ab4b023aba4e4775050263441f55d62e 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,22 @@
> +2010-09-02  W. James MacLean  <wjmaclean at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [chromium] Thumbnails not generated for GPU Rendered Pages
> +        https://bugs.webkit.org/show_bug.cgi?id=44127
> +
> +        Replicates existing functionality, use existing tests.
> +
> +        Adds pixel-readback for GPU composited pages to allow for thumbnailing,
> +        printing and other services to work with GPU rendered pages.
> +
> +        * platform/graphics/chromium/LayerRendererChromium.cpp:
> +        (WebCore::LayerRendererChromium::drawLayers):
> +        (WebCore::LayerRendererChromium::present):
> +        (WebCore::LayerRendererChromium::getFramebufferPixels):
> +        * platform/graphics/chromium/LayerRendererChromium.h:
> +        (WebCore::LayerRendererChromium::rootLayerTextureSize):
> +
>  2010-09-02  Andreas Kling  <andreas.kling at nokia.com>
>  
>          Rubber-stamped by Simon Hausmann.
> diff --git a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> index 50338d2d5f7324e5a119c509190dfef8698b1769..5c3c0b1b316564f4dad5baef0feb458f11fee150 100644
> --- a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> +++ b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> @@ -39,6 +39,7 @@
>  #include "GLES2Context.h"
>  #include "LayerChromium.h"
>  #include "NotImplemented.h"
> +#include <wtf/OwnArrayPtr.h>
>  #if PLATFORM(SKIA)
>  #include "NativeImageSkia.h"
>  #include "PlatformContextSkia.h"
> @@ -321,11 +322,41 @@ void LayerRendererChromium::drawLayers(const IntRect& updateRect, const IntRect&
>  
>      GLC(glDisable(GL_SCISSOR_TEST));
>  
> +    glFlush();
> +}
> +
> +void LayerRendererChromium::present()
> +{
>      m_gles2Context->swapBuffers();
>  
>      m_needsDisplay = false;
>  }
>  
> +void LayerRendererChromium::getFramebufferPixels(void *pixels, const IntRect& rect)
> +{
> +    ASSERT(rect.size() == rootLayerTextureSize());

Shouldn't you be asserting that it's not larger than the rootLayerTextureSize?  It won't always be equal to it, right?

> +
> +    if (!pixels)
> +        return;
> +
> +    makeContextCurrent();
> +
> +    GLC(glReadPixels(rect.x(), rect.y(), rect.width(), rect.height(),
> +                     GL_RGBA, GL_UNSIGNED_BYTE, pixels));
> +
> +    // Flip pixels vertically.
> +    const int rowBytes = 4 * rect.width();
> +    OwnArrayPtr<unsigned char> lineTemp(new unsigned char[rowBytes]);
> +    for (int row1 = 0, row2 = rect.height() - 1; row1 < rect.height() / 2; ++row1, --row2) {
> +        unsigned char* ptr1 = static_cast<unsigned char*>(pixels) + row1 * rowBytes;
> +        unsigned char* ptr2 = static_cast<unsigned char*>(pixels) + row2 * rowBytes;
> +
> +        memcpy(lineTemp.get(), ptr1, rowBytes);
> +        memcpy(ptr1, ptr2, rowBytes);
> +        memcpy(ptr2, lineTemp.get(), rowBytes);
> +    }
> +}
> +
>  // FIXME: This method should eventually be replaced by a proper texture manager.
>  unsigned LayerRendererChromium::createLayerTexture()
>  {
> diff --git a/WebCore/platform/graphics/chromium/LayerRendererChromium.h b/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> index 8f44afebd0a2c7b475d04ba48723f1cdc5455091..ffe41423d53e3f540e710233faf2a0f81562ce57 100644
> --- a/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> +++ b/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> @@ -64,6 +64,7 @@ public:
>      // Updates the contents of the root layer that fall inside the updateRect and recomposites
>      // all the layers.
>      void drawLayers(const IntRect& updateRect, const IntRect& visibleRect, const IntRect& contentRect, const IntPoint& scrollPosition);
> +    void present(); // Perform buffer swap to present rendered buffer.
>  
>      void setRootLayer(PassRefPtr<LayerChromium> layer) { m_rootLayer = layer; }
>      LayerChromium* rootLayer() { return m_rootLayer.get(); }
> @@ -90,6 +91,9 @@ public:
>      const ContentLayerChromium::SharedValues* contentLayerSharedValues() const { return m_contentLayerSharedValues.get(); }
>      const CanvasLayerChromium::SharedValues* canvasLayerSharedValues() const { return m_canvasLayerSharedValues.get(); }
>  
> +    IntSize rootLayerTextureSize() const { return IntSize(m_rootLayerTextureWidth, m_rootLayerTextureHeight); }
> +    void getFramebufferPixels(void *pixels, const IntRect& rect);
> +
>  private:
>      void updateLayersRecursive(LayerChromium* layer, const TransformationMatrix& parentMatrix, float opacity);
>  
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> index 527e8efccae39045fd985ded3bf20d7166afc69d..65f0e9d4d9a42d16bdccd97988b4abb8b19227ab 100644
> --- a/WebKit/chromium/ChangeLog
> +++ b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,23 @@
> +2010-09-02  W. James MacLean  <wjmaclean at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [chromium] Thumbnails not generated for GPU Rendered Pages
> +        https://bugs.webkit.org/show_bug.cgi?id=44127
> +
> +        Modified WebViewImpl::paint() to detect non-null canvas pointers when
> +        accelerated compositing is active, and instead fills the pixel buffer
> +        from the GPU framebuffer. Includes re-scaling support when provided
> +        canvas does not match size of current render layer. Limits pixel
> +        readback to rect passed to paint(), clipped by size of rootLayerTexture.
> +
> +        * src/WebViewImpl.cpp:
> +        (WebKit::WebViewImpl::paint):
> +        (WebKit::clearSkBitmap):
> +        (WebKit::WebViewImpl::doPixelReadbackToCanvas):
> +        (WebKit::clearCGBitmap):
> +        * src/WebViewImpl.h:
> +
>  2010-09-02  Ilya Sherman  <isherman at google.com>
>  
>          Reviewed by Eric Seidel.
> diff --git a/WebKit/chromium/src/WebViewImpl.cpp b/WebKit/chromium/src/WebViewImpl.cpp
> index 4b129d694b1f112f27b21ae724db67a68a74e02a..d10b2a8f1acbcbe8493bca580de81e319f3fa952 100644
> --- a/WebKit/chromium/src/WebViewImpl.cpp
> +++ b/WebKit/chromium/src/WebViewImpl.cpp
> @@ -64,7 +64,6 @@
>  #include "HTMLNames.h"
>  #include "Image.h"
>  #include "InspectorController.h"
> -#include "IntRect.h"
>  #include "KeyboardCodes.h"
>  #include "KeyboardEvent.h"
>  #include "MIMETypeRegistry.h"
> @@ -115,6 +114,10 @@
>  #include "WebViewClient.h"
>  #include "wtf/OwnPtr.h"
>  
> +#if PLATFORM(CG)
> +#include <CoreGraphics/CGContext.h>
> +#endif
> +
>  #if OS(WINDOWS)
>  #include "RenderThemeChromiumWin.h"
>  #else
> @@ -952,6 +955,7 @@ void WebViewImpl::paint(WebCanvas* canvas, const WebRect& rect)
>              webframe->paint(canvas, rect);
>  #if USE(ACCELERATED_COMPOSITING)
>      } else {
> +

nit: please remove the added newline

>          // Draw the contents of the root layer.
>          updateRootLayerContents(rect);
>  
> @@ -968,10 +972,118 @@ void WebViewImpl::paint(WebCanvas* canvas, const WebRect& rect)
>  
>          // Ask the layer compositor to redraw all the layers.
>          m_layerRenderer->drawLayers(rect, visibleRect, contentRect, IntPoint(view->scrollX(), view->scrollY()));
> +
> +        // If a canvas was passed in, we use it to grab a copy of the
> +        // freshly-rendered pixels.
> +        if (canvas) {
> +            // Clip rect to the confines of the rootLayerTexture.
> +            IntRect resizeRect(rect.x, rect.y, rect.width, rect.height);
> +            resizeRect.intersect(IntRect(IntPoint(), m_layerRenderer->rootLayerTextureSize()));
> +            doPixelReadbackToCanvas(canvas, resizeRect);
> +        }
> +
> +        m_layerRenderer->present(); // Do final display by swapping buffers.
>      }
>  #endif
>  }
>  
> +#if USE(ACCELERATED_COMPOSITING)
> +#if PLATFORM(SKIA)
> +static inline void clearSkBitmap(const SkBitmap& bitmap)
> +{
> +    bitmap.eraseColor(SkColorSetARGB(0, 0, 0, 0));
> +}
> +
> +void WebViewImpl::doPixelReadbackToCanvas(WebCanvas* canvas, IntRect& rect)
> +{
> +    ASSERT(rect.right() <= m_layerRenderer->rootLayerTextureSize().width()
> +           && rect.bottom() <= m_layerRenderer->rootLayerTextureSize().height());
> +
> +    void* pixels = 0;
> +    const SkBitmap bitmap = canvas->getDevice()->accessBitmap(false);
> +    if (bitmap.config() == SkBitmap::kARGB_8888_Config) {
> +        IntRect bitmapRect(0, 0, bitmap.width(), bitmap.height());
> +
> +        SkAutoLockPixels bitmapLock(bitmap);
> +

The SkAutoLockPixels should move inside the if (rect==bitmapRect) 

> +        if (rect == bitmapRect) {
> +            pixels = bitmap.getPixels();
> +            m_layerRenderer->getFramebufferPixels(pixels, rect);
> +        } else {
> +            // Create temp bitmap of correct size to copy pixels into.
> +            skia::PlatformCanvas canvasResize;
> +            if (canvasResize.initialize(rect.width(), rect.height(), true)) {
> +                SkBitmap bitmapResize = canvasResize.getDevice()->accessBitmap(false);
> +                pixels = bitmapResize.getPixels();
> +                m_layerRenderer->getFramebufferPixels(pixels, rect);
> +                SkIRect srcRect(rect);

I think at this point srcRect should be (0, 0, rect.width(), rect.height())

> +                SkRect dstRect(bitmapRect);
> +                canvas->drawBitmapRect(bitmapResize, &srcRect, dstRect, 0);
> +            } else {
> +                clearSkBitmap(bitmap);
> +                ASSERT_NOT_REACHED();
> +            }
> +        }
> +    } else {
> +        clearSkBitmap(bitmap);
> +        ASSERT_NOT_REACHED();
> +    }
> +}
> +
> +#elif PLATFORM(CG)
> +static inline void clearCGBitmap(const CGContextRef& bitmap)
> +{
> +    CGContextClearRect(bitmap,
> +                       CGRectMake(0, 0, CGBitmapContextGetWidth(bitmap), CGBitmapContextGetHeight(bitmap)));
> +}
> +
> +void WebViewImpl::doPixelReadbackToCanvas(WebCanvas* canvas, IntRect& rect)
> +{
> +    ASSERT(rect.right() <= m_layerRenderer->rootLayerTextureSize().width()
> +           && rect.bottom() <= m_layerRenderer->rootLayerTextureSize().height());
> +
> +    void* pixels = 0;
> +    CGContextRef bitmap = reinterpret_cast<CGContextRef>(canvas);
> +    IntRect bitmapRect(0, 0, CGBitmapContextGetWidth(bitmap), CGBitmapContextGetHeight(bitmap));
> +    if (CGBitmapContextGetRowBytes(bitmap) == 4 * bitmapRect.width()) {
> +        if (rect == bitmapRect) {
> +          pixels = CGBitmapContextGetData(bitmap);
> +          m_layerRenderer->getFramebufferPixels(pixels, rect);
> +        } else {
> +            // Create temp bitmap of same size as rendered layer to copy pixels into.
> +            CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();
> +            CGContextRef bitmapResize = CGBitmapContextCreate(0, rect.width(), rect.height(),
> +                                                              8, 4 * rect.width(), colorSpace,
> +                                                              kCGImageAlphaPremultipliedLast);
> +            if (bitmapResize) {
> +              pixels = CGBitmapContextGetData(bitmapResize);
> +              m_layerRenderer->getFramebufferPixels(pixels, rect);
> +
> +              // Copy bitmap back to input bitmap. The image is inverted according to CG,
> +              // so set up the appropriate transform to invert vertical axis and move origin
> +              // to bottom left.
> +              CGContextSaveGState(bitmap);
> +              CGContextTranslateCTM(bitmap, 0, bitmapRect.height());
> +              CGContextScaleCTM(bitmap, 1.0, -1.0);
> +              CGContextDrawImage(bitmap, bitmapRect,
> +                                 CGBitmapContextCreateImage(bitmapResize));
> +              CGContextRestoreGState(bitmap);
> +            } else {
> +                clearCGBitmap(bitmap);
> +                ASSERT_NOT_REACHED();
> +            }
> +        }
> +    } else {
> +        clearCGBitmap(bitmap);
> +        ASSERT_NOT_REACHED();
> +    }
> +}
> +#else
> +#error Must port to your platform.
> +#endif
> +
> +#endif
> +
>  // FIXME: m_currentInputEvent should be removed once ChromeClient::show() can
>  // get the current-event information from WebCore.
>  const WebInputEvent* WebViewImpl::m_currentInputEvent = 0;
> diff --git a/WebKit/chromium/src/WebViewImpl.h b/WebKit/chromium/src/WebViewImpl.h
> index c29612123f4514c94dd7d970c73114a4beeec355..67cce0163e0a4f5551a2e608dee26305b5c55766 100644
> --- a/WebKit/chromium/src/WebViewImpl.h
> +++ b/WebKit/chromium/src/WebViewImpl.h
> @@ -45,6 +45,7 @@
>  #include "EditorClientImpl.h"
>  #include "GraphicsLayer.h"
>  #include "InspectorClientImpl.h"
> +#include <IntRect.h>
>  #include "LayerRendererChromium.h"
>  #include "NotificationPresenterImpl.h"
>  #include "SpeechInputClientImpl.h"
> @@ -386,6 +387,7 @@ private:
>  #if USE(ACCELERATED_COMPOSITING)
>      void setIsAcceleratedCompositingActive(bool);
>      void updateRootLayerContents(const WebRect&);
> +    void doPixelReadbackToCanvas(WebCanvas*, WebCore::IntRect&);

const WebCore::IntRect&  ? 

>  #endif
>  
>      WebViewClient* m_client;

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



More information about the webkit-unassigned mailing list