[Webkit-unassigned] [Bug 45092] [chromium] Accelerated Compositing: screen garbage when scrolling
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Sep 3 00:31:44 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=45092
--- Comment #4 from Vangelis Kokkevis <vangelis at chromium.org> 2010-09-03 00:31:44 PST ---
(From update of attachment 66447)
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index fad10df2a24eb70d8cc2aee9e84b229f9c0041ab..1c487a1123a87d694f63617e87a11569265bd1f2 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,31 @@
> +2010-09-02 Nat Duca <nduca at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + [chromium] Accelerated Compositing: screen garbage when scrolling
> + https://bugs.webkit.org/show_bug.cgi?id=45092
> +
> + Split LayerRenderChromium::drawLayers into several different
> + functions, responsible for preparing the backbuffer, updating the
> + root texture, compositing and performing the final
> + swapbuffers. This are then used by the new
> + WebViewImpl::paintComposited rendering path.
> +
> + * platform/graphics/chromium/LayerChromium.cpp:
> + (WebCore::LayerChromium::setBounds):
> + (WebCore::LayerChromium::setFrame):
> + (WebCore::LayerChromium::setNeedsDisplay):
> + (WebCore::LayerChromium::resetNeedsDisplay):
> + * platform/graphics/chromium/LayerChromium.h:
> + (WebCore::LayerChromium::dirtyRect):
> + * platform/graphics/chromium/LayerRendererChromium.cpp:
> + (WebCore::LayerRendererChromium::setRootLayerCanvasSize):
> + (WebCore::LayerRendererChromium::prepareToDrawLayers):
> + (WebCore::LayerRendererChromium::updateRootLayerTextureRect):
> + (WebCore::LayerRendererChromium::drawLayers):
> + (WebCore::LayerRendererChromium::present):
> + * platform/graphics/chromium/LayerRendererChromium.h:
> +
> 2010-09-02 Eric Seidel <eric at webkit.org>
>
> Reviewed by Dimitri Glazkov.
> diff --git a/WebCore/platform/graphics/chromium/LayerChromium.cpp b/WebCore/platform/graphics/chromium/LayerChromium.cpp
> index 6519f1ff2ba1826f2e285962b0381712a46e9dcc..92ec49b5cc7f7ff8b637b6d30a124a14dc0bc3bb 100644
> --- a/WebCore/platform/graphics/chromium/LayerChromium.cpp
> +++ b/WebCore/platform/graphics/chromium/LayerChromium.cpp
> @@ -299,10 +299,15 @@ void LayerChromium::setBounds(const IntSize& size)
> if (m_bounds == size)
> return;
>
> + bool firstResize = !m_bounds.width() && !m_bounds.height() && size.width() && size.height();
> +
> m_bounds = size;
> m_backingStoreSize = size;
>
> - setNeedsCommit();
> + if (firstResize)
> + setNeedsDisplay(FloatRect(0, 0, m_bounds.width(), m_bounds.height()));
> + else
> + setNeedsCommit();
> }
>
> void LayerChromium::setFrame(const FloatRect& rect)
> @@ -311,7 +316,7 @@ void LayerChromium::setFrame(const FloatRect& rect)
> return;
>
> m_frame = rect;
> - setNeedsCommit();
> + setNeedsDisplay(FloatRect(0, 0, m_bounds.width(), m_bounds.height()));
> }
>
> const LayerChromium* LayerChromium::rootLayer() const
> @@ -353,7 +358,6 @@ void LayerChromium::setNeedsDisplay(const FloatRect& dirtyRect)
> m_contentsDirty = true;
>
> m_dirtyRect.unite(dirtyRect);
> -
> setNeedsCommit();
> }
>
> @@ -363,6 +367,12 @@ void LayerChromium::setNeedsDisplay()
> m_contentsDirty = true;
> }
>
> +void LayerChromium::resetNeedsDisplay()
> +{
> + m_dirtyRect = FloatRect();
> + m_contentsDirty = false;
> +}
> +
> void LayerChromium::toGLMatrix(float* flattened, const TransformationMatrix& m)
> {
> flattened[0] = m.m11();
> diff --git a/WebCore/platform/graphics/chromium/LayerChromium.h b/WebCore/platform/graphics/chromium/LayerChromium.h
> index ba1508844ba35b3451f82cd589a653976521a08c..2d225daf5a2071babfebd6a7cf3cc05e26b39086 100644
> --- a/WebCore/platform/graphics/chromium/LayerChromium.h
> +++ b/WebCore/platform/graphics/chromium/LayerChromium.h
> @@ -112,6 +112,8 @@ public:
>
> void setNeedsDisplay(const FloatRect& dirtyRect);
> void setNeedsDisplay();
> + const FloatRect& dirtyRect() const { return m_dirtyRect; }
> + void resetNeedsDisplay();
>
> void setNeedsDisplayOnBoundsChange(bool needsDisplay) { m_needsDisplayOnBoundsChange = needsDisplay; }
>
> diff --git a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> index 50338d2d5f7324e5a119c509190dfef8698b1769..af60c8edfa77bbdea3edba8e2e5fb3b68b2f7b25 100644
> --- a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> +++ b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> @@ -109,6 +109,7 @@ void LayerRendererChromium::debugGLCall(const char* command, const char* file, i
> // Creates a canvas and an associated graphics context that the root layer will
> // render into.
> void LayerRendererChromium::setRootLayerCanvasSize(const IntSize& size)
> +
Please remove empty line
> {
> if (size == m_rootLayerCanvasSize)
> return;
> @@ -152,8 +153,8 @@ void LayerRendererChromium::useShader(unsigned programId)
>
> // Updates the contents of the root layer texture that fall inside the updateRect
> // and re-composits all sublayers.
> -void LayerRendererChromium::drawLayers(const IntRect& updateRect, const IntRect& visibleRect,
> - const IntRect& contentRect, const IntPoint& scrollPosition)
> +void LayerRendererChromium::prepareToDrawLayers(const IntRect& visibleRect, const IntRect& contentRect,
> + const IntPoint& scrollPosition)
> {
> ASSERT(m_hardwareCompositing);
>
> @@ -180,10 +181,11 @@ void LayerRendererChromium::drawLayers(const IntRect& updateRect, const IntRect&
> // The GL viewport covers the entire visible area, including the scrollbars.
> GLC(glViewport(0, 0, visibleRectWidth, visibleRectHeight));
>
> +
Please remove newly added blank line. Here and below.
> // Bind the common vertex attributes used for drawing all the layers.
> LayerChromium::prepareForDraw(layerSharedValues());
> +
>
> - // FIXME: These calls can be made once, when the compositor context is initialized.
The FIXME comment is still valid
> GLC(glDisable(GL_DEPTH_TEST));
> GLC(glDisable(GL_CULL_FACE));
> GLC(glDepthFunc(GL_LEQUAL));
> @@ -193,14 +195,12 @@ void LayerRendererChromium::drawLayers(const IntRect& updateRect, const IntRect&
> m_scrollPosition = scrollPosition;
>
> IntPoint scrollDelta = toPoint(scrollPosition - m_scrollPosition);
> - // Scroll only when the updateRect contains pixels for the newly uncovered region to avoid flashing.
> - if ((scrollDelta.x() && updateRect.width() >= abs(scrollDelta.x()) && updateRect.height() >= contentRect.height())
> - || (scrollDelta.y() && updateRect.height() >= abs(scrollDelta.y()) && updateRect.width() >= contentRect.width())) {
> + // Scroll the backbuffer
> + if (scrollDelta.x() || scrollDelta.y()) {
> // Scrolling works as follows: We render a quad with the current root layer contents
> // translated by the amount the page has scrolled since the last update and then read the
> // pixels of the content area (visible area excluding the scroll bars) back into the
> - // root layer texture. The newly exposed area is subesquently filled as usual with
> - // the contents of the updateRect.
> + // root layer texture. The newly exposed area will be filled by a subsequent drawLayersIntoRect call
> TransformationMatrix scrolledLayerMatrix;
> #if PLATFORM(SKIA)
> float scaleFactor = 1.0f;
> @@ -230,43 +230,64 @@ void LayerRendererChromium::drawLayers(const IntRect& updateRect, const IntRect&
> // no need to copy framebuffer pixels back into the texture.
> m_scrollPosition = scrollPosition;
> }
> +
> + // Translate all the composited layers by the scroll position.
> + TransformationMatrix matrix;
> + matrix.translate3d(-m_scrollPosition.x(), -m_scrollPosition.y(), 0);
>
> - // FIXME: The following check should go away when the compositor renders independently from its own thread.
> - // Ignore a 1x1 update rect at (0, 0) as that's used a way to kick off a redraw for the compositor.
> - if (!(!updateRect.x() && !updateRect.y() && updateRect.width() == 1 && updateRect.height() == 1)) {
> - // Update the root layer texture.
> - ASSERT((updateRect.x() + updateRect.width() <= m_rootLayerTextureWidth)
> - && (updateRect.y() + updateRect.height() <= m_rootLayerTextureHeight));
> + // Traverse the layer tree and update the layer transforms.
> + float opacity = 1;
> + const Vector<RefPtr<LayerChromium> >& sublayers = m_rootLayer->getSublayers();
> + size_t i;
> + for (i = 0; i < sublayers.size(); i++)
> + updateLayersRecursive(sublayers[i].get(), matrix, opacity);
> +}
>
> +void LayerRendererChromium::updateRootLayerTextureRect(const IntRect& updateRect)
> +{
> + ASSERT(m_hardwareCompositing);
> +
> + if (!m_rootLayer)
> + return;
> +
> + // Update the root layer texture.
> + ASSERT((updateRect.x() + updateRect.width() <= m_rootLayerTextureWidth)
> + && (updateRect.y() + updateRect.height() <= m_rootLayerTextureHeight));
You can use IntRect::right() and IntRect::bottom() to get the coordinates of the extremes of the rect.
> +
> #if PLATFORM(SKIA)
> - // Get the contents of the updated rect.
> - const SkBitmap bitmap = m_rootLayerCanvas->getDevice()->accessBitmap(false);
> - int rootLayerWidth = bitmap.width();
> - int rootLayerHeight = bitmap.height();
> - ASSERT(rootLayerWidth == updateRect.width() && rootLayerHeight == updateRect.height());
I'm not sure I follow here. Is the updateRect supposed to match the root layer size? I thought update rects can be smaller. Shouldn't rootLayerWidth == m_rootLayerTextureWidth? You are asserting a different condition in the lines above.
> - void* pixels = bitmap.getPixels();
> -
> - // Copy the contents of the updated rect to the root layer texture.
> - GLC(glTexSubImage2D(GL_TEXTURE_2D, 0, updateRect.x(), updateRect.y(), updateRect.width(), updateRect.height(), GL_RGBA, GL_UNSIGNED_BYTE, pixels));
Don't you need to bind the root layer texture before calling this?
> + // Get the contents of the updated rect.
> + const SkBitmap bitmap = m_rootLayerCanvas->getDevice()->accessBitmap(false);
> + int rootLayerWidth = bitmap.width();
> + int rootLayerHeight = bitmap.height();
> + ASSERT(rootLayerWidth == updateRect.width() && rootLayerHeight == updateRect.height());
> + void* pixels = bitmap.getPixels();
> + // Copy the contents of the updated rect to the root layer texture.
> + GLC(glTexSubImage2D(GL_TEXTURE_2D, 0, updateRect.x(), updateRect.y(), updateRect.width(), updateRect.height(), GL_RGBA, GL_UNSIGNED_BYTE, pixels));
> #elif PLATFORM(CG)
> - // Get the contents of the updated rect.
> - ASSERT(static_cast<int>(CGBitmapContextGetWidth(m_rootLayerCGContext.get())) == updateRect.width() && static_cast<int>(CGBitmapContextGetHeight(m_rootLayerCGContext.get())) == updateRect.height());
> - void* pixels = m_rootLayerBackingStore.data();
> -
> - // Copy the contents of the updated rect to the root layer texture.
> - // The origin is at the lower left in Core Graphics' coordinate system. We need to correct for this here.
> - GLC(glTexSubImage2D(GL_TEXTURE_2D, 0,
> - updateRect.x(), m_rootLayerTextureHeight - updateRect.y() - updateRect.height(),
> - updateRect.width(), updateRect.height(),
> - GL_RGBA, GL_UNSIGNED_BYTE, pixels));
> + // Get the contents of the updated rect.
> + // ASSERT(static_cast<int>(CGBitmapContextGetWidth(m_rootLayerCGContext.get())) == updateRect.width() && static_cast<int>(CGBitmapContextGetHeight(m_rootLayerCGContext.get())) == updateRect.height());
> + void* pixels = m_rootLayerBackingStore.data();
If this ASSERT is not necessary, please remove rather than commenting out.
> +
> + // Copy the contents of the updated rect to the root layer texture.
> + // The origin is at the lower left in Core Graphics' coordinate system. We need to correct for this here.
> + GLC(glTexSubImage2D(GL_TEXTURE_2D, 0,
> + updateRect.x(), m_rootLayerTextureHeight - updateRect.y() - updateRect.height(),
> + updateRect.width(), updateRect.height(),
> + GL_RGBA, GL_UNSIGNED_BYTE, pixels));
> #else
> #error "Need to implement for your platform."
> #endif
> - }
> +}
>
> +void LayerRendererChromium::drawLayers(const IntRect& visibleRect, const IntRect& contentRect)
> +{
> + ASSERT(m_hardwareCompositing);
> +
> glClearColor(0, 0, 1, 1);
> glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> -
> +
> + GLC(glBindTexture(GL_TEXTURE_2D, m_rootLayerTextureId));
> +
> // Render the root layer using a quad that takes up the entire visible area of the window.
> // We reuse the shader program used by ContentLayerChromium.
> const ContentLayerChromium::SharedValues* contentLayerValues = contentLayerSharedValues();
> @@ -290,22 +311,13 @@ void LayerRendererChromium::drawLayers(const IntRect& updateRect, const IntRect&
> GLC(glEnable(GL_BLEND));
> GLC(glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA));
>
> - // Translate all the composited layers by the scroll position.
> - TransformationMatrix matrix;
> - matrix.translate3d(-m_scrollPosition.x(), -m_scrollPosition.y(), 0);
> -
> - // Traverse the layer tree and update the layer transforms.
> - float opacity = 1;
> - const Vector<RefPtr<LayerChromium> >& sublayers = m_rootLayer->getSublayers();
> - size_t i;
> - for (i = 0; i < sublayers.size(); i++)
> - updateLayersRecursive(sublayers[i].get(), matrix, opacity);
> -
> - m_rootVisibleRect = visibleRect;
> -
> // Enable scissoring to avoid rendering composited layers over the scrollbars.
> GLC(glEnable(GL_SCISSOR_TEST));
> FloatRect scissorRect(contentRect);
> +
> + // Set the rootVisibleRect --- used by subsequent drawLayers calls
> + m_rootVisibleRect = visibleRect;
> +
> // The scissorRect should not include the scroll offset.
> scissorRect.move(-m_scrollPosition.x(), -m_scrollPosition.y());
> scissorToRect(scissorRect);
> @@ -316,13 +328,17 @@ void LayerRendererChromium::drawLayers(const IntRect& updateRect, const IntRect&
> GLC(glStencilMask(0));
>
> // Traverse the layer tree one more time to draw the layers.
> - for (i = 0; i < sublayers.size(); i++)
> + const Vector<RefPtr<LayerChromium> >& sublayers = m_rootLayer->getSublayers();
> + for (size_t i = 0; i < sublayers.size(); i++)
> drawLayersRecursive(sublayers[i].get(), scissorRect);
>
> GLC(glDisable(GL_SCISSOR_TEST));
> +}
>
> +void LayerRendererChromium::present()
> +{
> + // We're done! Time to swapbuffers!
> m_gles2Context->swapBuffers();
> -
> m_needsDisplay = false;
> }
>
> diff --git a/WebCore/platform/graphics/chromium/LayerRendererChromium.h b/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> index 8f44afebd0a2c7b475d04ba48723f1cdc5455091..2f9edef67c233ffb7c02d43517de5b754701b128 100644
> --- a/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> +++ b/WebCore/platform/graphics/chromium/LayerRendererChromium.h
> @@ -61,9 +61,17 @@ public:
> LayerRendererChromium(PassOwnPtr<GLES2Context> gles2Context);
> ~LayerRendererChromium();
>
> - // 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);
> + // updates size of root texture, if needed, and scrolls the backbuffer
> + void prepareToDrawLayers(const IntRect& visibleRect, const IntRect& contentRect, const IntPoint& scrollPosition);
> +
> + // updates a rectangle within the root layer texture
> + void updateRootLayerTextureRect(const IntRect& updateRect);
> +
> + // draws the current layers onto the backbuffer
> + void drawLayers(const IntRect& visibleRect, const IntRect& contentRect);
> +
> + // puts backbufeffer onscreen
Typo: backbuffer
> + void present();
>
> void setRootLayer(PassRefPtr<LayerChromium> layer) { m_rootLayer = layer; }
> LayerChromium* rootLayer() { return m_rootLayer.get(); }
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> index 527e8efccae39045fd985ded3bf20d7166afc69d..59509251a1e367266ed20c7fab2e70aed3457cda 100644
> --- a/WebKit/chromium/ChangeLog
> +++ b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,48 @@
> +2010-09-02 Nat Duca <nduca at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + [chromium] Accelerated Compositing: screen garbage when scrolling
> + https://bugs.webkit.org/show_bug.cgi?id=45092
> +
> + Introduce a new API on WebWidget for painting with accelerated
> + compositing that allows the compositor to properly distingiush
> + scrolling, invalidation and repainting from one another. The key
> + change is that in accelerated rendering case, invalidates and
> + scrolling pass directly to the compositor, rather than passing up
> + to the client as was the case in the software path. For
> + accelerated rendering, the previous paint() method is replaced by
> + a pair of methods: paintComposited and
> + invalidateCompositedRootLayerRect.
> +
> +
> + * public/WebWidget.h:
> + * public/WebWidgetClient.h:
> + (WebKit::WebWidgetClient::scheduleDeferredComposite):
> + * src/ChromeClientImpl.cpp:
> + (WebKit::ChromeClientImpl::invalidateContentsAndWindow):
> + (WebKit::ChromeClientImpl::scroll):
> + * src/WebPopupMenuImpl.cpp:
> + (WebKit::WebPopupMenuImpl::paint):
> + (WebKit::WebPopupMenuImpl::invalidateCompositedRootLayerRect):
> + (WebKit::WebPopupMenuImpl::paintComposited):
> + * src/WebPopupMenuImpl.h:
> + * src/WebViewImpl.cpp:
> + (WebKit::WebViewImpl::resize):
> + (WebKit::WebViewImpl::paint):
> + (WebKit::WebViewImpl::setRootGraphicsLayer):
> + (WebKit::WebViewImpl::setIsAcceleratedCompositingActive):
> + (WebKit::WebViewImpl::updateRootLayerContents):
> + (WebKit::WebViewImpl::setRootLayerNeedsDisplay):
> + (WebKit::WebViewImpl::invalidateRootLayerRect):
> + (WebKit::WebViewImpl::scrollRootLayer):
> + (WebKit::WebViewImpl::invalidateCompositedRootLayerRect):
> + (WebKit::WebViewImpl::paintComposited):
> + * src/WebViewImpl.h:
> + * tests/PopupMenuTest.cpp:
> + (WebKit::TestWebWidget::invalidateCompositedRootLayerRect):
> + (WebKit::TestWebWidget::paintComposited):
> +
> 2010-09-02 Ilya Sherman <isherman at google.com>
>
> Reviewed by Eric Seidel.
> diff --git a/WebKit/chromium/public/WebWidget.h b/WebKit/chromium/public/WebWidget.h
> index 5c9f54e79366b83afee7c0e1e3a3288c34412ca9..da690a63edc3d3ca3a104e9b8774c3a2e23b783a 100644
> --- a/WebKit/chromium/public/WebWidget.h
> +++ b/WebKit/chromium/public/WebWidget.h
> @@ -59,15 +59,28 @@ public:
> // Called to layout the WebWidget. This MUST be called before Paint,
> // and it may result in calls to WebWidgetClient::didInvalidateRect.
> virtual void layout() = 0;
> -
> - // Called to paint the specified region of the WebWidget onto the given
> - // canvas. You MUST call Layout before calling this method. It is
> - // okay to call paint multiple times once layout has been called,
> - // assuming no other changes are made to the WebWidget (e.g., once
> - // events are processed, it should be assumed that another call to
> - // layout is warranted before painting again).
> - virtual void paint(WebCanvas*, const WebRect&) = 0;
> -
> +
> + // Called to paint the specified viewport of the WebWidget
> + // onto the specified canvas at (viewport.x,viewport.y). You MUST call
Does viewport specify the region of the WebWidget that will be grabbed or the place where it's
going to end up on the canvas? The comment seems to indicate both but I'm not sure that's correct.
> + // Layout before calling this method. It is okay to call paint
> + // multiple times once layout has been called, assuming no other
> + // changes are made to the WebWidget (e.g., once events are
> + // processed, it should be assumed that another call to layout is
> + // warranted before painting again).
> + virtual void paint(WebCanvas*, const WebRect& viewport) = 0;
> +
> + // Triggers compositing of the current layers onto the screen.
> + // The finish argument controls whether the compositor will waiting for the
> + // GPU to finish rendering before returning. You MUST call Layout
> + // before calling this method, for the same reasons described in
> + // the paint method above.
> + virtual void composite(bool finish) = 0;
> +
> + // Called to inform the WebWidget of a change in native widget settings.
> + // Implementors that cache rendered copies of widgets need to re-render
> + // on receiving this message
> + virtual void themeChanged() = 0;
> +
> // Called to inform the WebWidget of an input event. Returns true if
> // the event has been processed, false otherwise.
> virtual bool handleInputEvent(const WebInputEvent&) = 0;
> diff --git a/WebKit/chromium/public/WebWidgetClient.h b/WebKit/chromium/public/WebWidgetClient.h
> index bd7bd6a74fcbaf371df3e9a6e410bedbf3592320..b3ee9f56fd1147787b2e46925f0de451d9ac451b 100644
> --- a/WebKit/chromium/public/WebWidgetClient.h
> +++ b/WebKit/chromium/public/WebWidgetClient.h
> @@ -45,7 +45,10 @@ class WebWidgetClient {
> public:
> // Called when a region of the WebWidget needs to be re-painted.
> virtual void didInvalidateRect(const WebRect&) { }
> -
> +
> + // Called when a call to WebWidget::composite is required
> + virtual void scheduleComposite() { }
> +
> // Called when a region of the WebWidget, given by clipRect, should be
> // scrolled by the specified dx and dy amounts.
> virtual void didScrollRect(int dx, int dy, const WebRect& clipRect) { }
> diff --git a/WebKit/chromium/src/ChromeClientImpl.cpp b/WebKit/chromium/src/ChromeClientImpl.cpp
> index e6f14007180d8d3bf6128b130dc28abaaec1f90a..2fda8e68e614ff36817b4c38badfb1d31160248d 100644
> --- a/WebKit/chromium/src/ChromeClientImpl.cpp
> +++ b/WebKit/chromium/src/ChromeClientImpl.cpp
> @@ -492,8 +492,16 @@ void ChromeClientImpl::invalidateContentsAndWindow(const IntRect& updateRect, bo
> {
> if (updateRect.isEmpty())
> return;
> - if (m_webView->client())
> - m_webView->client()->didInvalidateRect(updateRect);
> +#if USE(ACCELERATED_COMPOSITING)
> + if (!m_webView->isAcceleratedCompositingActive()) {
> +#endif
> + if (m_webView->client())
> + m_webView->client()->didInvalidateRect(updateRect);
> +#if USE(ACCELERATED_COMPOSITING)
> + } else {
> + m_webView->invalidateRootLayerRect(updateRect);
> + }
> +#endif
> }
>
> void ChromeClientImpl::invalidateContentsForSlowScroll(const IntRect& updateRect, bool immediate)
> @@ -507,11 +515,19 @@ void ChromeClientImpl::scroll(
> const IntRect& clipRect)
> {
> m_webView->hidePopups();
> - if (m_webView->client()) {
> - int dx = scrollDelta.width();
> - int dy = scrollDelta.height();
> - m_webView->client()->didScrollRect(dx, dy, clipRect);
> +#if USE(ACCELERATED_COMPOSITING)
> + if (!m_webView->isAcceleratedCompositingActive()) {
> +#endif
> + if (m_webView->client()) {
> + int dx = scrollDelta.width();
> + int dy = scrollDelta.height();
> + m_webView->client()->didScrollRect(dx, dy, clipRect);
> + }
> +#if USE(ACCELERATED_COMPOSITING)
> + } else {
> + m_webView->scrollRootLayer(scrollDelta, clipRect);
> }
> +#endif
> }
>
> IntPoint ChromeClientImpl::screenToWindow(const IntPoint&) const
> diff --git a/WebKit/chromium/src/WebPopupMenuImpl.cpp b/WebKit/chromium/src/WebPopupMenuImpl.cpp
> index 75d6cc143824b7f59291029b427ce920c23e4539..717ff82e25e5cf4acd03283683a608ad7847705a 100644
> --- a/WebKit/chromium/src/WebPopupMenuImpl.cpp
> +++ b/WebKit/chromium/src/WebPopupMenuImpl.cpp
> @@ -155,12 +155,12 @@ void WebPopupMenuImpl::layout()
> {
> }
>
> -void WebPopupMenuImpl::paint(WebCanvas* canvas, const WebRect& rect)
> +void WebPopupMenuImpl::paint(WebCanvas* canvas, const WebRect& viewport)
> {
> if (!m_widget)
> return;
>
> - if (!rect.isEmpty()) {
> + if (!viewport.isEmpty()) {
> #if WEBKIT_USING_CG
> GraphicsContext gc(canvas);
> #elif WEBKIT_USING_SKIA
> @@ -170,10 +170,19 @@ void WebPopupMenuImpl::paint(WebCanvas* canvas, const WebRect& rect)
> #else
> notImplemented();
> #endif
> - m_widget->paint(&gc, rect);
> + m_widget->paint(&gc, viewport);
> }
> }
>
> +void WebPopupMenuImpl::themeChanged()
> +{
> + notImplemented();
> +}
> +void WebPopupMenuImpl::composite(bool finish)
> +{
> + notImplemented();
> +}
> +
> bool WebPopupMenuImpl::handleInputEvent(const WebInputEvent& inputEvent)
> {
> if (!m_widget)
> diff --git a/WebKit/chromium/src/WebPopupMenuImpl.h b/WebKit/chromium/src/WebPopupMenuImpl.h
> index edbb4ab50d416b03b7bd1a5fce800556091ea227..221ba038f2c4c736d677fabfa322e051f904264c 100644
> --- a/WebKit/chromium/src/WebPopupMenuImpl.h
> +++ b/WebKit/chromium/src/WebPopupMenuImpl.h
> @@ -63,6 +63,8 @@ public:
> virtual void resize(const WebSize&);
> virtual void layout();
> virtual void paint(WebCanvas* canvas, const WebRect& rect);
> + virtual void themeChanged();
> + virtual void composite(bool finish);
> virtual bool handleInputEvent(const WebInputEvent&);
> virtual void mouseCaptureLost();
> virtual void setFocus(bool enable);
> diff --git a/WebKit/chromium/src/WebViewImpl.cpp b/WebKit/chromium/src/WebViewImpl.cpp
> index 4b129d694b1f112f27b21ae724db67a68a74e02a..8a48442fc9d5d63a8297aead124347b0cee745d5 100644
> --- a/WebKit/chromium/src/WebViewImpl.cpp
> +++ b/WebKit/chromium/src/WebViewImpl.cpp
> @@ -909,7 +909,12 @@ void WebViewImpl::resize(const WebSize& newSize)
>
> if (m_client) {
> WebRect damagedRect(0, 0, m_size.width, m_size.height);
> - m_client->didInvalidateRect(damagedRect);
> + if (isAcceleratedCompositingActive()) {
I don't have the code handy but I seem to remember that isAcceleratedCompositingActive() is defined only if USE(ACCELERATED_COMPOSTING) is.
> +#if USE(ACCELERATED_COMPOSITING)
> + invalidateRootLayerRect(damagedRect);
> +#endif
> + } else
> + m_client->didInvalidateRect(damagedRect);
> }
>
> #if OS(DARWIN)
> @@ -943,33 +948,17 @@ void WebViewImpl::layout()
>
> void WebViewImpl::paint(WebCanvas* canvas, const WebRect& rect)
> {
> -
> + if (isAcceleratedCompositingActive()) {
Same comment as above. Should isAcceleratedCompositingActive stay inside the #if ?
> #if USE(ACCELERATED_COMPOSITING)
> - if (!isAcceleratedCompositingActive()) {
> + invalidateRootLayerRect(rect);
> + doComposite(canvas, false);
> #endif
> + } else {
> + // TODO, if accelerated, redirect to paint composited loop
In WebKit TODO -> FIXME
> WebFrameImpl* webframe = mainFrameImpl();
> if (webframe)
> webframe->paint(canvas, rect);
> -#if USE(ACCELERATED_COMPOSITING)
> - } else {
> - // Draw the contents of the root layer.
> - updateRootLayerContents(rect);
> -
> - WebFrameImpl* webframe = mainFrameImpl();
> - if (!webframe)
> - return;
> - FrameView* view = webframe->frameView();
> - if (!view)
> - return;
> -
> - // The visibleRect includes scrollbars whereas the contentRect doesn't.
> - IntRect visibleRect = view->visibleContentRect(true);
> - IntRect contentRect = view->visibleContentRect(false);
> -
> - // Ask the layer compositor to redraw all the layers.
> - m_layerRenderer->drawLayers(rect, visibleRect, contentRect, IntPoint(view->scrollX(), view->scrollY()));
> }
> -#endif
> }
>
> // FIXME: m_currentInputEvent should be removed once ChromeClient::show() can
> @@ -2105,9 +2094,17 @@ bool WebViewImpl::tabsToLinks() const
> #if USE(ACCELERATED_COMPOSITING)
> void WebViewImpl::setRootGraphicsLayer(WebCore::PlatformLayer* layer)
> {
> + bool wasActive = m_isAcceleratedCompositingActive;
> setIsAcceleratedCompositingActive(layer ? true : false);
> if (m_layerRenderer)
> m_layerRenderer->setRootLayer(layer);
> + if (wasActive != m_isAcceleratedCompositingActive) {
> + WebRect damagedRect(0, 0, m_size.width, m_size.height);
> + if (m_isAcceleratedCompositingActive)
> + invalidateRootLayerRect(damagedRect);
> + else
> + m_client->didInvalidateRect(damagedRect);
> + }
> }
>
> void WebViewImpl::setIsAcceleratedCompositingActive(bool active)
> @@ -2119,10 +2116,6 @@ void WebViewImpl::setIsAcceleratedCompositingActive(bool active)
> m_layerRenderer = LayerRendererChromium::create(getOnscreenGLES2Context());
> if (m_layerRenderer->hardwareCompositing()) {
> m_isAcceleratedCompositingActive = true;
> -
> - // Force a redraw the entire view so that the compositor gets the entire view,
> - // rather than just the currently-dirty subset.
> - m_client->didInvalidateRect(IntRect(0, 0, m_size.width, m_size.height));
> } else {
> m_layerRenderer.clear();
> m_isAcceleratedCompositingActive = false;
> @@ -2138,12 +2131,6 @@ void WebViewImpl::updateRootLayerContents(const WebRect& rect)
> if (!isAcceleratedCompositingActive())
> return;
>
> - // FIXME: The accelerated compositing path invalidates a 1x1 rect at (0, 0)
> - // in order to get the renderer to ask the compositor to redraw. This is only
> - // temporary until we get the compositor to render directly from its own thread.
> - if (!rect.x && !rect.y && rect.width == 1 && rect.height == 1)
> - return;
> -
> WebFrameImpl* webframe = mainFrameImpl();
> if (!webframe)
> return;
> @@ -2195,21 +2182,174 @@ void WebViewImpl::updateRootLayerContents(const WebRect& rect)
>
> void WebViewImpl::setRootLayerNeedsDisplay()
> {
> - // FIXME: For now we're posting a repaint event for the entire page which is an overkill.
> - if (WebFrameImpl* webframe = mainFrameImpl()) {
> - if (FrameView* view = webframe->frameView()) {
> - // FIXME: Temporary hack to invalidate part of the page so that we get called to render
> - // again.
> - IntRect visibleRect = view->visibleContentRect(true);
> - m_client->didInvalidateRect(IntRect(0, 0, 1, 1));
> + m_client->scheduleComposite();
> + if (m_layerRenderer)
> + m_layerRenderer->setNeedsDisplay();
> +}
> +
> +// WebViewImpl method
> +void WebViewImpl::scrollRootLayer(const WebSize& scrollDelta, const WebRect& clipRect)
> +{
> + ASSERT(m_layerRenderer);
> + // Compute the damage rect in viewport space
> + // Should only be scrolling in one direction at a time.
> + ASSERT(!(scrollDelta.width && scrollDelta.height));
> + WebFrameImpl* webframe = mainFrameImpl();
> + if (!webframe)
> + return;
> + FrameView* view = webframe->frameView();
> + if (!view)
> + return;
> +
> + FloatRect damagedRect;
> + // Put the viewportRect in _layer space_ prior to the scroll since the logic
> + // below emits damage in the space "before" scrolling
> + // Using layer space for damage rects allows us to intermix invalidates with scrolls
> + IntRect contentRect = view->visibleContentRect(false);
> + FloatRect viewportRect(contentRect.x(),
> + contentRect.y(),
> + contentRect.width(), contentRect.height());
> +
> + // Compute the region we will expose by scrolling, and paint that into a
> + // shared memory section.
> + if (scrollDelta.width) {
> + float dx = (float)scrollDelta.width;
> + damagedRect.setY(viewportRect.y());
> + damagedRect.setHeight(viewportRect.height());
> + if (dx > 0) {
> + damagedRect.setX(viewportRect.x());
> + damagedRect.setWidth(dx);
> + } else {
Is it not possible that you could have a scrollDelta with non-zero width _and_ height?
> + damagedRect.setX(viewportRect.right() + dx);
> + damagedRect.setWidth(-dx);
> + }
> + } else {
> + float dy = (float)scrollDelta.height;
> + damagedRect.setX(viewportRect.x());
> + damagedRect.setWidth(viewportRect.width());
> + if (dy > 0) {
> + damagedRect.setY(viewportRect.y());
> + damagedRect.setHeight(dy);
> + } else {
> + damagedRect.setY(viewportRect.bottom() + dy);
> + damagedRect.setHeight(-dy);
> }
> }
> +
> + // TODO: add a smarter damage aggregation logic? Right now, LayerChromium does simple union-ing
> + IntRect iDamagedRect(damagedRect);
> + iDamagedRect.unite(m_scrollDamage);
> + m_scrollDamage = iDamagedRect;
> + setRootLayerNeedsDisplay();
> +}
> +void WebViewImpl::invalidateRootLayerRect(const WebRect& rect)
> +{
> + // rect is in viewport space. Convert to layer space
> + // so that invalidations and scroll invalidations play well with one-another
> + ASSERT(m_layerRenderer);
> +
> + WebFrameImpl* webframe = mainFrameImpl();
> + if (!webframe)
> + return;
> + FrameView* view = webframe->frameView();
> + if (!view)
> + return;
>
> - if (m_layerRenderer)
> - m_layerRenderer->setNeedsDisplay();
> + FloatRect lsRect(rect.x + view->scrollX(),
> + rect.y + view->scrollY(),
> + rect.width, rect.height);
> +
> + // TODO: add a smarter damage aggregation logic? Right now, LayerChromium does simple union-ing
> + m_layerRenderer->rootLayer()->setNeedsDisplay(lsRect);
> }
> #endif // USE(ACCELERATED_COMPOSITING)
>
> +// WebWidet method
> +void WebViewImpl::themeChanged()
> +{
> + if (isAcceleratedCompositingActive()) {
> +#if USE(ACCELERATED_COMPOSITING)
> + m_layerRenderer->rootLayer()->setNeedsDisplay();
> + setRootLayerNeedsDisplay();
> +#endif
> + } else {
> + WebRect damagedRect(0, 0, m_size.width, m_size.height);
> + m_client->didInvalidateRect(damagedRect);
> + }
> +}
> +
> +void WebViewImpl::composite(bool finish)
> +{
> +#if USE(ACCELERATED_COMPOSITING)
> + doComposite(0, finish);
> +#endif
> +}
> +#if USE(ACCELERATED_COMPOSITING)
> +void WebViewImpl::doComposite(WebCanvas* canvas, bool finish)
> +{
> + ASSERT(isAcceleratedCompositingActive());
> + WebFrameImpl* webframe = mainFrameImpl();
> + if (!webframe)
> + return;
> + FrameView* view = webframe->frameView();
> + if (!view)
> + return;
> +
> + // The visibleRect includes scrollbars whereas the contentRect doesn't.
> + IntRect visibleRect = view->visibleContentRect(true);
> + IntRect contentRect = view->visibleContentRect(false);
> + IntRect viewport = IntRect(0, 0, m_size.width, m_size.height);
> +
> + // Give the compoisitor a chance to setup/resize the root texture handle and perform scrolling
> + m_layerRenderer->prepareToDrawLayers(visibleRect, contentRect, IntPoint(view->scrollX(), view->scrollY()));
> +
> + // Draw the contents of the root layer
> + Vector<FloatRect> damageRects;
> + damageRects.append(FloatRect(m_scrollDamage.x, m_scrollDamage.y, m_scrollDamage.width, m_scrollDamage.height));
> + damageRects.append(m_layerRenderer->rootLayer()->dirtyRect());
> + for (size_t i = 0; i < damageRects.size(); ++i) {
> + // the damage rects for the root layer is in layer space [e.g. unscrolled.]
> + // convert from layer space to viewport space
> + const FloatRect ssRect = damageRects[i];
> + FloatRect rect(ssRect.x() - view->scrollX(),
> + ssRect.y() - view->scrollY(),
> + ssRect.width(), ssRect.height());
> +
> + // intersect this rectangle with the viewport
> + rect.intersect(viewport);
> +
> +
> + // now render it
> + WebRect wrect(rect.x(), rect.y(), rect.width(), rect.height());
> + if (wrect.width && wrect.height) {
> + updateRootLayerContents(wrect);
> + m_layerRenderer->updateRootLayerTextureRect(wrect);
> + }
> + }
> + m_layerRenderer->rootLayer()->resetNeedsDisplay();
> + m_scrollDamage = WebRect();
> +
> + // Draw the actual layers...
> + m_layerRenderer->drawLayers(visibleRect, contentRect);
> + m_layerRenderer->drawLayers(visibleRect, contentRect);
> +
> + // readback into the canvas, if requested
> + // FIXME Insert wjmaclean's readback code here for webkit bug 44127
> +
> + // finish if requested
> + // FIXME: handle finish flag
> +
> + // we're done... put result onscreen
> + m_layerRenderer->present();
> +
> + // tell the browser that the rendering is done ---
> + // TODO: this is being called too early, we need to defer calling this
> + // until the swapbuffers itself has executed on the GPU.
> + // m_client->onCompositeComplete();
> +}
> +#endif
> +
> +
> PassOwnPtr<GLES2Context> WebViewImpl::getOnscreenGLES2Context()
> {
> return GLES2Context::create(GLES2ContextInternal::create(gles2Context(), false));
> diff --git a/WebKit/chromium/src/WebViewImpl.h b/WebKit/chromium/src/WebViewImpl.h
> index c29612123f4514c94dd7d970c73114a4beeec355..bcd4f46d87f1a5fc3d6fd64f35a92dbdc6960286 100644
> --- a/WebKit/chromium/src/WebViewImpl.h
> +++ b/WebKit/chromium/src/WebViewImpl.h
> @@ -34,6 +34,7 @@
> #include "WebGLES2Context.h"
> #include "WebNavigationPolicy.h"
> #include "WebPoint.h"
> +#include "WebRect.h"
> #include "WebSize.h"
> #include "WebString.h"
> #include "WebView.h"
> @@ -92,6 +93,8 @@ public:
> virtual void resize(const WebSize&);
> virtual void layout();
> virtual void paint(WebCanvas*, const WebRect&);
> + virtual void themeChanged();
> + virtual void composite(bool finish);
> virtual bool handleInputEvent(const WebInputEvent&);
> virtual void mouseCaptureLost();
> virtual void setFocus(bool enable);
> @@ -322,6 +325,9 @@ public:
> }
>
> #if USE(ACCELERATED_COMPOSITING)
> + void doComposite(WebCanvas*, bool finish);
> + void scrollRootLayer(const WebSize& scrollDelta, const WebRect& clipRect);
> + void invalidateRootLayerRect(const WebRect&);
> void setRootLayerNeedsDisplay();
> void setRootGraphicsLayer(WebCore::PlatformLayer*);
> #endif
> @@ -510,6 +516,7 @@ private:
> RefPtr<WebCore::Node> m_mouseCaptureNode;
>
> #if USE(ACCELERATED_COMPOSITING)
> + WebRect m_scrollDamage;
> OwnPtr<WebCore::LayerRendererChromium> m_layerRenderer;
> bool m_isAcceleratedCompositingActive;
> #endif
> diff --git a/WebKit/chromium/tests/PopupMenuTest.cpp b/WebKit/chromium/tests/PopupMenuTest.cpp
> index 50319af23b0a92f44c9158efb3e6461823981d10..aef29cf2d04124b4c87ef07a2e5f385096782b68 100644
> --- a/WebKit/chromium/tests/PopupMenuTest.cpp
> +++ b/WebKit/chromium/tests/PopupMenuTest.cpp
> @@ -128,6 +128,8 @@ public:
> virtual void resize(const WebSize&) { }
> virtual void layout() { }
> virtual void paint(WebCanvas*, const WebRect&) { }
> + virtual void themeChanged() { }
> + virtual void composite(bool finish) { }
> virtual bool handleInputEvent(const WebInputEvent&) { return true; }
> virtual void mouseCaptureLost() { }
> virtual void setFocus(bool) { }
--
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