Changeset 290785 in webkit


Ignore:
Timestamp:
Mar 3, 2022 10:25:57 AM (5 months ago)
Author:
Simon Fraser
Message:

nasa.gov page with fixed backgrounds paints incorrectly on scroll
​https://bugs.webkit.org/show_bug.cgi?id=237405
<rdar://66568551>

Reviewed by Antti Koivisto.
Source/WebCore:

​https://www.nasa.gov/specials/artemis/ shows an issue where elements with background-attachment:fixed
don't repaint on scroll. This page has scrollable <html> and <body>, and the elements with fixed
backgrounds are composited, so this reveals that we fail to repaint composited children
of an overflow scroll in this case.

Fix by having RenderLayerScrollableArea::scrollTo() do repaints on slow repaint objects
which are scrolled by the current scroller.

Do some unrelated cleanup in code that I was going to use in this patch but turned out
not to need: rename hasFixedBackgroundImage() to hasAnyFixedBackground() for clarity,
and share the implementation with hasAnyLocalBackground().

Test: fast/repaint/background-attachment-fixed-in-composited-scroll.html

  • rendering/RenderElement.cpp:

(WebCore::RenderElement::styleWillChange):
(WebCore::RenderElement::willBeDestroyed):

  • rendering/RenderLayer.cpp:
  • rendering/RenderLayerScrollableArea.cpp:

(WebCore::RenderLayerScrollableArea::scrollTo):

  • rendering/style/FillLayer.cpp:

(WebCore::FillLayer::hasImageWithAttachment const):
(WebCore::FillLayer::hasFixedImage const): Deleted.

  • rendering/style/FillLayer.h:
  • rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::hasAnyLocalBackground const): Deleted.

  • rendering/style/RenderStyle.h:

(WebCore::RenderStyle::hasBackgroundImage const):
(WebCore::RenderStyle::hasAnyFixedBackground const):
(WebCore::RenderStyle::hasAnyLocalBackground const):
(WebCore::RenderStyle::hasFixedBackgroundImage const): Deleted.

LayoutTests:

Repaint test which is only valid for mac-wk2 (iOS does not support background-attachment:fixed).

  • TestExpectations:
  • fast/repaint/background-attachment-fixed-in-composited-scroll-expected.txt: Added.
  • fast/repaint/background-attachment-fixed-in-composited-scroll.html: Added.
  • platform/mac-wk2/TestExpectations:
