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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 2 21:40:39 PDT 2010


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





--- Comment #45 from Vangelis Kokkevis <vangelis at chromium.org>  2010-09-02 21:40:39 PST ---
(From update of attachment 66402)
>  
> +void LayerRendererChromium::getFramebufferPixels(void *pixels, const IntSize& size)

This method should take an IntRect as it shouldn't always assume that we're reading starting from (0,0)

> +{
> +    ASSERT(size == rootLayerTextureSize());
> +
> +    if (!pixels)
> +        return;
> +
> +    makeContextCurrent();
> +
> +    GLC(glReadPixels(0, 0, size.width(), size.height(),
> +                     GL_RGBA, GL_UNSIGNED_BYTE, pixels));
> +
> +    // Flip pixels vertically.
> +    const int rowBytes = 4 * size.width();
> +    OwnArrayPtr<unsigned char> lineTemp(new unsigned char[rowBytes]);
> +    for (int row1 = 0, row2 = size.height() - 1; row1 < size.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);
> +    }
> +}
> +




> +
> +        // 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.
> +            int xMax = min(rect.x + rect.width - 1, m_layerRenderer->rootLayerTextureSize().width() - 1);
> +            int yMax = min(rect.y + rect.height - 1, m_layerRenderer->rootLayerTextureSize().height() - 1);
> +            WebRect resizeRect(rect.x, rect.y, xMax - rect.x + 1, yMax - rect.y + 1);

A cleaner way to do this would be to create an IntRect out of WebRect and call IntRect::intersect() with the rect of 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, WebRect& rect)
> +{
> +    ASSERT((rect.x + rect.width -1) < m_layerRenderer->rootLayerTextureSize().width()
> +           && (rect.y + rect.height -1) < m_layerRenderer->rootLayerTextureSize().height());

If you continue using IntRect, you can call IntRect::right and IntRect::bottom instead of doing the math. But I think what you really want to 
ASSERT is rect.intersect(rootLayerRect) == rect

> +
> +    void* pixels = 0;
> +    const SkBitmap bitmap = canvas->getDevice()->accessBitmap(false);
> +    if (bitmap.config() == SkBitmap::kARGB_8888_Config) {
> +        WebRect bitmapRect(0, 0, bitmap.width(), bitmap.height());
> +        IntSize size(bitmapRect.width, bitmapRect.height);
> +
> +        SkAutoLockPixels bitmapLock(bitmap);
> +
> +        if (rect == bitmapRect) {
> +            pixels = bitmap.getPixels();
> +            m_layerRenderer->getFramebufferPixels(pixels, size);
> +        } else {
> +            IntSize resize(rect.x + rect.width, rect.y + rect.height);
> +
> +            // Create temp bitmap of correct size to copy pixels into.
> +            skia::PlatformCanvas canvasResize;
> +            if (canvasResize.initialize(resize.width(), resize.height(), true)) {
> +                SkBitmap bitmapResize = canvasResize.getDevice()->accessBitmap(false);
> +                pixels = bitmapResize.getPixels();
> +                m_layerRenderer->getFramebufferPixels(pixels, resize);

It seems like you're always reading from (0,0) up to the width you need.  Ideally you want to read only the rect you were asked to render.  Alternatively, you can eliminate a lot
of the trimming logic and always read the entire backbuffer and then use the srcRect in drawBitmapRect to specify which part you want out of it. I think though that reading only the region you need is more elegant.


> +                SkIRect srcRect;
> +                srcRect.set(rect.x, rect.y, rect.width, rect.height);
> +                SkRect dstRect = SkRect::MakeWH(bitmap.width(), bitmap.height());
> +                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, WebRect& rect)
> +{
> +    ASSERT((rect.x + rect.width -1) < m_layerRenderer->rootLayerTextureSize().width()
> +           && (rect.y + rect.height -1) < m_layerRenderer->rootLayerTextureSize().height());
> +
> +    void* pixels = 0;
> +    CGContextRef bitmap = reinterpret_cast<CGContextRef>(canvas);
> +    WebRect bitmapRect(0, 0, CGBitmapContextGetWidth(bitmap), CGBitmapContextGetHeight(bitmap));
> +    IntSize size(bitmapRect.width, bitmapRect.height);
> +    if (CGBitmapContextGetRowBytes(bitmap) == 4 * bitmapRect.width) {
> +        if (rect == bitmapRect) {
> +          pixels = CGBitmapContextGetData(bitmap);
> +          m_layerRenderer->getFramebufferPixels(pixels, size);
> +        } else {
> +            IntSize resize(rect.x + rect.width, rect.y + rect.height);
> +
> +            // Create temp bitmap of same size as rendered layer to copy pixels into.
> +            CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();
> +            CGContextRef bitmapResize = CGBitmapContextCreate(0, resize.width(), resize.height(),
> +                                                              8, 4 * resize.width(), colorSpace,
> +                                                              kCGImageAlphaPremultipliedLast);
> +            if (bitmapResize) {
> +              pixels = CGBitmapContextGetData(bitmapResize);
> +              m_layerRenderer->getFramebufferPixels(pixels, resize);
> +
> +              // 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,
> +                                 CGRectMake(bitmapRect.x, bitmapRect.y, bitmapRect.width, bitmapRect.height),
> +                                 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..fd75a20117f9ffdb98223c9498d4ee7ae98643fd 100644
> --- a/WebKit/chromium/src/WebViewImpl.h
> +++ b/WebKit/chromium/src/WebViewImpl.h
> @@ -386,6 +386,7 @@ private:
>  #if USE(ACCELERATED_COMPOSITING)
>      void setIsAcceleratedCompositingActive(bool);
>      void updateRootLayerContents(const WebRect&);
> +    void doPixelReadbackToCanvas(WebCanvas*, WebRect&);
>  #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