[For CT only] Stat opaqueness to hit test [chromium/src : main]
Xianzhu Wang (Gerrit)
Attention is currently required from: Philip Rogers.
Xianzhu Wang would like Philip Rogers to review this change.
[For CT only] Stat opaqueness to hit test
Change-Id: Ia237422ab5ccf76aa5fca75725911276b2a8dcf9
---
M base/trace_event/builtin_categories.h
M cc/layers/layer.cc
M cc/layers/layer.h
M third_party/blink/renderer/core/exported/web_plugin_container_impl.cc
M third_party/blink/renderer/core/layout/layout_object.cc
M third_party/blink/renderer/core/layout/layout_object.h
M third_party/blink/renderer/core/paint/box_painter.cc
M third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc
M third_party/blink/renderer/core/paint/svg_model_object_painter.cc
M third_party/blink/renderer/platform/graphics/compositing/chunk_to_layer_mapper.cc
M third_party/blink/renderer/platform/graphics/compositing/chunk_to_layer_mapper.h
M third_party/blink/renderer/platform/graphics/compositing/layers_as_json.cc
M third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc
M third_party/blink/renderer/platform/graphics/paint/paint_artifact.cc
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.cc
M third_party/blink/renderer/platform/graphics/paint/paint_controller.h
20 files changed, 150 insertions(+), 36 deletions(-)
To view, visit change 4596916. To unsubscribe, or for help writing mail filters, visit settings.
Xianzhu Wang (Gerrit)
Attention is currently required from: Philip Rogers.
1 comment:
Patchset:
Philip, can you review this patch (for CT only)? I would like to have another pair of eyes to verify if the collected data is trustful.
Here is the collected data: https://docs.google.com/spreadsheets/d/1em7VVS8TGr9VJc4YE-doYqAthqggZ67vww9h0IL3bsc/edit#gid=0.
This doesn't collect data for real hit tests. Do you think the results are good for us to move forward? I would like to land some of the code into production and collect real world hit test data.
To view, visit change 4596916. To unsubscribe, or for help writing mail filters, visit settings.
Philip Rogers (Gerrit)
Attention is currently required from: Xianzhu Wang.
3 comments:
Patchset:
Philip, can you review this patch (for CT only)? I would like to have another pair of eyes to verify […]
The comment descriptions in the spreadsheet helped me a lot.
Do you have a takeaway from the current data? One thing I noticed is "a/layer-opaque-hit-area/value" is high. Do you think this indicates enough of an opportunity to try considering opaque-for-hit-testing layers as reliable? Or do you think it would be better to add this plumbing for the purposes of metrics first?
Because of all of the non-opaque-for-hit-test reasons (border radius, pointer events, non-composited clips, etc), I am a little worried that a fully generic solution here would be complex. Would it be worth looking at some of the pages identified in the data to see if there is a pattern?
Because "a/layer-not-opaque-hit-squash-area/value" is small, can we conclude that optimizations to avoid squashing non-hit-test-opaque chunks are not worth pursuing? Initially, I thought yes, but there are many chunks per layer ("a/chunk-opaque-hit/value" + "a/chunk-not-opaque-hit/value" vs "a/layer-opaque-hit/value" + "a/layer-not-opaque-hit-chunk/value"), and chunks seem to be only ~50% opaque to hit testing, so a low ratio is expected. Can we conclude anything about the potential in pursuing optimizations to avoid squashing non-hit-test-opaque chunks?
File third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc:
Patch Set #4, Line 1157:
if (layer_.bounds().IsEmpty() ||This will over-count frequently-repainted layers. Do you think it would be worth trying a snapshot approach where the analysis is run once per page by looping over all layers after loading has completed?
If you want to try this, one hacky way is to add a sleep(40) in ValidateAndMeasurePage in "tools/perf/contrib/cluster_telemetry/generic_trace.py" to make the page wait quite a while, and then do the analysis for local frame roots in Document::ImplicitClose + a timer.
File third_party/blink/renderer/platform/graphics/paint/paint_chunker.cc:
Patch Set #4, Line 180:
if (opaque_to_hit_test) {What if we have a paint chunk with a solid background (opaque to hit testing) and a child with pointer-events: none? Will this result in a paint chunk that is considered opaque to hit testing, even though the pointer-events: none should make it non-opaque?
To view, visit change 4596916. To unsubscribe, or for help writing mail filters, visit settings.
Xianzhu Wang (Gerrit)
Attention is currently required from: Philip Rogers.
3 comments:
Patchset:
Do you have a takeaway from the current data? One thing I noticed is "a/layer-opaque-hit-area/value" is high. Do you think this indicates enough of an opportunity to try considering opaque-for-hit-testing layers as reliable? Or do you think it would be better to add this plumbing for the purposes of metrics first?
I think that indicates a potential opportunity, but I would like to get some real world data with UMA metrics first. The CT data are not properly weighed for real world scroll hit tests.
> Because of all of the non-opaque-for-hit-test reasons (border radius, pointer events, non-composited clips, etc), I am a little worried that a fully generic solution here would be complex. Would it be worth looking at some of the pages identified in the data to see if there is a pattern?
For opaqueness-for-hit-test of paint chunks, we only need to consider the reasons not caused by paint properties. Border radius is considered because the background is clipped using paint operations instead of paint properties during painting.
Non-opaque-for-hit-test caused by paint properties will cause the result of GeometryMapper::LocalToAncestorVisualRect() to be not tight, which affects opaqueness-for-hit-test of the cc::Layer during PAC update.
Yes, I think it will be worth looking at some pages.
Because "a/layer-not-opaque-hit-squash-area/value" is small, can we conclude that optimizations to avoid squashing non-hit-test-opaque chunks are not worth pursuing? Initially, I thought yes, but there are many chunks per layer ("a/chunk-opaque-hit/value" + "a/chunk-not-opaque-hit/value" vs "a/layer-opaque-hit/value" + "a/layer-not-opaque-hit-chunk/value"), and chunks seem to be only ~50% opaque to hit testing, so a low ratio is expected. Can we conclude anything about the potential in pursuing optimizations to avoid squashing non-hit-test-opaque chunks?
The current data may be not enough for us to conclude. I can experiment and get more data.
File third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc:
Patch Set #4, Line 1157:
if (layer_.bounds().IsEmpty() ||
This will over-count frequently-repainted layers. […]
I added g_doing_full_update to exclude repaint-only updates to avoid the over-count for some layers. It will still over-count all layers in a frequently PAC-updated pages, which gives more weight for such pages, but this doesn't seem to make the already improper weight issue of CT (compared to UMA) much worse.
File third_party/blink/renderer/platform/graphics/paint/paint_chunker.cc:
Patch Set #4, Line 180:
if (opaque_to_hit_test) {
What if we have a paint chunk with a solid background (opaque to hit testing) and a child with point […]
I thought this was a problem, but I verified that pointer-events:none just makes the element itself transparent to hit test, but doesn't affect opaque-for-hit-test of parent elements (thus doesn't affect the existing opaque-for-hit-test of the paint chunk).
To view, visit change 4596916. To unsubscribe, or for help writing mail filters, visit settings.
Philip Rogers (Gerrit)
Attention is currently required from: Xianzhu Wang.
3 comments:
Patchset:
Because of all of the non-opaque-for-hit-test reasons (border radius, pointer events, non-composited clips, etc), I am a little worried that a fully generic solution here would be complex. Would it be worth looking at some of the pages identified in the data to see if there is a pattern?
For opaqueness-for-hit-test of paint chunks, we only need to consider the reasons not caused by paint properties. Border radius is considered because the background is clipped using paint operations instead of paint properties during painting.
Non-opaque-for-hit-test caused by paint properties will cause the result of GeometryMapper::LocalToAncestorVisualRect() to be not tight, which affects opaqueness-for-hit-test of the cc::Layer during PAC update.
Before your comment, I did not really understand that the non-tightness will handle the non-composited cases. A solution here does seem easier than I had expected.
File third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc:
Patch Set #4, Line 1157:
if (layer_.bounds().IsEmpty() ||
I added g_doing_full_update to exclude repaint-only updates to avoid the over-count for some layers. […]
Done
File third_party/blink/renderer/platform/graphics/paint/paint_chunker.cc:
Patch Set #4, Line 180:
if (opaque_to_hit_test) {
I thought this was a problem, but I verified that pointer-events:none just makes the element itself […]
ahh, good point!
To view, visit change 4596916. To unsubscribe, or for help writing mail filters, visit settings.
Mazlum Gulnaz (Gerrit)
Attention is currently required from: Xianzhu Wang.
1 comment:
Patchset:
هم لا شي يعجب
To view, visit change 4596916. To unsubscribe, or for help writing mail filters, visit settings.