[HIC] Create composited layers for canvas descendants [chromium/src : main]
Philip Rogers (Gerrit)
Philip Rogers added 2 comments![]( "Open in Gerrit")
if (element && element->IsInCanvasSubtree()) {`IsInCanvasSubtree` is general and is set even without the feature enabled. Can you wrap this logic in:
```
if (RuntimeEnabledFeatures::CanvasDrawElementEnabled()) {
auto* element = DynamicTo<Element>(object.GetNode());
if (element && element->IsInCanvasSubtree()) {
return true;
}
```
auto* element = DynamicTo<Element>(object.GetNode());Nit: maybe just wrap all of this logic in the feature check, rather than only checking the feature flag on line 400?
Related details
- Stefan Zager
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)
Philip Rogers added 1 comment![]( "Open in Gerrit")
// EXPECT_FALSE(CcLayerByDOMElementId("grandchild_a"));nit: remove (the uncommented version is below)
Related details
- Stefan Zager
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)
Philip Rogers added 1 comment![]( "Open in Gerrit")
if (is_drawn_canvas_child || PaintedOutputInvisible(object.StyleRef())) {I think some of the test failures (e.g., svg_rect.html) are due to using effectively invisible. It causes us to not render this content when we call drawElementImage, which uses PaintChunksToCCLayer, which calls ConversionContext<Result>::Convert, which skips effectively invisible chunks.
WDYT of making cc::Layer SetDrawsContent false as an alternative? We can do this by modifying Layer::PushDirtyPropertiesTo to check if the layer is in the layer tree host's list of draw element layer ids, and if so, set draws_content to false.
Stefan Zager (Gerrit)
Stefan Zager added 4 comments![]( "Open in Gerrit")
// EXPECT_FALSE(CcLayerByDOMElementId("grandchild_a"));nit: remove (the uncommented version is below)
Done
`IsInCanvasSubtree` is general and is set even without the feature enabled. Can you wrap this logic in:
```
if (RuntimeEnabledFeatures::CanvasDrawElementEnabled()) {
auto* element = DynamicTo<Element>(object.GetNode());
if (element && element->IsInCanvasSubtree()) {
return true;
}
```
Done
Nit: maybe just wrap all of this logic in the feature check, rather than only checking the feature flag on line 400?
Done
if (is_drawn_canvas_child || PaintedOutputInvisible(object.StyleRef())) {I think some of the test failures (e.g., svg_rect.html) are due to using effectively invisible. It causes us to not render this content when we call drawElementImage, which uses PaintChunksToCCLayer, which calls ConversionContext<Result>::Convert, which skips effectively invisible chunks.
WDYT of making cc::Layer SetDrawsContent false as an alternative? We can do this by modifying Layer::PushDirtyPropertiesTo to check if the layer is in the layer tree host's list of draw element layer ids, and if so, set draws_content to false.
The problem is that we shouldn't emplace `effectively_invisible` for the `PaintLayerPainter::Paint` call from `CanvasRenderingContext`. I fixed this by checking for `PaintFlag::kPrivacyPreserving`, which is only set from that call site. That's pretty hacky, I'm open to other ideas.
Related details
- Philip Rogers
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)
Philip Rogers voted and added 2 comments![]( "Open in Gerrit")
Votes added by Philip Rogers
| Code-Review | +1 |
2 comments
if (is_drawn_canvas_child || PaintedOutputInvisible(object.StyleRef())) {Stefan ZagerI think some of the test failures (e.g., svg_rect.html) are due to using effectively invisible. It causes us to not render this content when we call drawElementImage, which uses PaintChunksToCCLayer, which calls ConversionContext<Result>::Convert, which skips effectively invisible chunks.
WDYT of making cc::Layer SetDrawsContent false as an alternative? We can do this by modifying Layer::PushDirtyPropertiesTo to check if the layer is in the layer tree host's list of draw element layer ids, and if so, set draws_content to false.
The problem is that we shouldn't emplace `effectively_invisible` for the `PaintLayerPainter::Paint` call from `CanvasRenderingContext`. I fixed this by checking for `PaintFlag::kPrivacyPreserving`, which is only set from that call site. That's pretty hacky, I'm open to other ideas.
- Stefan Zager
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |