[html-in-canvas] Give canvas subtree layers ids and display items [chromium/src : main]
0 views
Skip to first unread message
Philip Rogers (Gerrit)
Jan 22, 2026, 1:03:15 PMJan 22
to Stefan Zager, Chromium LUCI CQ, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Attention needed from Stefan Zager
Philip Rogers voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Stefan Zager
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stefan Zager (Gerrit)
Jan 22, 2026, 2:21:21 PMJan 22
to Philip Rogers, Chromium LUCI CQ, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Attention needed from Philip Rogers
Stefan Zager added 3 comments![]( "Open in Gerrit")
File third_party/blink/renderer/core/paint/compositing/compositing_test.cc
Line 3995, Patchset 11:
Stefan Zager . unresolved
Can you add a scenario where an entire subtree is moved from the canvas to the body, and make sure all the `canvas_subtree_id()` values get cleared?
Line 4031, Patchset 11:
auto* nested_child_layer = CcLayerByDOMElementId("nested_child");Stefan Zager . unresolved
Can you test `nested_canvas` as well?
File third_party/blink/renderer/core/paint/paint_layer_painter.cc
Line 407, Patchset 11:
if (canvas && element->IsInCanvasSubtree() && canvas->layoutSubtree()) {Stefan Zager . unresolved
`IsInCanvasSubtree()` is redundant/unnecessary here.
If you want to optimize, this might be best since `IsInCanvasSubtree()` is fast and will fail for most content:
```
auto* element = DynamicTo<Element>(object.GetNode());
if (element && element->IsInCanvasSubtree()) [[unlikely]] {
auto* canvas = DynamicTo<HTMLCanvasElement>(element->parentElement());
if (canvas && canvas->layoutSubtree()) {
// ...
}
}
```
Related details
Attention is currently required from:
- Philip Rogers
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
Jan 22, 2026, 2:37:50 PMJan 22
to Stefan Zager, Chromium LUCI CQ, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Attention needed from Stefan Zager
Philip Rogers voted and added 3 comments![]( "Open in Gerrit")
Votes added by Philip Rogers
| Auto-Submit | +1 |
| Commit-Queue | +1 |
3 comments
File third_party/blink/renderer/core/paint/compositing/compositing_test.cc
Can you add a scenario where an entire subtree is moved from the canvas to the body, and make sure all the `canvas_subtree_id()` values get cleared?
Philip Rogers
Done. I went with removing the layoutsubtree attribute.
Line 4031, Patchset 11:
auto* nested_child_layer = CcLayerByDOMElementId("nested_child");Stefan Zager . resolved
Can you test `nested_canvas` as well?
Philip Rogers
Done
File third_party/blink/renderer/core/paint/paint_layer_painter.cc
Line 407, Patchset 11:
if (canvas && element->IsInCanvasSubtree() && canvas->layoutSubtree()) {Stefan Zager . resolved
`IsInCanvasSubtree()` is redundant/unnecessary here.
If you want to optimize, this might be best since `IsInCanvasSubtree()` is fast and will fail for most content:
```
auto* element = DynamicTo<Element>(object.GetNode());
if (element && element->IsInCanvasSubtree()) [[unlikely]] {
auto* canvas = DynamicTo<HTMLCanvasElement>(element->parentElement());
if (canvas && canvas->layoutSubtree()) {
// ...
}
}
```
Attention is currently required from:
- Stefan Zager
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
Jan 22, 2026, 3:05:35 PMJan 22
to Stefan Zager, Chromium LUCI CQ, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Attention needed from Stefan Zager
Philip Rogers voted![]( "Open in Gerrit")
| Auto-Submit | +1 |
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Stefan Zager
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
Jan 22, 2026, 4:21:29 PMJan 22
to Stefan Zager, Chromium LUCI CQ, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Attention needed from Stefan Zager
Philip Rogers voted and added 1 comment![]( "Open in Gerrit")
Votes added by Philip Rogers
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
File third_party/blink/renderer/core/paint/compositing/compositing_test.cc
Line 4031, Patchset 11:
auto* nested_child_layer = CcLayerByDOMElementId("nested_child");Stefan Zager . resolved
Philip RogersCan you test `nested_canvas` as well?
Done
Philip Rogers
Update: it is complex to get canvas to create a cc layer for itself (see: third_party/blink/renderer/core/paint/html_canvas_painter_test.cc). I'd like to not test that for now.
Related details
Attention is currently required from:
- Stefan Zager
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stefan Zager (Gerrit)
Jan 22, 2026, 5:33:12 PMJan 22
to Philip Rogers, Chromium LUCI CQ, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Attention needed from Philip Rogers
Stefan Zager voted and added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Philip Rogers
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
Jan 22, 2026, 6:17:37 PMJan 22
to Stefan Zager, Chromium LUCI CQ, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Philip Rogers voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jan 22, 2026, 6:21:13 PMJan 22
to Philip Rogers, Stefan Zager, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[html-in-canvas] Give canvas subtree layers ids and display items
This patch has three changes:
1. Set canvas_subtree_id on cc::Layer. This is done by adding a canvas
subtree id scope in PaintLayerPainter, and plumbing this through paint
and onto the cc::Layer.
2. Set cc::Layer draws_content to false if a layer has a canvas subtree
id. This causes the layers to not paint, even if they have display
items.
3. Ensure cc::Layers have display items. Now that #2 hides layers with
display items, this patch removes the effectively invisible logic added
in https://crrev.com/1572581 which prevented cc::Layers from having
display items.
Fixed: 475579174
Change-Id: Iadaef4c10e0d597d94b4136cbcbbabc640df7d41
Auto-Submit: Philip Rogers <p...@chromium.org>
Reviewed-by: Stefan Zager <sza...@chromium.org>
Commit-Queue: Philip Rogers <p...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1573347}
Files:
- M cc/layers/layer.cc
- M cc/layers/layer.h
- M cc/layers/layer_impl.h
- M third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc
- M third_party/blink/renderer/core/paint/compositing/compositing_test.cc
- M third_party/blink/renderer/core/paint/paint_flags.h
- M third_party/blink/renderer/core/paint/paint_layer_painter.cc
- M third_party/blink/renderer/platform/BUILD.gn
- M third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc
- M third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.h
- M third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer_test.cc
- M third_party/blink/renderer/platform/graphics/compositing/pending_layer.cc
- M third_party/blink/renderer/platform/graphics/compositing/pending_layer.h
- M third_party/blink/renderer/platform/graphics/paint/paint_chunk.cc
- M third_party/blink/renderer/platform/graphics/paint/paint_chunk.h
- M third_party/blink/renderer/platform/graphics/paint/paint_chunker.cc
- M third_party/blink/renderer/platform/graphics/paint/paint_chunker.h
- M third_party/blink/renderer/platform/graphics/paint/paint_controller.h
- A third_party/blink/renderer/platform/graphics/paint/scoped_canvas_subtree_id.h
- M third_party/blink/web_tests/TestExpectations
Change size: L
Delta: 20 files changed, 336 insertions(+), 40 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Stefan Zager
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
Jan 30, 2026, 8:02:46 PMJan 30
to Chromium LUCI CQ, Stefan Zager, Dirk Schulze, Stephen Chenney, AyeAye, chromium...@chromium.org, kinuko...@chromium.org, blink-reviews-p...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, cc-...@chromium.org
Philip Rogers has created a revert of this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages