New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 819057 link

Starred by 1 user
Status: Fixed
Owner:
riajiang@chromium.org
Closed: Mar 2018
Cc:
rjkroege@chromium.org
sadrul@chromium.org
█ gklassen@chromium.org
kylec...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

HitTestAggregator

Project Member Reported by fsam...@chromium.org, Mar 6 2018 
Currently, HitTestRegion optionally takes in a LocalSurfaceId from the parent.

Unfortunately, this is a bit racy, because the primary Surface might not be available at time of aggregation. Instead, either the parent specifies both the fallback and primary LocalSurfaceId or we simply ask SurfaceAggregator which surfaces it included at aggregation time.

For example, SurfaceAggregator has a previous_contained_surfaces map that HitTestAggregator might use. It's currently keyed on full SurfaceIds but a simple refactor could allow us to look up by FrameSinkId.

Comment 1 by riajiang@chromium.org, Mar 8 2018

Cc: -riajiang@chromium.org gklassen@chromium.org kylec...@chromium.org sadrul@chromium.org
Components: -Blink>HitTesting Internals>Services>Viz
Labels: event-targeting OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Owner: riajiang@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 16 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95

commit 57c26837a9a7ab1a0d6fec90192a0d68d98a7b95
Author: Ria Jiang <riajiang@chromium.org>
Date: Fri Mar 16 19:21:52 2018

Use the latest LocalSurfaceId at aggregation time and prevent cycles in HTA.

1. Clients used to submit local_surface_id in hit-test data; however, this can be
racy because we have primary_surface_id and fallback_surface_id and we can use
any of the latest one in that range. Change it to not using local_surface_id
submitted from clients, but instead asking SurfaceAggregator at HitTestAggregator
::Aggregator() time for the latest local_surface_id to use - this is done by
adding LatestLocalSurfaceIdLookupDelegate in surfaces to talk to viz::Display,
where we have access to the surfaces SurfaceAggregator used at aggregation time.
Add FrameSinkIdMap in SurfaceAggregator to keep track of previously contained
frame sinks.

2. Keep track of the list of child regions we've aggregated so far in
HitTestAggregator to prevent cycles.

3. When preparing hit-test data, skip the quad if its FrameSinkId has changed
between fallback and primary because we don't know which FrameSinkId would be used
to draw this quad.

4. Add viz::TestLatestLocalSurfaceIdLookupDelegate to be used in tests.

Bug:  819057 ,  819435 
Test: viz_unittests, content_unittests

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I35ec40ae00af1ba4e3937002c1c0d261dae8b136
Reviewed-on: https://chromium-review.googlesource.com/957881
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543778}
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/client/hit_test_data_provider_draw_quad.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/client/hit_test_data_provider_draw_quad_unittest.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/host/host_frame_sink_manager_unittest.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/BUILD.gn
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/display/display.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/display/display.h
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/display/surface_aggregator.h
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/display/surface_aggregator_unittest.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/frame_sinks/compositor_frame_sink_support.h
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/frame_sinks/direct_layer_tree_frame_sink_unittest.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/hit_test/hit_test_aggregator.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/hit_test/hit_test_aggregator.h
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/hit_test/hit_test_aggregator_unittest.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/hit_test/hit_test_manager.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/hit_test/hit_test_manager.h
[add] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/service/surfaces/latest_local_surface_id_lookup_delegate.h
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/test/BUILD.gn
[add] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/test/test_latest_local_surface_id_lookup_delegate.cc
[add] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/components/viz/test/test_latest_local_surface_id_lookup_delegate.h
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/services/viz/public/interfaces/hit_test/hit_test_region_list.mojom
[modify] https://crrev.com/57c26837a9a7ab1a0d6fec90192a0d68d98a7b95/ui/aura/hit_test_data_provider_aura.cc

Comment 3 by riajiang@chromium.org, Mar 27 2018

Status: Fixed (was: Started)
►

Sign in to add a comment