- Revision
- 290785
- Author
- simon.fra...@apple.com
- Date
- 2022-03-03 10:25:57 -0800 (Thu, 03 Mar 2022)
Log 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:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (290784 => 290785)
--- trunk/LayoutTests/ChangeLog 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/LayoutTests/ChangeLog 2022-03-03 18:25:57 UTC (rev 290785)
@@ -1,3 +1,18 @@
+2022-03-03 Simon Fraser <simon.fra...@apple.com>
+
+ 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.
+
+ 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:
+
2022-03-03 Jon Lee <jon...@apple.com>
Unreviewed test gardening.
Modified: trunk/LayoutTests/TestExpectations (290784 => 290785)
--- trunk/LayoutTests/TestExpectations 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/LayoutTests/TestExpectations 2022-03-03 18:25:57 UTC (rev 290785)
@@ -92,6 +92,7 @@
compositing/scrolling/async-overflow-scrolling [ Skip ]
compositing/layer-creation/clipping-scope [ Skip ]
fast/repaint/background-attachment-local-scroll.html [ Skip ]
+fast/repaint/background-attachment-fixed-in-composited-scroll.html [ Skip ]
# WebKit2 only.
printing/printing-events.html [ Skip ]
Added: trunk/LayoutTests/fast/repaint/background-attachment-fixed-in-composited-scroll-expected.txt (0 => 290785)
--- trunk/LayoutTests/fast/repaint/background-attachment-fixed-in-composited-scroll-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/repaint/background-attachment-fixed-in-composited-scroll-expected.txt 2022-03-03 18:25:57 UTC (rev 290785)
@@ -0,0 +1,11 @@
+Test that a scroll of an overflow scrolling element, with a composoited `background-attachment:fixed` child, repaints
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS layerTreeAsText.indexOf('(rect 0.00 0.00 280.00 200.00)') > -1 is true
+PASS layerTreeAsText.indexOf('(rect 11.00 11.00 278.00 178.00)') > -1 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/repaint/background-attachment-fixed-in-composited-scroll.html (0 => 290785)
--- trunk/LayoutTests/fast/repaint/background-attachment-fixed-in-composited-scroll.html (rev 0)
+++ trunk/LayoutTests/fast/repaint/background-attachment-fixed-in-composited-scroll.html 2022-03-03 18:25:57 UTC (rev 290785)
@@ -0,0 +1,95 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <style>
+ #scroller {
+ width: 300px;
+ height: 300px;
+ overflow: scroll;
+ border: 1px solid black;
+ }
+
+ .filler {
+ width: 30px;
+ height: 150px;
+ margin: 10px;
+ background-color: silver;
+ }
+
+ .fixed-background {
+ position: relative;
+ margin: 10px;
+ height: 200px;
+ background-image: linear-gradient(lightblue, palegreen);
+ background-attachment: fixed;
+ background-size: 1px 300px;
+ border: 1px solid blue;
+ box-sizing: border-box;
+ }
+
+ .fixed-child-background {
+ position: relative;
+ height: 200px;
+ padding: 10px;
+ background-size: 1px 300px;
+ border: 1px solid blue;
+ box-sizing: border-box;
+ }
+
+ .fixed-child-background > div {
+ height: 100%;
+ background-image: linear-gradient(lightblue, palegreen);
+ background-attachment: fixed;
+ background-size: 1px 300px;
+ }
+
+ .composited {
+ transform: translateZ(0);
+ }
+
+ /* Hide the scrollbars to remove scrollbar repaints. */
+ ::-webkit-scrollbar {
+ display: none;
+ }
+ </style>
+ <script src=""
+ <script>
+ jsTestIsAsync = true;
+ description("Test that a scroll of an overflow scrolling element, with a composoited `background-attachment:fixed` child, repaints");
+
+ function startTrackingRepaints()
+ {
+ window.internals.startTrackingRepaints();
+ document.getElementById('scroller').scrollTop = 800;
+
+ logRepaints();
+ }
+
+ function logRepaints()
+ {
+ layerTreeAsText = window.internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_REPAINT_RECTS);
+ window.internals.stopTrackingRepaints();
+ shouldBeTrue("layerTreeAsText.indexOf('(rect 0.00 0.00 280.00 200.00)') > -1");
+ shouldBeTrue("layerTreeAsText.indexOf('(rect 11.00 11.00 278.00 178.00)') > -1");
+
+ finishJSTest();
+ }
+
+ window.addEventListener('load', () => {
+ if (!window.testRunner || !window.internals)
+ return;
+
+ setTimeout(startTrackingRepaints, 0);
+ }, false);
+ </script>
+</head>
+<body>
+ <div id="scroller">
+ <div class="composited filler"></div>
+ <div class="fixed-background"></div>
+ <div class="fixed-child-background"><div></div></div>
+ <div class="filler"></div>
+ </div>
+
+<script src=""
+</html>
Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (290784 => 290785)
--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2022-03-03 18:25:57 UTC (rev 290785)
@@ -43,6 +43,7 @@
fast/media/mq-prefers-contrast.html [ Pass ]
fast/repaint/background-attachment-local-scroll.html [ Pass ]
+fast/repaint/background-attachment-fixed-in-composited-scroll.html [ Pass ]
fast/scrolling/unfocusing-page-while-keyboard-scrolling.html [ Pass ]
http/tests/ssl/applepay [ Pass ]
Modified: trunk/Source/WebCore/ChangeLog (290784 => 290785)
--- trunk/Source/WebCore/ChangeLog 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/Source/WebCore/ChangeLog 2022-03-03 18:25:57 UTC (rev 290785)
@@ -1,3 +1,43 @@
+2022-03-03 Simon Fraser <simon.fra...@apple.com>
+
+ 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.
+
+ 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.
+
2022-03-03 Alan Bujtas <za...@apple.com>
A text node longer than 65,535 characters following another text node is invisible in a scrolling context
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (290784 => 290785)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2022-03-03 18:25:57 UTC (rev 290785)
@@ -903,7 +903,7 @@
}
bool newStyleSlowScroll = false;
- if (newStyle.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument()) {
+ if (newStyle.hasAnyFixedBackground() && !settings().fixedBackgroundsPaintRelativeToDocument()) {
newStyleSlowScroll = true;
bool drawsRootBackground = isDocumentElementRenderer() || (isBody() && !rendererHasBackground(document().documentElement()->renderer()));
if (drawsRootBackground && newStyle.hasEntirelyFixedBackground() && view().compositor().supportsFixedRootBackgroundCompositing())
@@ -1052,7 +1052,7 @@
if (!renderTreeBeingDestroyed() && element())
document().contentChangeObserver().rendererWillBeDestroyed(*element());
#endif
- if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
+ if (m_style.hasAnyFixedBackground() && !settings().fixedBackgroundsPaintRelativeToDocument())
view().frameView().removeSlowRepaintObject(*this);
unregisterForVisibleInViewportCallback();
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (290784 => 290785)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2022-03-03 18:25:57 UTC (rev 290785)
@@ -3035,7 +3035,7 @@
{
return paintingReflection && !layer->has3DTransform();
}
-
+
static inline bool shouldSuppressPaintingLayer(RenderLayer* layer)
{
if (layer->renderer().style().isNotFinal() && !layer->isRenderViewLayer() && !layer->renderer().isDocumentElementRenderer())
Modified: trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp (290784 => 290785)
--- trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp 2022-03-03 18:25:57 UTC (rev 290785)
@@ -379,10 +379,24 @@
requiresRepaint = m_layer.backing()->needsRepaintOnCompositedScroll();
}
+ auto isScrolledBy = [](RenderObject& renderer, RenderLayer& scrollableLayer) {
+ auto layer = renderer.enclosingLayer();
+ return layer && layer->ancestorLayerIsInContainingBlockChain(scrollableLayer);
+ };
+
// Just schedule a full repaint of our object.
- if (requiresRepaint)
+ if (requiresRepaint) {
renderer.repaintUsingContainer(repaintContainer, rectForRepaint);
+ // We also have to repaint any descendant composited layers that have fixed backgrounds.
+ if (auto slowRepaintObjects = view.frameView().slowRepaintObjects()) {
+ for (auto& renderer : *slowRepaintObjects) {
+ if (isScrolledBy(renderer, m_layer))
+ renderer.repaint();
+ }
+ }
+ }
+
// Schedule the scroll and scroll-related DOM events.
if (Element* element = renderer.element())
element->document().addPendingScrollEventTarget(*element);
Modified: trunk/Source/WebCore/rendering/style/FillLayer.cpp (290784 => 290785)
--- trunk/Source/WebCore/rendering/style/FillLayer.cpp 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/Source/WebCore/rendering/style/FillLayer.cpp 2022-03-03 18:25:57 UTC (rev 290785)
@@ -387,10 +387,10 @@
return false;
}
-bool FillLayer::hasFixedImage() const
+bool FillLayer::hasImageWithAttachment(FillAttachment attachment) const
{
for (auto* layer = this; layer; layer = layer->m_next.get()) {
- if (layer->m_image && layer->attachment() == FillAttachment::FixedBackground)
+ if (layer->m_image && layer->attachment() == attachment)
return true;
}
return false;
Modified: trunk/Source/WebCore/rendering/style/FillLayer.h (290784 => 290785)
--- trunk/Source/WebCore/rendering/style/FillLayer.h 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/Source/WebCore/rendering/style/FillLayer.h 2022-03-03 18:25:57 UTC (rev 290785)
@@ -149,7 +149,7 @@
bool containsImage(StyleImage&) const;
bool imagesAreLoaded() const;
bool hasImage() const { return m_next ? hasImageInAnyLayer() : m_image; }
- bool hasFixedImage() const;
+ bool hasImageWithAttachment(FillAttachment) const;
bool hasOpaqueImage(const RenderElement&) const;
bool hasRepeatXY() const;
bool clipOccludesNextLayers(bool firstLayer) const;
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (290784 => 290785)
--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2022-03-03 18:25:57 UTC (rev 290785)
@@ -1689,15 +1689,6 @@
return allLayersAreFixed(backgroundLayers());
}
-bool RenderStyle::hasAnyLocalBackground() const
-{
- for (auto* layer = &backgroundLayers(); layer; layer = layer->next()) {
- if (layer->image() && layer->attachment() == FillAttachment::LocalBackground)
- return true;
- }
- return false;
-}
-
const CounterDirectiveMap* RenderStyle::counterDirectives() const
{
return m_rareNonInheritedData->counterDirectives.get();
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (290784 => 290785)
--- trunk/Source/WebCore/rendering/style/RenderStyle.h 2022-03-03 18:20:43 UTC (rev 290784)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h 2022-03-03 18:25:57 UTC (rev 290785)
@@ -211,11 +211,11 @@
bool hasMarginBeforeQuirk() const { return marginBefore().hasQuirk(); }
bool hasMarginAfterQuirk() const { return marginAfter().hasQuirk(); }
- bool hasBackgroundImage() const { return m_backgroundData->background->hasImage(); }
- bool hasFixedBackgroundImage() const { return m_backgroundData->background->hasFixedImage(); }
+ bool hasBackgroundImage() const { return backgroundLayers().hasImage(); }
+ bool hasAnyFixedBackground() const { return backgroundLayers().hasImageWithAttachment(FillAttachment::FixedBackground); }
bool hasEntirelyFixedBackground() const;
- bool hasAnyLocalBackground() const;
+ bool hasAnyLocalBackground() const { return backgroundLayers().hasImageWithAttachment(FillAttachment::LocalBackground); }
bool hasAppearance() const { return appearance() != NoControlPart; }
bool hasEffectiveAppearance() const { return effectiveAppearance() != NoControlPart; }
