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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 25 14:23:33 PDT 2010


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





--- Comment #17 from Vangelis Kokkevis <vangelis at chromium.org>  2010-08-25 14:23:33 PST ---
(From update of attachment 65465)
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index eb5910eda9a40bf990446c741d3d1a25b1621ae7..58447952d5a92685ee5ff989361a79e6b3cedbd2 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,23 @@
> +2010-08-25  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::getRootLayerTextureWidth):
> +        (WebCore::LayerRendererChromium::getRootLayerTextureHeight):
> +
>  2010-08-25  Ilya Tikhonovsky  <loislo at chromium.org>
>  
>          Reviewed by Yury Semikhatsky.
> diff --git a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> index cf238718b21bb40bb6976627d9f233854e4469fb..bacc852b0db1cdc3e8fa2e27e722a8cd5f4daf32 100644
> --- a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> +++ b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> @@ -310,11 +310,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 int width, const int height, const int rowBytes)
> +{
> +    ASSERT(width == m_rootLayerTextureWidth && height == m_rootLayerTextureHeight);
> +
> +    if (!pixels)
> +        return;
> +
> +    makeContextCurrent();
> +
> +    GLC(glReadPixels(0, 0, width, height,
> +                     GL_RGBA, GL_UNSIGNED_BYTE, pixels));
> +
> +    // Flip pixels vertically.
> +    OwnPtr<unsigned char> lineTemp(new unsigned char[rowBytes]);
> +    for (int row1 = 0, row2 = height - 1; row1 < 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 24bbe652828c1f98a531e2128f6918d1e9e8c363..b5c93871b09f7557f015f2f755e86e11d585505b 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,10 @@ public:
>      const ContentLayerChromium::SharedValues* contentLayerSharedValues() const { return m_contentLayerSharedValues.get(); }
>      const CanvasLayerChromium::SharedValues* canvasLayerSharedValues() const { return m_canvasLayerSharedValues.get(); }
>  
> +    int getRootLayerTextureWidth() const { return m_rootLayerTextureWidth; }
> +    int getRootLayerTextureHeight() const { return m_rootLayerTextureHeight; }
> +    void getFramebufferPixels(void *pixels, const int width, const int height, const int rowBytes);
> +
>  private:
>      void updateLayersRecursive(LayerChromium* layer, const TransformationMatrix& parentMatrix, float opacity);
>  
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> index 5d860ed71441c401246d8dfc17723e5ebe9bfa41..6c6942c16cdb56a607b1fb424e900864e8aa92bc 100644
> --- a/WebKit/chromium/ChangeLog
> +++ b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,18 @@
> +2010-08-25  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.
> +
> +        * src/WebViewImpl.cpp:
> +        (WebKit::WebViewImpl::paint):
> +
>  2010-08-25  Satish Sampath  <satish at chromium.org>
>  
>          Reviewed by Jeremy Orlow.
> diff --git a/WebKit/chromium/src/WebViewImpl.cpp b/WebKit/chromium/src/WebViewImpl.cpp
> index 383b7167314edaf342daeb48f3b56610ed753148..4bd56c09910108fb626c068845c8add40249cf87 100644
> --- a/WebKit/chromium/src/WebViewImpl.cpp
> +++ b/WebKit/chromium/src/WebViewImpl.cpp
> @@ -115,6 +115,10 @@
>  #include "WebViewClient.h"
>  #include "wtf/OwnPtr.h"
>  
> +#if WEBKIT_USING_CG
> +#include <CoreGraphics/CGContext.h>
> +#endif
> +
>  #if OS(WINDOWS)
>  #include "RenderThemeChromiumWin.h"
>  #else
> @@ -955,6 +959,7 @@ void WebViewImpl::paint(WebCanvas* canvas, const WebRect& rect)
>              webframe->paint(canvas, rect);
>  #if USE(ACCELERATED_COMPOSITING)
>      } else {
> +
>          // Draw the contents of the root layer.
>          updateRootLayerContents(rect);
>  
> @@ -971,6 +976,78 @@ 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) {
> +            void* pixels = 0;
> +#if WEBKIT_USING_SKIA
> +            const SkBitmap bitmap = canvas->getDevice()->accessBitmap(false);
> +
> +            int width = bitmap.width();
> +            int height = bitmap.height();
> +            int rowBytes = bitmap.rowBytes();
> +
> +            SkAutoLockPixels bitmapLock(bitmap);
> +
> +            if (m_layerRenderer->getRootLayerTextureWidth() == width
> +                && m_layerRenderer->getRootLayerTextureHeight() == height) {
> +                pixels = bitmap.getPixels();
> +                m_layerRenderer->getFramebufferPixels(pixels, width, height, rowBytes);
> +            } else {
> +                width = m_layerRenderer->getRootLayerTextureWidth();
> +                height = m_layerRenderer->getRootLayerTextureHeight();
> +
> +                // Create temp bitmap of correct size to copy pixels into.
> +                OwnPtr<skia::PlatformCanvas> canvas2 = OwnPtr<skia::PlatformCanvas>(new skia::PlatformCanvas());
> +                if (canvas2.get() && canvas2->initialize(width, height, true)) {
> +                    SkBitmap bitmap2 = canvas2->getDevice()->accessBitmap(false);
> +                    pixels = bitmap2.getPixels();
> +                    m_layerRenderer->getFramebufferPixels(pixels, width, height, rowBytes);
> +                    canvas->drawBitmap(bitmap2, 0, 0, 0);
> +                }
> +            }
> +#elif WEBKIT_USING_CG
> +            CGContextRef bitmap = reinterpret_cast<CGContextRef>(canvas);
> +
> +            int width = CGBitmapContextGetWidth(bitmap);
> +            int height = CGBitmapContextGetHeight(bitmap);
> +            int rowBytes = CGBitmapContextGetBytesPerRow(bitmap);
> +
> +            if (m_layerRenderer->getRootLayerTextureWidth() == width
> +                && m_layerRenderer->getRootLayerTextureHeight() == height) {
> +              pixels = CGBitmapContextGetData(bitmap);
> +              m_layerRenderer->getFramebufferPixels(pixels, width, height, rowBytes);
> +            } else {
> +                width = m_layerRenderer->getRootLayerTextureWidth();
> +                height = m_layerRenderer->getRootLayerTextureHeight();
> +
> +                // Create temp bitmap of same size as rendered layer to copy pixels into.
> +                CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();
> +                CGContextRef bitmap2 = CGBitmapContextCreate(0, width, height, 8, 4 * width, colorSpace, 
> +                                                             kCGImageAlphaPremultipliedLast);
> +                if (bitmap2) {
> +                  pixels = CGBitmapContextGetData(bitmap2);
> +                  m_layerRenderer->getFramebufferPixels(pixels, width, height, rowBytes);
> +
> +                  // 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, CGBitmapContextGetHeight(bitmap));
> +                  CGContextScaleCTM(bitmap, 1.0, -1.0);
> +                  CGContextDrawImage(bitmap,
> +                                     CGRectMake(0, 0, CGBitmapContextGetWidth(bitmap), CGBitmapContextGetHeight(bitmap)),
> +                                     CGBitmapContextCreateImage(bitmap2));
> +                  CGContextRestoreGState(bitmap);
> +                }
> +            }
> +#else
> +#error Must port to your platform.
> +#endif
> +        }
> +
> +        m_layerRenderer->present(); // Do final display by swapping buffers.
>      }
>  #endif
>  }

