Skip to content

Commit

Permalink
[LBSE] Fix MotionMark repaint issues
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=270103

Reviewed by Rob Buis.

In LBSE transform changes do not result in relayouts, instead repaints
are triggered. That failed to repaint the old position of an object
before the transform change, only the new position - fix that.

Covered by existing tests, when LBSE is active.

* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/js-late-marker-and-object-creation-expected.png:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/js-late-marker-creation-expected.png:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/js-late-pattern-and-object-creation-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/marker-orient-auto-expected.png:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/non-scaling-stroke-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/pattern-deep-referencing-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/pattern-incorrect-tiling-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/pattern-referencing-preserve-aspect-ratio-expected.png: Copied from LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/js-late-marker-and-object-creation-expected.png.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/pattern-rotate-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/pattern-scaled-pattern-space-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/pattern-scaling-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/pattern-skew-transformed-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/pattern-with-transformation-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/shapes-supporting-markers-expected.png:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/transformed-pattern-clamp-svg-root-expected.png: Added.
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/repaint/foreign-object-repainting-after-modifying-container-transform-repaint-rects-expected.txt:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/repaint/foreign-object-repainting-after-modifying-transform-repaint-rects-expected.txt:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/repaint/image-repainting-after-modifying-container-transform-repaint-rects-expected.txt:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/repaint/image-repainting-after-modifying-transform-repaint-rects-expected.txt:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/repaint/shape-repainting-after-modifying-container-transform-repaint-rects-expected.txt:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/repaint/shape-repainting-after-modifying-transform-repaint-rects-expected.txt:
* Source/WebCore/rendering/ReferencedSVGResources.cpp:
(WebCore::CSSSVGResourceElementClient::resourceChanged):
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::repaintRendererOrClientsOfReferencedSVGResources const):
(WebCore::RenderElement::repaintOldAndNewPositionsForSVGRenderer const):
* Source/WebCore/rendering/RenderElement.h:

Canonical link: https://commits.webkit.org/275702@main
  • Loading branch information
nikolaszimmermann authored and Ahmad Saleem committed Mar 5, 2024
1 parent 0960edb commit a78e4b3
Show file tree
Hide file tree
Showing 24 changed files with 47 additions and 29 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
PASS
(repaint rects
(rect 0 0 1 1)
(rect 50 13 100 100)
)

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
PASS
(repaint rects
(rect 0 0 1 1)
(rect 50 13 100 100)
)

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
(repaint rects
(rect 0 0 1 1)
(rect 50 50 100 100)
)

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
(repaint rects
(rect 0 0 1 1)
(rect 50 50 100 100)
)

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
(repaint rects
(rect 0 0 1 1)
(rect 50 50 100 100)
)

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
(repaint rects
(rect 0 0 1 1)
(rect 50 50 100 100)
)

36 changes: 8 additions & 28 deletions Source/WebCore/rendering/ReferencedSVGResources.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,42 +66,22 @@ void CSSSVGResourceElementClient::resourceChanged(SVGElement& element)
return;

#if ENABLE(LAYER_BASED_SVG_ENGINE)
if (!m_clientRenderer.document().settings().layerBasedSVGEngineEnabled()) {
m_clientRenderer.repaint();
return;
}

// Special case for markers. Markers can be attached to RenderSVGPath object. Marker positions are computed
// once during layout, or if the shape itself changes. Here we manually update the marker positions without
// requiring a relayout. Instead we can simply repaint the path - via the updateLayerPosition() logic, properly
// repainting the old repaint boundaries and the new ones (after the marker change).
if (auto* pathClientRenderer = dynamicDowncast<RenderSVGPath>(m_clientRenderer); pathClientRenderer && is<SVGMarkerElement>(element))
pathClientRenderer->updateMarkerPositions();