Location:
trunk
Files:
2 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r290783 r290785  
     12022-03-03  Simon Fraser  <simon.fraser@apple.com>
     2
     3        nasa.gov page with fixed backgrounds paints incorrectly on scroll
     4        https://bugs.webkit.org/show_bug.cgi?id=237405
     5        <rdar://66568551>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Repaint test which is only valid for mac-wk2 (iOS does not support background-attachment:fixed).
     10
     11        * TestExpectations:
     12        * fast/repaint/background-attachment-fixed-in-composited-scroll-expected.txt: Added.
     13        * fast/repaint/background-attachment-fixed-in-composited-scroll.html: Added.
     14        * platform/mac-wk2/TestExpectations:
     15
    1162022-03-03  Jon Lee  <jonlee@apple.com>
    217

  • trunk/LayoutTests/TestExpectations

    r290729 r290785  
    9393compositing/layer-creation/clipping-scope [ Skip ]
    9494fast/repaint/background-attachment-local-scroll.html [ Skip ]
     95fast/repaint/background-attachment-fixed-in-composited-scroll.html [ Skip ]
    9596
    9697# WebKit2 only.

  • trunk/LayoutTests/platform/mac-wk2/TestExpectations

    r290745 r290785  
    4444
    4545fast/repaint/background-attachment-local-scroll.html [ Pass ]
     46fast/repaint/background-attachment-fixed-in-composited-scroll.html [ Pass ]
    4647fast/scrolling/unfocusing-page-while-keyboard-scrolling.html [ Pass ]
    4748

  • trunk/Source/WebCore/ChangeLog

    r290782 r290785  
     12022-03-03  Simon Fraser  <simon.fraser@apple.com>
     2
     3        nasa.gov page with fixed backgrounds paints incorrectly on scroll
     4        https://bugs.webkit.org/show_bug.cgi?id=237405
     5        <rdar://66568551>
     6
     7        Reviewed by Antti Koivisto.
     8       
     9        https://www.nasa.gov/specials/artemis/ shows an issue where elements with background-attachment:fixed
     10        don't repaint on scroll. This page has scrollable <html> and <body>, and the elements with fixed
     11        backgrounds are composited, so this reveals that we fail to repaint composited children
     12        of an overflow scroll in this case.
     13
     14        Fix by having RenderLayerScrollableArea::scrollTo() do repaints on slow repaint objects
     15        which are scrolled by the current scroller.
     16       
     17        Do some unrelated cleanup in code that I was going to use in this patch but turned out
     18        not to need: rename hasFixedBackgroundImage() to hasAnyFixedBackground() for clarity,
     19        and share the implementation with hasAnyLocalBackground().
     20
     21        Test: fast/repaint/background-attachment-fixed-in-composited-scroll.html
     22
     23        * rendering/RenderElement.cpp:
     24        (WebCore::RenderElement::styleWillChange):
     25        (WebCore::RenderElement::willBeDestroyed):
     26        * rendering/RenderLayer.cpp:
     27        * rendering/RenderLayerScrollableArea.cpp:
     28        (WebCore::RenderLayerScrollableArea::scrollTo):
     29        * rendering/style/FillLayer.cpp:
     30        (WebCore::FillLayer::hasImageWithAttachment const):
     31        (WebCore::FillLayer::hasFixedImage const): Deleted.
     32        * rendering/style/FillLayer.h:
     33        * rendering/style/RenderStyle.cpp:
     34        (WebCore::RenderStyle::hasAnyLocalBackground const): Deleted.
     35        * rendering/style/RenderStyle.h:
     36        (WebCore::RenderStyle::hasBackgroundImage const):
     37        (WebCore::RenderStyle::hasAnyFixedBackground const):
     38        (WebCore::RenderStyle::hasAnyLocalBackground const):
     39        (WebCore::RenderStyle::hasFixedBackgroundImage const): Deleted.
     40
    1412022-03-03  Alan Bujtas  <zalan@apple.com>
    242

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

    r290564 r290785  
    904904
    905905    bool newStyleSlowScroll = false;
    906     if (newStyle.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument()) {
     906    if (newStyle.hasAnyFixedBackground() && !settings().fixedBackgroundsPaintRelativeToDocument()) {
    907907        newStyleSlowScroll = true;
    908908        bool drawsRootBackground = isDocumentElementRenderer() || (isBody() && !rendererHasBackground(document().documentElement()->renderer()));
    … …  
    10531053        document().contentChangeObserver().rendererWillBeDestroyed(*element());
    10541054#endif
    1055     if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
     1055    if (m_style.hasAnyFixedBackground() && !settings().fixedBackgroundsPaintRelativeToDocument())
    10561056        view().frameView().removeSlowRepaintObject(*this);
    10571057

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

    r290770 r290785  
    30363036    return paintingReflection && !layer->has3DTransform();
    30373037}
    3038    
     3038
    30393039static inline bool shouldSuppressPaintingLayer(RenderLayer* layer)
    30403040{

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

    r290625 r290785  
    380380    }
    381381
     382    auto isScrolledBy = [](RenderObject& renderer, RenderLayer& scrollableLayer) {
     383        auto layer = renderer.enclosingLayer();
     384        return layer && layer->ancestorLayerIsInContainingBlockChain(scrollableLayer);
     385    };
     386
    382387    // Just schedule a full repaint of our object.
    383     if (requiresRepaint)
     388    if (requiresRepaint) {
    384389        renderer.repaintUsingContainer(repaintContainer, rectForRepaint);
     390
     391        // We also have to repaint any descendant composited layers that have fixed backgrounds.
     392        if (auto slowRepaintObjects = view.frameView().slowRepaintObjects()) {
     393            for (auto& renderer : *slowRepaintObjects) {
     394                if (isScrolledBy(renderer, m_layer))
     395                    renderer.repaint();
     396            }
     397        }
     398    }
    385399
    386400    // Schedule the scroll and scroll-related DOM events.

  • trunk/Source/WebCore/rendering/style/FillLayer.cpp

    r286795 r290785  
    388388}
    389389
    390 bool FillLayer::hasFixedImage() const
     390bool FillLayer::hasImageWithAttachment(FillAttachment attachment) const
    391391{
    392392    for (auto* layer = this; layer; layer = layer->m_next.get()) {
    393         if (layer->m_image && layer->attachment() == FillAttachment::FixedBackground)
     393        if (layer->m_image && layer->attachment() == attachment)
    394394            return true;
    395395    }

  • trunk/Source/WebCore/rendering/style/FillLayer.h

    r286795 r290785  
    150150    bool imagesAreLoaded() const;
    151151    bool hasImage() const { return m_next ? hasImageInAnyLayer() : m_image; }
    152     bool hasFixedImage() const;
     152    bool hasImageWithAttachment(FillAttachment) const;
    153153    bool hasOpaqueImage(const RenderElement&) const;
    154154    bool hasRepeatXY() const;

  • trunk/Source/WebCore/rendering/style/RenderStyle.cpp

    r290779 r290785  
    16881688{
    16891689    return allLayersAreFixed(backgroundLayers());
    1690 }
    1691 
    1692 bool RenderStyle::hasAnyLocalBackground() const
    1693 {
    1694     for (auto* layer = &backgroundLayers(); layer; layer = layer->next()) {
    1695         if (layer->image() && layer->attachment() == FillAttachment::LocalBackground)
    1696             return true;
    1697     }
    1698     return false;
    16991690}
    17001691

  • trunk/Source/WebCore/rendering/style/RenderStyle.h

    r290779 r290785  
    212212    bool hasMarginAfterQuirk() const { return marginAfter().hasQuirk(); }
    213213
    214     bool hasBackgroundImage() const { return m_backgroundData->background->hasImage(); }
    215     bool hasFixedBackgroundImage() const { return m_backgroundData->background->hasFixedImage(); }
     214    bool hasBackgroundImage() const { return backgroundLayers().hasImage(); }
     215    bool hasAnyFixedBackground() const { return backgroundLayers().hasImageWithAttachment(FillAttachment::FixedBackground); }
    216216
    217217    bool hasEntirelyFixedBackground() const;
    218     bool hasAnyLocalBackground() const;
     218    bool hasAnyLocalBackground() const { return backgroundLayers().hasImageWithAttachment(FillAttachment::LocalBackground); }
    219219
    220220    bool hasAppearance() const { return appearance() != NoControlPart; }
Note: See TracChangeset for help on using the changeset viewer.