[Webkit-unassigned] [Bug 44127] [chromium] Thumbnails not generated for GPU Rendered Pages
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Aug 17 15:51:46 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=44127
--- Comment #5 from Vangelis Kokkevis <vangelis at chromium.org> 2010-08-17 15:51:45 PST ---
(From update of attachment 64628)
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index ad3ea90..6854b00 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,16 @@
> +2010-08-17 W. James MacLean <wjmaclean at google.com>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + [chromium] Thumbnails not generated for GPU Rendered Pages
> + https://bugs.webkit.org/show_bug.cgi?id=44127
> +
> + Added functions to allow readback of GPU buffer for generating Chromium thumbnails.
> +
> + * platform/graphics/chromium/LayerRendererChromium.cpp:
> + (WebCore::LayerRendererChromium::readPixels):
> + * platform/graphics/chromium/LayerRendererChromium.h:
> +
> 2010-08-17 Darin Fisher <darin at chromium.org>
>
> Reviewed by Darin Adler.
> diff --git a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> index 2f70efa..b73ae86 100644
> --- a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> +++ b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> @@ -506,6 +506,36 @@ void LayerRendererChromium::drawLayers(const IntRect& updateRect, const IntRect&
> m_needsDisplay = false;
> }
>
> +void LayerRendererChromium::readPixels(skia::PlatformCanvas* canvasPtr)
> +{
> + if (!canvasPtr)
> + return;
> +
> + makeContextCurrent();
> +
> + checkGLError();
> +
> + const SkBitmap bitmap = canvasPtr->getDevice()->accessBitmap(false);
> + void* pixels = bitmap.getPixels();
> +
> + glReadPixels(0, 0, bitmap.width(), bitmap.height(),
> + GL_RGBA, GL_UNSIGNED_BYTE, pixels);
> +
> + checkGLError();
> +
> + // Flip pixels vertically ...
> + OwnPtr<unsigned char> lineTemp(new unsigned char[bitmap.rowBytes()]);
> + for (int row1 = 0, row2 = bitmap.height() - 1; row1 < bitmap.height() / 2; ++row1, --row2){
> + unsigned char *ptr1 = static_cast<unsigned char *>(pixels) + row1 * bitmap.rowBytes();
> + unsigned char *ptr2 = static_cast<unsigned char *>(pixels) + row2 * bitmap.rowBytes();
> +
> + memcpy(lineTemp.get(), ptr1, bitmap.rowBytes());
> + memcpy(ptr1, ptr2, bitmap.rowBytes());
> + memcpy(ptr2, lineTemp.get(), bitmap.rowBytes());
> + }
> +}
> +
> +
> // Returns the id of the texture currently associated with the layer or
> // -1 if the id hasn't been registered yet.
> int LayerRendererChromium::getTextureId(LayerChromium* layer)
> diff --git a/WebCore/platform/graphics/chromium/LayerRendererChromium.h b/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> index e4474b5..0a20ca3 100644
> --- a/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> +++ b/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> @@ -87,6 +87,8 @@ public:
>
> GraphicsContext* rootLayerGraphicsContext() const { return m_rootLayerGraphicsContext.get(); }
>
> + void readPixels(skia::PlatformCanvas* canvasPtr);
> +
> private:
> enum ShaderProgramType { DebugBorderProgram, ScrollLayerProgram, ContentLayerProgram, CanvasLayerProgram, NumShaderProgramTypes };
>
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> index c5b2a5d..c8c919b 100644
> --- a/WebKit/chromium/ChangeLog
> +++ b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,18 @@
> +2010-08-17 W. James MacLean <wjmaclean at google.com>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + [chromium] Thumbnails not generated for GPU Rendered Pages
> + https://bugs.webkit.org/show_bug.cgi?id=44127
> +
> + Added functions to allow readback of GPU buffer for generating Chromium thumbnails.
> +
> + * public/WebView.h:
> + (WebKit::WebView::readPixels):
> + * src/WebViewImpl.cpp:
> + (WebKit::WebViewImpl::readPixels):
> + * src/WebViewImpl.h:
> +
> 2010-08-17 Sheriff Bot <webkit.review.bot at gmail.com>
>
> Unreviewed, rolling out r65516.
> diff --git a/WebKit/chromium/public/WebView.h b/WebKit/chromium/public/WebView.h
> index 1b94da2..3433335 100644
> --- a/WebKit/chromium/public/WebView.h
> +++ b/WebKit/chromium/public/WebView.h
> @@ -212,6 +212,8 @@ public:
> // (accept false) effect. Return true on success.
> virtual bool setDropEffect(bool accept) = 0;
>
> + virtual void readPixels(WebCanvas *canvas) {}
> +
>
> // Support for resource loading initiated by plugins -------------------
>
> diff --git a/WebKit/chromium/src/WebViewImpl.cpp b/WebKit/chromium/src/WebViewImpl.cpp
> index 62b20d5..ff00c9a 100644
> --- a/WebKit/chromium/src/WebViewImpl.cpp
> +++ b/WebKit/chromium/src/WebViewImpl.cpp
> @@ -943,6 +943,12 @@ void WebViewImpl::layout()
> }
> }
>
> +void WebViewImpl::readPixels(WebCanvas *canvas)
> +{
> + ASSERT(isAcceleratedCompositingActive());
> + m_layerRenderer->readPixels(canvas);
> +}
> +
> void WebViewImpl::paint(WebCanvas* canvas, const WebRect& rect)
> {
>
> diff --git a/WebKit/chromium/src/WebViewImpl.h b/WebKit/chromium/src/WebViewImpl.h
> index c296121..9b0906c 100644
> --- a/WebKit/chromium/src/WebViewImpl.h
> +++ b/WebKit/chromium/src/WebViewImpl.h
> @@ -158,6 +158,7 @@ public:
> const WebPoint& screenPoint);
> virtual int dragIdentity();
> virtual bool setDropEffect(bool accept);
> + virtual void readPixels(WebCanvas *canvas);
> virtual unsigned long createUniqueIdentifierForRequest();
> virtual void inspectElementAt(const WebPoint& point);
> virtual WebString inspectorSettings() const;
WebCore/ChangeLog:8
+ Added functions to allow readback of GPU buffer for generating Chromium thumbnails.
Usually you would put the above line at the top of the changelog (following [chromium]) and then just provide a link to the bug. No need to repeat the bug description here.
WebKit/chromium/ChangeLog:8
+ Added functions to allow readback of GPU buffer for generating Chromium thumbnails.
In this changelog entry you can be more specific about the changes you made to the WebKit/chromium side, for example something like "Adding a method to WebViewImpl to read the contents of a composited page"
WebKit/chromium/public/WebView.h:215
+ virtual void readPixels(WebCanvas *canvas) {}
Does this method need to appear in WebView or just WebViewImpl? Generally the WebView API is a pure virtual one.
WebKit/chromium/src/WebViewImpl.cpp:948
+ ASSERT(isAcceleratedCompositingActive());
Is this method only called if acceleratedCompositing is active? It seems that this ASSERT() should be converted to an if () and have a different path if you're not using accelerated compositing. In either case though, the code needs to be withing a:
#if USE(ACCELERATED_COMPOSITING)
block
WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:509
+ void LayerRendererChromium::readPixels(skia::PlatformCanvas* canvasPtr)
Skia is only used on windows and linux. We'll need a CG path for the mac. A good example of similar code is in: GraphicsContext3DInternal::paintRenderingResultsToCanvas()
WebCore/platform/graphics/chromium/LayerRendererChromium.h:90
+ void readPixels(skia::PlatformCanvas* canvasPtr);
nit: Use a more descriptive name for this method like: getFramebufferPixels() or something of that sort?
--
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