auto useUpdateLayerPositionsLogic = [&]() -> std::optional<CheckedPtr<RenderLayer>> {
auto& document = m_clientRenderer.document();
if (!document.settings().layerBasedSVGEngineEnabled())
return std::nullopt;

// Don't attempt to update anything during layout - the post-layout phase will invoke RenderLayer::updateLayerPosition(), if necessary.
if (document.view()->layoutContext().isInLayout())
return std::nullopt;

// If no layers are available, always use the renderer based repaint() logic.
if (!m_clientRenderer.hasLayer())
return std::nullopt;

// Use the cheaper update mechanism for all SVG renderers -- in proper subtrees, that do not need layout themselves.
if (!m_clientRenderer.isSVGLayerAwareRenderer() || m_clientRenderer.needsLayout())
return std::nullopt;

return std::make_optional(downcast<RenderLayerModelObject>(m_clientRenderer).checkedLayer());
};

// LBSE: Instead of repainting the current boundaries, utilize RenderLayer::updateLayerPositionsAfterStyleChange() to repaint
// the old and the new repaint boundaries, if they differ -- instead of just the new boundaries.
if (auto layer = useUpdateLayerPositionsLogic()) {
(*layer.value()).updateLayerPositionsAfterStyleChange();
return;
}
#endif

m_clientRenderer.repaintOldAndNewPositionsForSVGRenderer();
#else
m_clientRenderer.repaint();
#endif
}

WTF_MAKE_ISO_ALLOCATED_IMPL(ReferencedSVGResources);
Expand Down
33 changes: 32 additions & 1 deletion Source/WebCore/rendering/RenderElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2211,7 +2211,7 @@ void RenderElement::repaintRendererOrClientsOfReferencedSVGResources() const
{
auto* enclosingResourceContainer = lineageOfType<RenderSVGResourceContainer>(*this).first();
if (!enclosingResourceContainer) {
repaint();
repaintOldAndNewPositionsForSVGRenderer();
return;
}

Expand All @@ -2227,6 +2227,37 @@ void RenderElement::repaintClientsOfReferencedSVGResources() const
if (auto* enclosingResourceContainer = lineageOfType<RenderSVGResourceContainer>(*this).first())
enclosingResourceContainer->repaintAllClients();
}

void RenderElement::repaintOldAndNewPositionsForSVGRenderer() const
{
auto useUpdateLayerPositionsLogic = [&]() -> std::optional<CheckedPtr<RenderLayer>> {
if (!document().settings().layerBasedSVGEngineEnabled())
return std::nullopt;

// Don't attempt to update anything during layout - the post-layout phase will invoke RenderLayer::updateLayerPosition(), if necessary.
if (document().view()->layoutContext().isInLayout())
return std::nullopt;

// If no layers are available, always use the renderer based repaint() logic.
if (!hasLayer())
return std::nullopt;

// Use the cheaper update mechanism for all SVG renderers -- in proper subtrees, that do not need layout themselves.
if (!isSVGLayerAwareRenderer() || needsLayout())
return std::nullopt;

return std::make_optional(downcast<RenderLayerModelObject>(*this).checkedLayer());
};

// LBSE: Instead of repainting the current boundaries, utilize RenderLayer::updateLayerPositionsAfterStyleChange() to repaint
// the old and the new repaint boundaries, if they differ -- instead of just the new boundaries.
if (auto layer = useUpdateLayerPositionsLogic()) {
(*layer.value()).updateLayerPositionsAfterStyleChange();
return;
}

repaint();
}
#endif

#if ENABLE(TEXT_AUTOSIZING)
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/rendering/RenderElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ class RenderElement : public RenderObject {
#if ENABLE(LAYER_BASED_SVG_ENGINE)
void repaintClientsOfReferencedSVGResources() const;
void repaintRendererOrClientsOfReferencedSVGResources() const;
void repaintOldAndNewPositionsForSVGRenderer() const;
#endif

bool borderImageIsLoadedAndCanBeRendered() const;
Expand Down

0 comments on commit a78e4b3

Please sign in to comment.