WebCore/platform/graphics/chromium/LayerRendererChromium.h:94
 +      int getRootLayerTextureWidth() const { return m_rootLayerTextureWidth; }
Accessors don't typically start with get.  These two should be: rootLayerTextureWidth() and rootLayerTextureHeight()

WebKit/chromium/src/WebViewImpl.cpp:118
 +  #if WEBKIT_USING_CG
The WebCore code has been using
#if PLATFORM(CG) instead of WEBKIT_USING_CG

WebKit/chromium/src/WebViewImpl.cpp:984
 +  #if WEBKIT_USING_SKIA
#if PLATFORM(SKIA)

WebKit/chromium/src/WebViewImpl.cpp:989
 +              int rowBytes = bitmap.rowBytes();
In getFramebufferPixels() we assume that we have 4 bytes per pixel (RGBA).  If rowBytes doesn't match 4*width then we're in trouble.  Maybe better here to do an:

ASSERT(bitmap.config() == SkBitmap::kARGB_8888_Config)

and not pass the rowBytes down to getFrameBufferPixels.

WebKit/chromium/src/WebViewImpl.cpp:1002
 +                  OwnPtr<skia::PlatformCanvas> canvas2 = OwnPtr<skia::PlatformCanvas>(new skia::PlatformCanvas());
Please use a more descriptive name for canvas2 and bitmap2

WebKit/chromium/src/WebViewImpl.cpp:1016
 +  
ASSERT(rowBytes == width * 4) 

WebKit/chromium/src/WebViewImpl.cpp:1007
 +                      canvas->drawBitmap(bitmap2, 0, 0, 0);
I think you want to call drawBitmapRect here to specify that the target size if the size of your WebCanvas so that you get the scaling.

WebKit/chromium/src/WebViewImpl.cpp:999
 +                  height = m_layerRenderer->getRootLayerTextureHeight();
these variables shadow the width and height defined in the outside scope and make things confusing (plus I think you need the canvas width further down anyway).  Please rename.

WebKit/chromium/src/WebViewImpl.cpp:1010
 +  #elif WEBKIT_USING_CG
#if PLATFORM(CG)

-- 
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