Skip to content

Canvas with relative width does not repaint correctly on parent resize #44463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
webkit-commit-queue merged 1 commit into from
May 1, 2025
Merged

Canvas with relative width does not repaint correctly on parent resize #44463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions LayoutTests/compositing/tiling/huge-layer-canvas-resize-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
(repaint rects
(rect 0 0 3400 2000)
(rect 2500 0 900 2028)
(rect 2500 0 900 2041)
)
(GraphicsLayer
(anchor 0.00 0.00)
(bounds 3400.00 2041.00)
(children 1
(GraphicsLayer
(bounds 3400.00 2041.00)
(contentsOpaque 1)
(children 1
(GraphicsLayer
(bounds 3400.00 2000.00)
(children 1
(GraphicsLayer
(bounds 3400.00 2000.00)
(usingTiledLayer 1)
(drawsContent 1)
(repaint rects
(rect 0.00 0.00 2500.00 2000.00)
)
)
)
)
)
)
)
)

75 changes: 75 additions & 0 deletions LayoutTests/compositing/tiling/huge-layer-canvas-resize.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<!DOCTYPE html>
<head>
<style>
body {
margin: 0;
}
#container {
width: 2500px;
height: 2000px;
}
#layer {
width: 100%;
height: 100%;
-webkit-transform:translateZ(0);
}
#target {
display: block;
width: 100%;
height: 100%;
}
</style>
<script type="text/javascript" charset="utf-8">
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

function testOnLoad()
{
var canvas = document.getElementById('target');
var context = canvas.getContext('2d');

const gradient = context.createLinearGradient(0, 0, canvas.width, 0);
gradient.addColorStop(0, "white");
gradient.addColorStop(1, "black");
context.fillStyle = gradient;
context.fillRect(0, 0, canvas.width, canvas.height);
context.moveTo(0, canvas.height);
context.lineTo(canvas.width, 0);
context.moveTo(0, 0);
context.lineTo(canvas.width, canvas.height);
context.lineWidth = 20;
context.strokeStyle = "red";
context.stroke();

setTimeout(() => {
if (window.internals)
window.internals.startTrackingRepaints();
document.getElementById('container').style.width = "3400px";
if (window.internals) {
document.getElementById('layers').textContent = window.internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_REPAINT_RECTS);
window.internals.stopTrackingRepaints();
}
if (window.testRunner)
testRunner.notifyDone();
}, 1000);
Copy link
Contributor

@shallawa shallawa May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly are you waiting 1000ms for? This makes the behavior of the test un-deterministic. Besides one second is too long time to wait. Can not it be replaced by

requestAnimationFrame(() => {
    ...
});

Copy link
Contributor Author

@cathiechen cathiechen May 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make sure the repaint rects before css change has flashed, so I added 1000 ms. requestAnimationFrame seems not long enough, especially in stress mode.

Copy link
Contributor Author

@cathiechen cathiechen May 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rego suggested me to use two requestAnimationFrame, it works! I have created a PR to address the code review comments. Please take a look, thanks:) #45040

}

window.addEventListener('load', testOnLoad, false);
</script>
</head>
<body>
<!--
If the size of canvas changes, it should trigger full repaint.
The repaint issue is only revealed on tiled layer, for untiled layer,
a full repaint is triggered by GraphicsLayerCA::shouldRepaintOnSizeChange.
-->
<div id="container">
<div id="layer">
<canvas id="target" width="2500" height="2000"></canvas>
</div>
</div>
<pre id="layers">Layer tree including repaint rects appears here in DRT.</pre>
</body>
</html>
27 changes: 27 additions & 0 deletions LayoutTests/compositing/tiling/huge-layer-img-resize-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

