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

Issue 739429 link

Starred by 5 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 732656



Sign in to add a comment

Use stable id to identify SharedQuadState instead of raw pointer on DrawQuad

Project Member Reported by weiliangc@chromium.org, Jul 5 2017

Issue description

SharedQuadState is common state shared by multiple DrawQuads. With stable id we would be able to identify same SQS and DQ across frames.

In order to delete the raw pointer we need to be able to find the SQS corresponding to the DQ. SQS and DQ are in order. Caching information about stable id to SQS map would be difficult for keeping up to date.

The plan is to implement iterator that walks both DQ list and SQS list at the same time, in either front to back or back to front order and answers question that needs information from both DQ and SQS.

Steps including:
- Add stable id to both DQ and SQS.
- Remove access to SQS* from DQ whenever possible.
- Implement QuadIterator that takes in RenderPass, and use QuadIterator in rest of cases.


 
Thanks for working on this!

> and answers question that needs information from both DQ and SQS.

What questions do you mean here? Are there things you can't do if it returns both the DQ* and the SQS*?


re comment#1: mostly ShouldDrawWithBlending (checks both opacity and needs_blending_) and IsRight/BottomEdge (check DQ's rect with SQS's rect). There are multiple call sites so I want to make sure the logic is no duplicated.
Ah, perhaps those functions can take a SQS* instead of finding it themselves.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1 2017

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

commit a41ecf3a3724c9d75f79b1b9050e97f09b334938
Author: Weiliang Chen <weiliangc@chromium.org>
Date: Tue Aug 01 23:40:03 2017

cc: Add function to replace quad with solid color to reduce duplication

Add a function to replace existing quad with a solid color draw quad to
reduce duplicate code in overlay.

R=danakj@chromium.org

Bug: 739429
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I6ea4df426dde50122b73923d970ea3e6429dd8aa
Reviewed-on: https://chromium-review.googlesource.com/596647
Commit-Queue: weiliangc <weiliangc@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: John Bauman <jbauman@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491153}
[modify] https://crrev.com/a41ecf3a3724c9d75f79b1b9050e97f09b334938/cc/output/dc_layer_overlay.cc
[modify] https://crrev.com/a41ecf3a3724c9d75f79b1b9050e97f09b334938/cc/output/overlay_strategy_underlay.cc
[modify] https://crrev.com/a41ecf3a3724c9d75f79b1b9050e97f09b334938/cc/output/overlay_unittest.cc
[modify] https://crrev.com/a41ecf3a3724c9d75f79b1b9050e97f09b334938/cc/quads/render_pass.cc
[modify] https://crrev.com/a41ecf3a3724c9d75f79b1b9050e97f09b334938/cc/quads/render_pass.h

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 2 2017

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

commit 5b0f31f6370019772b2bc9c7d25ac53178a33af7
Author: Weiliang Chen <weiliangc@chromium.org>
Date: Wed Aug 02 17:20:58 2017

cc: Always use SetNew to modify SharedQuadState data

Always calls SetNew right after creating SharedQuadstate to set all the
values on SharedQuadState. This is to help catch all cases when
switching to use id in later CLs.

R=danakj@chromium.org

Bug: 739429
Change-Id: I029f73d2ff3b261ce12da65ba7eb49b6323f37c1
Reviewed-on: https://chromium-review.googlesource.com/596768
Commit-Queue: weiliangc <weiliangc@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491417}
[modify] https://crrev.com/5b0f31f6370019772b2bc9c7d25ac53178a33af7/ash/fast_ink/fast_ink_view.cc
[modify] https://crrev.com/5b0f31f6370019772b2bc9c7d25ac53178a33af7/components/exo/surface.cc
[modify] https://crrev.com/5b0f31f6370019772b2bc9c7d25ac53178a33af7/components/viz/service/display/surface_aggregator.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 2 2017

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

commit a294c58f59a21a787af6631c2ae82daaf98fdca0
Author: Weiliang Chen <weiliangc@chromium.org>
Date: Wed Aug 02 21:35:17 2017

cc: Stop taking in SharedQuadState* when copying DrawQuad

When copying DrawQuad to a RenderPass, the SharedQuadState should
already be on that RenderPass. Do not take SharedQuadState* as input
parameter for copying DrawQuad, use the last SharedQuadState on the
RenderPass instead. This not only avoids invalid SharedQuadState* being
passed in, but also reinforce that SharedQuadState and DrawQuad are
appended in order so it satisifies assumption for future implementation
of iterator after SharedQuadState id.

R=danakj@chromium.org

Bug: 739429
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I63fc2ca7a250fb8f6f9056473e0c6ab046652018
Reviewed-on: https://chromium-review.googlesource.com/597228
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: weiliangc <weiliangc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491509}
[modify] https://crrev.com/a294c58f59a21a787af6631c2ae82daaf98fdca0/cc/ipc/cc_param_traits_unittest.cc
[modify] https://crrev.com/a294c58f59a21a787af6631c2ae82daaf98fdca0/cc/quads/draw_quad_unittest.cc
[modify] https://crrev.com/a294c58f59a21a787af6631c2ae82daaf98fdca0/cc/quads/render_pass.cc
[modify] https://crrev.com/a294c58f59a21a787af6631c2ae82daaf98fdca0/cc/quads/render_pass.h
[modify] https://crrev.com/a294c58f59a21a787af6631c2ae82daaf98fdca0/components/viz/service/display/surface_aggregator.cc

Sign in to add a comment