Changeset 245490 in webkit


Ignore:
Timestamp:
May 17, 2019, 6:32:03 PM (6 years ago)
Author:
Simon Fraser
Message:

REGRESSION (r245170): gmail.com inbox table header flickers
​https://bugs.webkit.org/show_bug.cgi?id=198005
<rdar://problem/50907718>

Reviewed by Antti Koivisto.

Source/WebCore:

When a layer started as painting into shared backing, but then became independently
composited (e.g. by having to clip composited children), it wouldn't have the "overlap"
indirect compositing reason. This allowed requiresOwnBackingStore() to say that it
could paint into some ancestor, but this breaks overlap. So in this code path,
put IndirectCompositingReason::Overlap back on the layer which restores the previous
behavior.

Make some logging changes to help diagnose things like this.

Test: compositing/shared-backing/overlap-after-end-sharing.html

  • rendering/RenderLayer.cpp:

(WebCore::RenderLayer::calculateClipRects const):
(WebCore::outputPaintOrderTreeLegend):
(WebCore::outputPaintOrderTreeRecursive):

  • rendering/RenderLayer.h:
  • rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::computeCompositingRequirements):
(WebCore::RenderLayerCompositor::updateBacking):
(WebCore::RenderLayerCompositor::requiresOwnBackingStore const):
(WebCore::RenderLayerCompositor::reasonsForCompositing const):
(WebCore::RenderLayerCompositor::requiresCompositingForIndirectReason const):

  • rendering/RenderLayerCompositor.h:

