Changeset 151623 in webkit


Ignore:
Timestamp:
Jun 16, 2013 10:24:40 AM (11 years ago)
Author:
Simon Fraser
Message:

Source/WebCore: Painting of fixed background images is wrong in composited layers
​https://bugs.webkit.org/show_bug.cgi?id=65793

Reviewed by Sam Weinig.

The code that computed background image geometry for background-attachment:fixed
images was unaware of compositing, so often painting the image at the wrong location.

Fix by having RenderBoxModelObject::calculateBackgroundImageGeometry() do the correct
math for fixed backgrounds in composited layer by offsetting the viewport rect by
the paint container's absolute position.

Tests: compositing/backgrounds/fixed-background-on-descendant.html

compositing/backgrounds/fixed-backgrounds.html

  • rendering/RenderBox.cpp:

(WebCore::RenderBox::getBackgroundPaintedExtent): Now returns a bool indicating
whether it is returning a reliable extent rect. It can return false in the case where
a background is fixed, since computing the correct extent would require finding
the appropriate composited ancestor to pass to calculateBackgroundImageGeometry().
This is OK since this function is used for "background opaque" optimizations.
(WebCore::RenderBox::computeBackgroundIsKnownToBeObscured): If getBackgroundPaintedExtent()
returns false, return false.
(WebCore::RenderBox::maskClipRect): We removed mask-attachment, so we never need to
compute the composited ancestor here and can pass null.
(WebCore::RenderBox::repaintLayerRectsForImage): Unwrap a comment.
If the changed image is related to a fixed background, geometry.hasNonLocalGeometry()
will be true. In that cause, just repaint the entire renderer rather than groveling
around for a composited ancestor.

  • rendering/RenderBox.h: Changed name and signature of backgroundPaintedExtent.
  • rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::paintFillLayerExtended): calculateBackgroundImageGeometry()
now needs to know the painting container.
(WebCore::RenderBoxModelObject::calculateBackgroundImageGeometry): Now takes a painting
container, that is required to correctly compute the viewport-relative offset for fixed
backgrounds. geometry.setHasNonLocalGeometry() is set for fixed backgrounds to indicate
to callers that, if they didn't pass a paint container, the destRect is not accurate.
The main bug fix is also here: we move the viewportRect by the absolute location of
paint container, which is equivalent to the composited layer offset.
(WebCore::RenderBoxModelObject::getGeometryForBackgroundImage): calculateBackgroundImageGeometry()
takes a paint container.

  • rendering/RenderBoxModelObject.h:

(WebCore::RenderBoxModelObject::BackgroundImageGeometry::BackgroundImageGeometry):
(WebCore::RenderBoxModelObject::BackgroundImageGeometry::setHasNonLocalGeometry):
(WebCore::RenderBoxModelObject::BackgroundImageGeometry::hasNonLocalGeometry):

  • rendering/RenderImage.cpp:

(WebCore::RenderImage::computeBackgroundIsKnownToBeObscured): If getBackgroundPaintedExtent()
can't cheaply give an accurate answer, return false.

  • rendering/RenderLayerBacking.cpp:

(WebCore::RenderLayerBacking::updateDirectlyCompositedBackgroundImage): Pass the paint container,
which is our own renderer.

LayoutTests: Fixed background images behave strangely with webkit transitions
​https://bugs.webkit.org/show_bug.cgi?id=65793

Reviewed by Sam Weinig.

Ref tests that compare fixed background rendering after a scroll, with and
without compositing, with a couple of layer configurations.

  • compositing/backgrounds/fixed-background-on-descendant-expected.html: Added.
  • compositing/backgrounds/fixed-background-on-descendant.html: Added.
  • compositing/backgrounds/fixed-backgrounds-expected.html: Added.
  • compositing/backgrounds/fixed-backgrounds.html: Added.