(repaint rects
(rect 0 0 3400 2000)
(rect 2500 0 900 2028)
(rect 2500 0 900 2041)
)
(GraphicsLayer
(anchor 0.00 0.00)
(bounds 3400.00 2041.00)
(children 1
(GraphicsLayer
(bounds 3400.00 2041.00)
(contentsOpaque 1)
(children 1
(GraphicsLayer
(bounds 3400.00 2000.00)
(usingTiledLayer 1)
(drawsContent 1)
(repaint rects
(rect 0.00 0.00 2500.00 2000.00)
)
)
)
)
)
)

83 changes: 83 additions & 0 deletions LayoutTests/compositing/tiling/huge-layer-img-resize.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<!DOCTYPE html>
<head>
<style>
body {
margin: 0;
}
#container {
width: 2500px;
height: 2000px;
}
#layer {
width: 100%;
height: 100%;
-webkit-transform:translateZ(0);
}
img {
display: block;
width: 100%;
height: 100%;
}
</style>
<script type="text/javascript" charset="utf-8">
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

function testOnLoad()
{
var canvas = document.createElement('canvas');
var img = document.getElementById("image");
canvas.width = img.width;
canvas.height = img.height;
var context = canvas.getContext('2d');

const gradient = context.createLinearGradient(0, 0, canvas.width, 0);
gradient.addColorStop(0, "white");
gradient.addColorStop(1, "black");
context.fillStyle = gradient;
context.fillRect(0, 0, canvas.width, canvas.height);
context.moveTo(0, canvas.height);
context.lineTo(canvas.width, 0);
context.moveTo(0, 0);
context.lineTo(canvas.width, canvas.height);
context.lineWidth = 20;
context.strokeStyle = "red";
context.stroke();

img.src = canvas.toDataURL();
}

async function onImageLoad()
Copy link
Contributor

@shallawa shallawa May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this function marked as async? It does not have await statement.

{
setTimeout(() => {
if (window.internals)
window.internals.startTrackingRepaints();
document.getElementById('container').style.width = "3400px";
if (window.internals) {
document.getElementById('layers').textContent = window.internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_REPAINT_RECTS);
window.internals.stopTrackingRepaints();
}
if (window.testRunner)
testRunner.notifyDone();
}, 1000);
}

window.addEventListener('load', testOnLoad, false);
</script>
</head>
<body>
<!--
If the size of image changes, it should trigger full repaint.
The repaint issue is only revealed on tiled layer, for untiled layer,
a full repaint is triggered by GraphicsLayerCA::shouldRepaintOnSizeChange.
-->
<div id="container">
<div id="layer">
<img onload="onImageLoad()" id="image">
</div>
</div>
<pre id="layers">Layer tree including repaint rects appears here in DRT.</pre>
</body>
</html>
2 changes: 2 additions & 0 deletions LayoutTests/platform/glib/TestExpectations
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4093,6 +4093,8 @@ webkit.org/b/169918 compositing/text-on-scaled-surface.html [ ImageOnlyFailure ]
webkit.org/b/169918 compositing/tiling/backface-preserve-3d-tiled.html [ Failure ]
webkit.org/b/210850 compositing/tiling/coverage-adjustment-secondary-quad-mapping.html [ Failure ]
webkit.org/b/169918 compositing/tiling/crash-reparent-tiled-layer.html [ Failure ]
webkit.org/b/169918 compositing/tiling/huge-layer-canvas-resize.html [ Failure ]
webkit.org/b/169918 compositing/tiling/huge-layer-img-resize.html [ Failure ]
webkit.org/b/169918 compositing/tiling/huge-layer-img.html [ Failure ]
webkit.org/b/169918 compositing/tiling/huge-layer-with-layer-children-resize.html [ Failure ]
webkit.org/b/169918 compositing/tiling/huge-layer-with-layer-children.html [ Failure ]
Expand Down
31 changes: 31 additions & 0 deletions LayoutTests/platform/ios/compositing/tiling/huge-layer-canvas-resize-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
(repaint rects
(rect 0 0 3400 2000)
(rect 2500 0 900 2027)
(rect 2500 0 900 2040)
)
(GraphicsLayer
(anchor 0.00 0.00)
(bounds 3400.00 2040.00)
(children 1
(GraphicsLayer
(bounds 3400.00 2040.00)
(contentsOpaque 1)
(children 1
(GraphicsLayer
(bounds 3400.00 2000.00)
(children 1
(GraphicsLayer
(bounds 3400.00 2000.00)
(usingTiledLayer 1)
(drawsContent 1)
(repaint rects
(rect 0.00 0.00 2500.00 2000.00)
)
)
)
)
)
)
)
)