LayoutTests:

  • compositing/shared-backing/overlap-after-end-sharing-expected.html: Added.
  • compositing/shared-backing/overlap-after-end-sharing.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r245483 r245490  
    3333
    3434        * platform/ios/TestExpectations: Skipping in iOS
     35
     362019-05-17  Simon Fraser  <simon.fraser@apple.com>
     37
     38        REGRESSION (r245170): gmail.com inbox table header flickers
     39        https://bugs.webkit.org/show_bug.cgi?id=198005
     40        <rdar://problem/50907718>
     41
     42        Reviewed by Antti Koivisto.
     43
     44        * compositing/shared-backing/overlap-after-end-sharing-expected.html: Added.
     45        * compositing/shared-backing/overlap-after-end-sharing.html: Added.
    3546
    36472019-05-17  Simon Fraser  <simon.fraser@apple.com>

  • trunk/Source/WebCore/ChangeLog

    r245488 r245490  
    147147        * platform/ios/UserAgentIOS.mm:
    148148        (WebCore::standardUserAgentWithApplicationName):
     149
     1502019-05-17  Simon Fraser  <simon.fraser@apple.com>
     151
     152        REGRESSION (r245170): gmail.com inbox table header flickers
     153        https://bugs.webkit.org/show_bug.cgi?id=198005
     154        <rdar://problem/50907718>
     155
     156        Reviewed by Antti Koivisto.
     157
     158        When a layer started as painting into shared backing, but then became independently
     159        composited (e.g. by having to clip composited children), it wouldn't have the "overlap"
     160        indirect compositing reason. This allowed requiresOwnBackingStore() to say that it
     161        could paint into some ancestor, but this breaks overlap. So in this code path,
     162        put IndirectCompositingReason::Overlap back on the layer which restores the previous
     163        behavior.
     164
     165        Make some logging changes to help diagnose things like this.
     166
     167        Test: compositing/shared-backing/overlap-after-end-sharing.html
     168
     169        * rendering/RenderLayer.cpp:
     170        (WebCore::RenderLayer::calculateClipRects const):
     171        (WebCore::outputPaintOrderTreeLegend):
     172        (WebCore::outputPaintOrderTreeRecursive):
     173        * rendering/RenderLayer.h:
     174        * rendering/RenderLayerCompositor.cpp:
     175        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
     176        (WebCore::RenderLayerCompositor::updateBacking):
     177        (WebCore::RenderLayerCompositor::requiresOwnBackingStore const):
     178        (WebCore::RenderLayerCompositor::reasonsForCompositing const):
     179        (WebCore::RenderLayerCompositor::requiresCompositingForIndirectReason const):
     180        * rendering/RenderLayerCompositor.h:
    149181
    1501822019-05-17  Simon Fraser  <simon.fraser@apple.com>

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

    r245336 r245490  
    68886888}
    68896889
     6890TextStream& operator<<(TextStream& ts, IndirectCompositingReason reason)
     6891{
     6892    switch (reason) {
     6893    case IndirectCompositingReason::None: ts << "none"; break;
     6894    case IndirectCompositingReason::Stacking: ts << "stacking"; break;
     6895    case IndirectCompositingReason::OverflowScrollPositioning: ts << "overflow positioning"; break;
     6896    case IndirectCompositingReason::Overlap: ts << "overlap"; break;
     6897    case IndirectCompositingReason::BackgroundLayer: ts << "background layer"; break;
     6898    case IndirectCompositingReason::GraphicalEffect: ts << "graphical effect"; break;
     6899    case IndirectCompositingReason::Perspective: ts << "perspective"; break;
     6900    case IndirectCompositingReason::Preserve3D: ts << "preserve-3d"; break;
     6901    }
     6902
     6903    return ts;
     6904}
     6905
    68906906} // namespace WebCore
    68916907
    … …  
    69226938{
    69236939    stream.nextLine();
    6924     stream << "(S)tacking Context/(F)orced SC/O(P)portunistic SC, (N)ormal flow only, (O)verflow clip, (A)lpha (opacity or mask), has (B)lend mode, (I)solates blending, (T)ransform-ish, (F)ilter, Fi(X)ed position, (C)omposited, (P)rovides backing/uses (p)rovided backing, (c)omposited descendant, (s)scrolling ancestor\n"
     6940    stream << "(S)tacking Context/(F)orced SC/O(P)portunistic SC, (N)ormal flow only, (O)verflow clip, (A)lpha (opacity or mask), has (B)lend mode, (I)solates blending, (T)ransform-ish, (F)ilter, Fi(X)ed position, (C)omposited, (P)rovides backing/uses (p)rovided backing/paints to (a)ncestor, (c)omposited descendant, (s)scrolling ancestor\n"
    69256941        "Dirty (z)-lists, Dirty (n)ormal flow lists\n"
    69266942        "Traversal needs: requirements (t)raversal on descendants, (b)acking or hierarchy traversal on descendants, (r)equirements traversal on all descendants, requirements traversal on all (s)ubsequent layers, (h)ierarchy traversal on all descendants, update of paint (o)rder children\n"
    … …  
    69486964    stream << (layer.renderer().isFixedPositioned() ? "X" : "-");
    69496965    stream << (layer.isComposited() ? "C" : "-");
    6950     stream << ((layer.isComposited() && layer.backing()->hasBackingSharingLayers()) ? "P" : (layer.paintsIntoProvidedBacking() ? "p" : "-"));
     6966   
     6967    auto compositedPaintingDestinationString = [&layer]() {
     6968        if (layer.paintsIntoProvidedBacking())
     6969            return "p";
     6970
     6971        if (!layer.isComposited())
     6972            return "-";
     6973
     6974        if (layer.backing()->hasBackingSharingLayers())
     6975            return "P";
     6976       
     6977        if (layer.backing()->paintsIntoCompositedAncestor())
     6978            return "a";
     6979
     6980        return "-";
     6981    };
     6982
     6983    stream << compositedPaintingDestinationString();
    69516984    stream << (layer.hasCompositingDescendant() ? "c" : "-");
    69526985    stream << (layer.hasCompositedScrollingAncestor() ? "s" : "-");
    … …  
    69877020        auto& backing = *layer.backing();
    69887021        stream << " (layerID " << backing.graphicsLayer()->primaryLayerID() << ")";
     7022       
     7023        if (layer.indirectCompositingReason() != WebCore::IndirectCompositingReason::None)
     7024            stream << " " << layer.indirectCompositingReason();
    69897025
    69907026        auto scrollingNodeID = backing.scrollingNodeIDForRole(WebCore::ScrollCoordinationRole::Scrolling);

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

    r245293 r245490  
    118118};
    119119
     120enum class IndirectCompositingReason {
     121    None,
     122    Stacking,
     123    OverflowScrollPositioning,
     124    Overlap,
     125    BackgroundLayer,
     126    GraphicalEffect, // opacity, mask, filter, transform etc.
     127    Perspective,
     128    Preserve3D
     129};
     130
    120131struct ScrollRectToVisibleOptions {
    121132    SelectionRevealMode revealMode { SelectionRevealMode::Reveal };
    … …  
    871882    void setViewportConstrainedNotCompositedReason(ViewportConstrainedNotCompositedReason reason) { m_viewportConstrainedNotCompositedReason = reason; }
    872883    ViewportConstrainedNotCompositedReason viewportConstrainedNotCompositedReason() const { return static_cast<ViewportConstrainedNotCompositedReason>(m_viewportConstrainedNotCompositedReason); }
    873    
     884
     885    IndirectCompositingReason indirectCompositingReason() const { return static_cast<IndirectCompositingReason>(m_indirectCompositingReason); }
     886
    874887    bool isRenderFragmentedFlow() const { return renderer().isRenderFragmentedFlow(); }
    875888    bool isOutOfFlowRenderFragmentedFlow() const { return renderer().isOutOfFlowRenderFragmentedFlow(); }
    … …  
    11581171    void setHasCompositingDescendant(bool b)  { m_hasCompositingDescendant = b; }
    11591172   
    1160     enum class IndirectCompositingReason {
    1161         None,
    1162         Stacking,
    1163         OverflowScrollPositioning,
    1164         Overlap,
    1165         BackgroundLayer,
    1166         GraphicalEffect, // opacity, mask, filter, transform etc.
    1167         Perspective,
    1168         Preserve3D
    1169     };
    1170    
    11711173    void setIndirectCompositingReason(IndirectCompositingReason reason) { m_indirectCompositingReason = static_cast<unsigned>(reason); }
    1172     IndirectCompositingReason indirectCompositingReason() const { return static_cast<IndirectCompositingReason>(m_indirectCompositingReason); }
    11731174    bool mustCompositeForIndirectReasons() const { return m_indirectCompositingReason; }
    11741175
    … …  
    13881389WTF::TextStream& operator<<(WTF::TextStream&, const RenderLayer&);
    13891390WTF::TextStream& operator<<(WTF::TextStream&, const RenderLayer::ClipRectsContext&);
     1391WTF::TextStream& operator<<(WTF::TextStream&, IndirectCompositingReason);
    13901392
    13911393} // namespace WebCore

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

    r245471 r245490  
    863863
    864864    if (layer.needsPostLayoutCompositingUpdate() || compositingState.fullPaintOrderTraversalRequired || compositingState.descendantsRequireCompositingUpdate) {
    865         layer.setIndirectCompositingReason(RenderLayer::IndirectCompositingReason::None);
     865        layer.setIndirectCompositingReason(IndirectCompositingReason::None);
    866866        willBeComposited = needsToBeComposited(layer, queryData);
    867867    }
    … …  
    878878    overlapMap.geometryMap().pushMappingsToAncestor(&layer, ancestorLayer, respectTransforms);
    879879
    880     RenderLayer::IndirectCompositingReason compositingReason = compositingState.subtreeIsCompositing ? RenderLayer::IndirectCompositingReason::Stacking : RenderLayer::IndirectCompositingReason::None;
     880    IndirectCompositingReason compositingReason = compositingState.subtreeIsCompositing ? IndirectCompositingReason::Stacking : IndirectCompositingReason::None;
    881881    bool layerPaintsIntoProvidedBacking = false;
    882882
    … …  
    890890                backingSharingState.appendSharingLayer(layer);
    891891                LOG(Compositing, " layer %p can share with %p", &layer, backingSharingState.backingProviderCandidate());
    892                 compositingReason = RenderLayer::IndirectCompositingReason::None;
     892                compositingReason = IndirectCompositingReason::None;
    893893                layerPaintsIntoProvidedBacking = true;
    894894            } else
    895                 compositingReason = RenderLayer::IndirectCompositingReason::Overlap;
     895                compositingReason = IndirectCompositingReason::Overlap;
    896896        } else
    897             compositingReason = RenderLayer::IndirectCompositingReason::None;
     897            compositingReason = IndirectCompositingReason::None;
    898898    }
    899899
    … …  
    904904    // own layers to draw on top of the accelerated video.
    905905    if (compositingState.compositingAncestor && compositingState.compositingAncestor->renderer().isVideo())
    906         compositingReason = RenderLayer::IndirectCompositingReason::Overlap;
    907 #endif
    908 
    909     if (compositingReason != RenderLayer::IndirectCompositingReason::None)
     906        compositingReason = IndirectCompositingReason::Overlap;
     907#endif
     908
     909    if (compositingReason != IndirectCompositingReason::None)
    910910        layer.setIndirectCompositingReason(compositingReason);
    911911
    912912    // Check if the computed indirect reason will force the layer to become composited.
    913913    if (!willBeComposited && layer.mustCompositeForIndirectReasons() && canBeComposited(layer)) {
     914        LOG_WITH_STREAM(Compositing, stream << "layer " << &layer << " compositing for indirect reason " << layer.indirectCompositingReason() << " (was sharing: " << layerPaintsIntoProvidedBacking << ")");
    914915        willBeComposited = true;
    915916        layerPaintsIntoProvidedBacking = false;
    … …  
    928929        currentState.compositingAncestor = &layer;
    929930       
    930         if (!layerPaintsIntoProvidedBacking) {
     931        if (layerPaintsIntoProvidedBacking) {
     932            layerPaintsIntoProvidedBacking = false;
     933            // layerPaintsIntoProvidedBacking was only true for layers that would otherwise composite because of overlap. If we can
     934            // no longer share, put this this indirect reason back on the layer so that requiresOwnBackingStore() sees it.
     935            layer.setIndirectCompositingReason(IndirectCompositingReason::Overlap);
     936            LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " was sharing now will composite");
     937        } else {
    931938            overlapMap.pushCompositingContainer();
    932939            LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " will composite, pushed container " << overlapMap);
    … …  
    934941
    935942        willBeComposited = true;
    936         layerPaintsIntoProvidedBacking = false;
    937943    };
    938944
    … …  
    972978        if (!willBeComposited && currentState.subtreeIsCompositing) {
    973979            // make layer compositing
    974             layer.setIndirectCompositingReason(RenderLayer::IndirectCompositingReason::BackgroundLayer);
     980            layer.setIndirectCompositingReason(IndirectCompositingReason::BackgroundLayer);
    975981            layerWillComposite();
    976982        }
    … …  
    10001006#endif
    10011007    // Now check for reasons to become composited that depend on the state of descendant layers.
    1002     RenderLayer::IndirectCompositingReason indirectCompositingReason;
     1008    IndirectCompositingReason indirectCompositingReason;
    10031009    if (!willBeComposited && canBeComposited(layer)
    10041010        && requiresCompositingForIndirectReason(layer, compositingState.compositingAncestor, currentState.subtreeIsCompositing, anyDescendantHas3DTransform, layerPaintsIntoProvidedBacking, indirectCompositingReason)) {
    … …  
    10091015    if (layer.reflectionLayer()) {
    10101016        // FIXME: Shouldn't we call computeCompositingRequirements to handle a reflection overlapping with another renderer?
    1011         layer.reflectionLayer()->setIndirectCompositingReason(willBeComposited ? RenderLayer::IndirectCompositingReason::Stacking : RenderLayer::IndirectCompositingReason::None);
     1017        layer.reflectionLayer()->setIndirectCompositingReason(willBeComposited ? IndirectCompositingReason::Stacking : IndirectCompositingReason::None);
    10121018    }
    10131019
    … …  
    15841590            // If we need to repaint, do so before making backing
    15851591            if (shouldRepaint == CompositingChangeRepaintNow)
    1586                 repaintOnCompositingChange(layer);
     1592                repaintOnCompositingChange(layer); // wrong backing
    15871593
    15881594            layer.ensureBacking();
    … …  
    22832289
    22842290    if (layer.mustCompositeForIndirectReasons()) {
    2285         RenderLayer::IndirectCompositingReason reason = layer.indirectCompositingReason();
    2286         return reason == RenderLayer::IndirectCompositingReason::Overlap
    2287             || reason == RenderLayer::IndirectCompositingReason::OverflowScrollPositioning
    2288             || reason == RenderLayer::IndirectCompositingReason::Stacking
    2289             || reason == RenderLayer::IndirectCompositingReason::BackgroundLayer
    2290             || reason == RenderLayer::IndirectCompositingReason::GraphicalEffect
    2291             || reason == RenderLayer::IndirectCompositingReason::Preserve3D; // preserve-3d has to create backing store to ensure that 3d-transformed elements intersect.
     2291        IndirectCompositingReason reason = layer.indirectCompositingReason();
     2292        return reason == IndirectCompositingReason::Overlap
     2293            || reason == IndirectCompositingReason::OverflowScrollPositioning
     2294            || reason == IndirectCompositingReason::Stacking
     2295            || reason == IndirectCompositingReason::BackgroundLayer
     2296            || reason == IndirectCompositingReason::GraphicalEffect
     2297            || reason == IndirectCompositingReason::Preserve3D; // preserve-3d has to create backing store to ensure that 3d-transformed elements intersect.
    22922298    }
    22932299
    … …  
    23482354
    23492355    switch (renderer.layer()->indirectCompositingReason()) {
    2350     case RenderLayer::IndirectCompositingReason::None:
     2356    case IndirectCompositingReason::None:
    23512357        break;
    2352     case RenderLayer::IndirectCompositingReason::Stacking:
     2358    case IndirectCompositingReason::Stacking:
    23532359        reasons.add(CompositingReason::Stacking);
    23542360        break;
    2355     case RenderLayer::IndirectCompositingReason::OverflowScrollPositioning:
     2361    case IndirectCompositingReason::OverflowScrollPositioning:
    23562362        reasons.add(CompositingReason::OverflowScrollPositioning);
    23572363        break;
    2358     case RenderLayer::IndirectCompositingReason::Overlap:
     2364    case IndirectCompositingReason::Overlap:
    23592365        reasons.add(CompositingReason::Overlap);
    23602366        break;
    2361     case RenderLayer::IndirectCompositingReason::BackgroundLayer:
     2367    case IndirectCompositingReason::BackgroundLayer:
    23622368        reasons.add(CompositingReason::NegativeZIndexChildren);
    23632369        break;
    2364     case RenderLayer::IndirectCompositingReason::GraphicalEffect:
     2370    case IndirectCompositingReason::GraphicalEffect:
    23652371        if (renderer.hasTransform())
    23662372            reasons.add(CompositingReason::TransformWithCompositedDescendants);
    … …  
    23862392#endif
    23872393        break;
    2388     case RenderLayer::IndirectCompositingReason::Perspective:
     2394    case IndirectCompositingReason::Perspective:
    23892395        reasons.add(CompositingReason::Perspective);
    23902396        break;
    2391     case RenderLayer::IndirectCompositingReason::Preserve3D:
     2397    case IndirectCompositingReason::Preserve3D:
    23922398        reasons.add(CompositingReason::Preserve3D);
    23932399        break;
    … …  
    28302836
    28312837// FIXME: why doesn't this handle the clipping cases?
    2832 bool RenderLayerCompositor::requiresCompositingForIndirectReason(const RenderLayer& layer, const RenderLayer* compositingAncestor, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, RenderLayer::IndirectCompositingReason& reason) const
     2838bool RenderLayerCompositor::requiresCompositingForIndirectReason(const RenderLayer& layer, const RenderLayer* compositingAncestor, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, IndirectCompositingReason& reason) const
    28332839{
    28342840    // When a layer has composited descendants, some effects, like 2d transforms, filters, masks etc must be implemented
    … …  
    28362842    auto& renderer = layer.renderer();
    28372843    if (hasCompositedDescendants && (layer.isolatesCompositedBlending() || layer.transform() || renderer.createsGroup() || renderer.hasReflection())) {
    2838         reason = RenderLayer::IndirectCompositingReason::GraphicalEffect;
     2844        reason = IndirectCompositingReason::GraphicalEffect;
    28392845        return true;
    28402846    }
    … …  
    28442850    if (has3DTransformedDescendants) {
    28452851        if (renderer.style().transformStyle3D() == TransformStyle3D::Preserve3D) {
    2846             reason = RenderLayer::IndirectCompositingReason::Preserve3D;
     2852            reason = IndirectCompositingReason::Preserve3D;
    28472853            return true;
    28482854        }
    28492855   
    28502856        if (renderer.style().hasPerspective()) {
    2851             reason = RenderLayer::IndirectCompositingReason::Perspective;
     2857            reason = IndirectCompositingReason::Perspective;
    28522858            return true;
    28532859        }
    … …  
    28562862    if (!paintsIntoProvidedBacking && renderer.isAbsolutelyPositioned() && compositingAncestor && layer.hasCompositedScrollingAncestor()) {
    28572863        if (layerContainingBlockCrossesCoordinatedScrollingBoundary(layer, *compositingAncestor)) {
    2858             reason = RenderLayer::IndirectCompositingReason::OverflowScrollPositioning;
     2864            reason = IndirectCompositingReason::OverflowScrollPositioning;
    28592865            return true;
    28602866        }
    28612867    }
    28622868
    2863     reason = RenderLayer::IndirectCompositingReason::None;
     2869    reason = IndirectCompositingReason::None;
    28642870    return false;
    28652871}

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

    r245373 r245490  
    479479    bool requiresCompositingForOverflowScrolling(const RenderLayer&, RequiresCompositingData&) const;
    480480    bool requiresCompositingForEditableImage(RenderLayerModelObject&) const;
    481     bool requiresCompositingForIndirectReason(const RenderLayer&, const RenderLayer* compositingAncestor, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, RenderLayer::IndirectCompositingReason&) const;
     481    bool requiresCompositingForIndirectReason(const RenderLayer&, const RenderLayer* compositingAncestor, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, IndirectCompositingReason&) const;
    482482
    483483    static bool layerContainingBlockCrossesCoordinatedScrollingBoundary(const RenderLayer&, const RenderLayer& compositedAncestor);
Note: See TracChangeset for help on using the changeset viewer.