Location:
trunk
Files:
5 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r151622 r151623  
     12013-06-15  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Fixed background images behave strangely with webkit transitions
     4        https://bugs.webkit.org/show_bug.cgi?id=65793
     5
     6        Reviewed by Sam Weinig.
     7       
     8        Ref tests that compare fixed background rendering after a scroll, with and
     9        without compositing, with a couple of layer configurations.
     10
     11        * compositing/backgrounds/fixed-background-on-descendant-expected.html: Added.
     12        * compositing/backgrounds/fixed-background-on-descendant.html: Added.
     13        * compositing/backgrounds/fixed-backgrounds-expected.html: Added.
     14        * compositing/backgrounds/fixed-backgrounds.html: Added.
     15
    1162013-06-15  Simon Fraser  <simon.fraser@apple.com>
    217

  • trunk/Source/WebCore/ChangeLog

    r151622 r151623  
     12013-06-15  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Painting of fixed background images is wrong in composited layers
     4        https://bugs.webkit.org/show_bug.cgi?id=65793
     5
     6        Reviewed by Sam Weinig.
     7       
     8        The code that computed background image geometry for background-attachment:fixed
     9        images was unaware of compositing, so often painting the image at the wrong location.
     10       
     11        Fix by having RenderBoxModelObject::calculateBackgroundImageGeometry() do the correct
     12        math for fixed backgrounds in composited layer by offsetting the viewport rect by
     13        the paint container's absolute position.
     14
     15        Tests: compositing/backgrounds/fixed-background-on-descendant.html
     16               compositing/backgrounds/fixed-backgrounds.html
     17
     18        * rendering/RenderBox.cpp:
     19        (WebCore::RenderBox::getBackgroundPaintedExtent): Now returns a bool indicating
     20        whether it is returning a reliable extent rect. It can return false in the case where
     21        a background is fixed, since computing the correct extent would require finding
     22        the appropriate composited ancestor to pass to calculateBackgroundImageGeometry().
     23        This is OK since this function is used for "background opaque" optimizations.
     24        (WebCore::RenderBox::computeBackgroundIsKnownToBeObscured): If getBackgroundPaintedExtent()
     25        returns false, return false.
     26        (WebCore::RenderBox::maskClipRect): We removed mask-attachment, so we never need to
     27        compute the composited ancestor here and can pass null.
     28        (WebCore::RenderBox::repaintLayerRectsForImage): Unwrap a comment.
     29        If the changed image is related to a fixed background, geometry.hasNonLocalGeometry()
     30        will be true. In that cause, just repaint the entire renderer rather than groveling
     31        around for a composited ancestor.
     32        * rendering/RenderBox.h: Changed name and signature of backgroundPaintedExtent.
     33        * rendering/RenderBoxModelObject.cpp:
     34        (WebCore::RenderBoxModelObject::paintFillLayerExtended): calculateBackgroundImageGeometry()
     35        now needs to know the painting container.
     36        (WebCore::RenderBoxModelObject::calculateBackgroundImageGeometry): Now takes a painting
     37        container, that is required to correctly compute the viewport-relative offset for fixed
     38        backgrounds. geometry.setHasNonLocalGeometry() is set for fixed backgrounds to indicate
     39        to callers that, if they didn't pass a paint container, the destRect is not accurate.
     40        The main bug fix is also here: we move the viewportRect by the absolute location of
     41        paint container, which is equivalent to the composited layer offset.
     42        (WebCore::RenderBoxModelObject::getGeometryForBackgroundImage): calculateBackgroundImageGeometry()
     43        takes a paint container.
     44        * rendering/RenderBoxModelObject.h:
     45        (WebCore::RenderBoxModelObject::BackgroundImageGeometry::BackgroundImageGeometry):
     46        (WebCore::RenderBoxModelObject::BackgroundImageGeometry::setHasNonLocalGeometry):
     47        (WebCore::RenderBoxModelObject::BackgroundImageGeometry::hasNonLocalGeometry):
     48        * rendering/RenderImage.cpp:
     49        (WebCore::RenderImage::computeBackgroundIsKnownToBeObscured): If getBackgroundPaintedExtent()
     50        can't cheaply give an accurate answer, return false.
     51        * rendering/RenderLayerBacking.cpp:
     52        (WebCore::RenderLayerBacking::updateDirectlyCompositedBackgroundImage): Pass the paint container,
     53        which is our own renderer.
     54
    1552013-06-15  Simon Fraser  <simon.fraser@apple.com>
    256

  • trunk/Source/WebCore/rendering/RenderBox.cpp

    r151549 r151623  
    11411141}
    11421142
    1143 LayoutRect RenderBox::backgroundPaintedExtent() const
     1143bool RenderBox::getBackgroundPaintedExtent(LayoutRect& paintedExtent) const
    11441144{
    11451145    ASSERT(hasBackground());
    … …  
    11471147
    11481148    Color backgroundColor = style()->visitedDependentColor(CSSPropertyBackgroundColor);
    1149     if (backgroundColor.isValid() && backgroundColor.alpha())
    1150         return backgroundRect;
    1151     if (!style()->backgroundLayers()->image() || style()->backgroundLayers()->next())
    1152         return backgroundRect;
     1149    if (backgroundColor.isValid() && backgroundColor.alpha()) {
     1150        paintedExtent = backgroundRect;
     1151        return true;
     1152    }
     1153
     1154    if (!style()->backgroundLayers()->image() || style()->backgroundLayers()->next()) {
     1155        paintedExtent =  backgroundRect;
     1156        return true;
     1157    }
     1158
    11531159    BackgroundImageGeometry geometry;
    1154     const_cast<RenderBox*>(this)->calculateBackgroundImageGeometry(style()->backgroundLayers(), backgroundRect, geometry);
    1155     return geometry.destRect();
     1160    calculateBackgroundImageGeometry(0, style()->backgroundLayers(), backgroundRect, geometry);
     1161    paintedExtent = geometry.destRect();
     1162    return !geometry.hasNonLocalGeometry();
    11561163}
    11571164
    … …  
    12601267        return false;
    12611268
    1262     LayoutRect backgroundRect = backgroundPaintedExtent();
     1269    LayoutRect backgroundRect;
     1270    if (!getBackgroundPaintedExtent(backgroundRect))
     1271        return false;
    12631272    return foregroundIsKnownToBeOpaqueInRect(backgroundRect, backgroundObscurationTestMaxDepth);
    12641273}
    … …  
    13481357        if (maskLayer->image()) {
    13491358            BackgroundImageGeometry geometry;
    1350             calculateBackgroundImageGeometry(maskLayer, borderBox, geometry);
     1359            // Masks should never have fixed attachment, so it's OK for paintContainer to be null.
     1360            calculateBackgroundImageGeometry(0, maskLayer, borderBox, geometry);
    13511361            result.unite(geometry.destRect());
    13521362        }
    … …  
    14461456    for (const FillLayer* curLayer = layers; curLayer; curLayer = curLayer->next()) {
    14471457        if (curLayer->image() && image == curLayer->image()->data() && curLayer->image()->canRender(this, style()->effectiveZoom())) {
    1448             // Now that we know this image is being used, compute the renderer and the rect
    1449             // if we haven't already
     1458            // Now that we know this image is being used, compute the renderer and the rect if we haven't already.
    14501459            if (!layerRenderer) {
    14511460                bool drawingRootBackground = drawingBackground && (isRoot() || (isBody() && !document()->documentElement()->renderer()->hasBackground()));
    … …  
    14741483
    14751484            BackgroundImageGeometry geometry;
    1476             layerRenderer->calculateBackgroundImageGeometry(curLayer, rendererRect, geometry);
     1485            layerRenderer->calculateBackgroundImageGeometry(0, curLayer, rendererRect, geometry);
     1486            if (geometry.hasNonLocalGeometry()) {
     1487                // Rather than incur the costs of computing the paintContainer for renderers with fixed backgrounds
     1488                // in order to get the right destRect, just repaint the entire renderer.
     1489                layerRenderer->repaint();
     1490                return true;
     1491            }
     1492           
    14771493            layerRenderer->repaintRectangle(geometry.destRect());
    14781494            if (geometry.destRect() == rendererRect)

  • trunk/Source/WebCore/rendering/RenderBox.h

    r151451 r151623  
    589589    virtual void updateFromStyle() OVERRIDE;
    590590
    591     LayoutRect backgroundPaintedExtent() const;
     591    // Returns false if it could not cheaply compute the extent (e.g. fixed background), in which case the returned rect may be incorrect.
     592    bool getBackgroundPaintedExtent(LayoutRect&) const;
    592593    virtual bool foregroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect, unsigned maxDepthToTest) const;
    593594    virtual bool computeBackgroundIsKnownToBeObscured() OVERRIDE;

  • trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

    r151402 r151623  
    977977    if (shouldPaintBackgroundImage) {
    978978        BackgroundImageGeometry geometry;
    979         calculateBackgroundImageGeometry(bgLayer, scrolledPaintRect, geometry, backgroundObject);
     979        calculateBackgroundImageGeometry(paintInfo.paintContainer, bgLayer, scrolledPaintRect, geometry, backgroundObject);
    980980        geometry.clip(paintInfo.rect);
    981981        if (!geometry.destRect().isEmpty()) {
    … …  
    12151215}
    12161216
    1217 void RenderBoxModelObject::calculateBackgroundImageGeometry(const FillLayer* fillLayer, const LayoutRect& paintRect,
    1218     BackgroundImageGeometry& geometry, RenderObject* backgroundObject)
     1217void RenderBoxModelObject::calculateBackgroundImageGeometry(const RenderLayerModelObject* paintContainer, const FillLayer* fillLayer, const LayoutRect& paintRect,
     1218    BackgroundImageGeometry& geometry, RenderObject* backgroundObject) const
    12191219{
    12201220    LayoutUnit left = 0;
    … …  
    12251225    // Determine the background positioning area and set destRect to the background painting area.
    12261226    // destRect will be adjusted later if the background is non-repeating.
     1227    // FIXME: transforms spec says that fixed backgrounds behave like scroll inside transforms. https://bugs.webkit.org/show_bug.cgi?id=15679
    12271228    bool fixedAttachment = fillLayer->attachment() == FixedBackgroundAttachment;
    1228 
     1229   
    12291230#if ENABLE(FAST_MOBILE_SCROLLING)
    12301231    if (view()->frameView() && view()->frameView()->canBlitOnScroll()) {
    … …  
    12661267            positioningAreaSize = pixelSnappedIntSize(paintRect.size() - LayoutSize(left + right, top + bottom), paintRect.location());
    12671268    } else {
     1269        geometry.setHasNonLocalGeometry();
     1270
    12681271        IntRect viewportRect = pixelSnappedIntRect(viewRect());
    12691272        if (fixedBackgroundPaintsInLocalCoordinates())
    … …  
    12711274        else if (FrameView* frameView = view()->frameView())
    12721275            viewportRect.setLocation(IntPoint(frameView->scrollOffsetForFixedPosition()));
    1273        
     1276
     1277        if (paintContainer) {
     1278            IntPoint absoluteContainerOffset = roundedIntPoint(paintContainer->localToAbsolute(FloatPoint()));
     1279            viewportRect.moveBy(-absoluteContainerOffset);
     1280        }
     1281
    12741282        geometry.setDestRect(pixelSnappedIntRect(viewportRect));
    12751283        positioningAreaSize = geometry.destRect().size();
    12761284    }
    12771285
    1278     RenderObject* clientForBackgroundImage = backgroundObject ? backgroundObject : this;
     1286    const RenderObject* clientForBackgroundImage = backgroundObject ? backgroundObject : this;
    12791287    IntSize fillTileSize = calculateFillTileSize(fillLayer, positioningAreaSize);
    12801288    fillLayer->image()->setContainerSizeForRenderer(clientForBackgroundImage, fillTileSize, style()->effectiveZoom());
    … …  
    13091317}
    13101318
    1311 void RenderBoxModelObject::getGeometryForBackgroundImage(IntRect& destRect, IntPoint& phase, IntSize& tileSize)
     1319void RenderBoxModelObject::getGeometryForBackgroundImage(const RenderLayerModelObject* paintContainer, IntRect& destRect, IntPoint& phase, IntSize& tileSize) const
    13121320{
    13131321    const FillLayer* backgroundLayer = style()->backgroundLayers();
    13141322    BackgroundImageGeometry geometry;
    1315     calculateBackgroundImageGeometry(backgroundLayer, destRect, geometry);
     1323    calculateBackgroundImageGeometry(paintContainer, backgroundLayer, destRect, geometry);
    13161324    phase = geometry.phase();
    13171325    tileSize = geometry.tileSize();

  • trunk/Source/WebCore/rendering/RenderBoxModelObject.h

    r151575 r151623  
    178178    bool canHaveBoxInfoInRegion() const { return !isFloating() && !isReplaced() && !isInline() && !hasColumns() && !isTableCell() && isBlockFlow(); }
    179179
    180     void getGeometryForBackgroundImage(IntRect& destRect, IntPoint& phase, IntSize& tileSize);
     180    void getGeometryForBackgroundImage(const RenderLayerModelObject* paintContainer, IntRect& destRect, IntPoint& phase, IntSize& tileSize) const;
    181181#if USE(ACCELERATED_COMPOSITING)
    182182    void contentChanged(ContentChangeType);
    … …  
    199199    class BackgroundImageGeometry {
    200200    public:
     201        BackgroundImageGeometry()
     202            : m_hasNonLocalGeometry(false)
     203        { }
    201204        IntPoint destOrigin() const { return m_destOrigin; }
    202205        void setDestOrigin(const IntPoint& destOrigin)
    … …  
    235238       
    236239        void clip(const IntRect&);
     240       
     241        void setHasNonLocalGeometry(bool hasNonLocalGeometry = true) { m_hasNonLocalGeometry = hasNonLocalGeometry; }
     242        bool hasNonLocalGeometry() const { return m_hasNonLocalGeometry; }
     243
    237244    private:
    238245        IntRect m_destRect;
    … …  
    240247        IntPoint m_phase;
    241248        IntSize m_tileSize;
     249        bool m_hasNonLocalGeometry; // Has background-attachment: fixed. Implies that we can't always cheaply compute destRect.
    242250    };
    243251
    244252    LayoutPoint adjustedPositionRelativeToOffsetParent(const LayoutPoint&) const;
    245253
    246     void calculateBackgroundImageGeometry(const FillLayer*, const LayoutRect& paintRect, BackgroundImageGeometry&, RenderObject* = 0);
     254    void calculateBackgroundImageGeometry(const RenderLayerModelObject* paintContainer, const FillLayer*, const LayoutRect& paintRect, BackgroundImageGeometry&, RenderObject* = 0) const;
    247255    void getBorderEdgeInfo(class BorderEdge[], const RenderStyle*, bool includeLogicalLeftEdge = true, bool includeLogicalRightEdge = true) const;
    248256    bool borderObscuresBackgroundEdge(const FloatSize& contextScale) const;

  • trunk/Source/WebCore/rendering/RenderImage.cpp

    r150796 r151623  
    511511    if (!hasBackground())
    512512        return false;
    513     return foregroundIsKnownToBeOpaqueInRect(backgroundPaintedExtent(), 0);
     513   
     514    LayoutRect paintedExtent;
     515    if (!getBackgroundPaintedExtent(paintedExtent))
     516        return false;
     517    return foregroundIsKnownToBeOpaqueInRect(paintedExtent, 0);
    514518}
    515519

  • trunk/Source/WebCore/rendering/RenderLayerBacking.cpp

    r151513 r151623  
    14651465
    14661466    RefPtr<Image> image = style->backgroundLayers()->image()->cachedImage()->image();
    1467     toRenderBox(renderer())->getGeometryForBackgroundImage(destRect, phase, tileSize);
     1467    toRenderBox(renderer())->getGeometryForBackgroundImage(m_owningLayer->renderer(), destRect, phase, tileSize);
    14681468    m_graphicsLayer->setContentsTileSize(tileSize);
    14691469    m_graphicsLayer->setContentsTilePhase(phase);
Note: See TracChangeset for help on using the changeset viewer.