27 changes: 27 additions & 0 deletions LayoutTests/platform/ios/compositing/tiling/huge-layer-img-resize-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

(repaint rects
(rect 0 0 3400 2000)
(rect 2500 0 900 2027)
(rect 2500 0 900 2040)
)
(GraphicsLayer
(anchor 0.00 0.00)
(bounds 3400.00 2040.00)
(children 1
(GraphicsLayer
(bounds 3400.00 2040.00)
(contentsOpaque 1)
(children 1
(GraphicsLayer
(bounds 3400.00 2000.00)
(usingTiledLayer 1)
(drawsContent 1)
(repaint rects
(rect 0.00 0.00 2500.00 2000.00)
)
)
)
)
)
)

2 changes: 2 additions & 0 deletions LayoutTests/platform/win/TestExpectations
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4179,6 +4179,8 @@ compositing/tiling/backface-preserve-3d-tiled.html [ Skip ]
compositing/tiling/coverage-adjustment-secondary-quad-mapping.html [ Skip ]
compositing/tiling/crash-reparent-tiled-layer.html [ Skip ]
compositing/tiling/huge-layer-add-remove-child.html [ Skip ]
compositing/tiling/huge-layer-canvas-resize.html [ Skip ]
compositing/tiling/huge-layer-img-resize.html [ Skip ]
compositing/tiling/huge-layer-img.html [ Skip ]
compositing/tiling/huge-layer-with-layer-children-resize.html [ Skip ]
compositing/tiling/huge-layer-with-layer-children.html [ Skip ]
Expand Down
19 changes: 17 additions & 2 deletions Source/WebCore/rendering/RenderReplaced.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "RenderElementInlines.h"
#include "RenderFlexibleBox.h"
#include "RenderFragmentedFlow.h"
#include "RenderHTMLCanvas.h"
#include "RenderHighlight.h"
#include "RenderImage.h"
#include "RenderLayer.h"
Expand Down Expand Up @@ -114,6 +115,19 @@ void RenderReplaced::styleDidChange(StyleDifference diff, const RenderStyle* old
intrinsicSizeChanged();
}

static void issueFullRepaintOnSizeChangeIfNeeded(RenderReplaced& renderer)
{
auto shouldRepaintOnSizeChange = [&] {
if (is<RenderHTMLCanvas>(renderer))
return true;
if (auto* renderImage = dynamicDowncast<RenderImage>(renderer); renderImage && !is<RenderMedia>(*renderImage) && !renderImage->isShowingMissingOrImageError())
return true;
return false;
};
if (shouldRepaintOnSizeChange())
renderer.repaint();
}

void RenderReplaced::layout()
{
StackStats::LayoutCheckPoint layoutCheckPoint;
Expand All @@ -132,12 +146,13 @@ void RenderReplaced::layout()
addVisualEffectOverflow();
updateLayerTransform();
invalidateBackgroundObscurationStatus();

repainter.repaintAfterLayout();
clearNeedsLayout();

if (replacedContentRect() != oldContentRect)
if (replacedContentRect() != oldContentRect) {
setPreferredLogicalWidthsDirty(true);
issueFullRepaintOnSizeChangeIfNeeded(*this);
Copy link
Contributor

@shallawa shallawa May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a long name for a function. Could it be just?

  if (shouldRepaintOnSizeChange(*this))
        repaint();

}
}

void RenderReplaced::intrinsicSizeChanged()
Expand Down