cc: Use sublayer scale from effect tree (1)

cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawPropertyUtils with
ComputeTransform and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5ab6dc318ba35577b1344437b78d6c1a1fc2fc11
Cr-Commit-Position: refs/heads/master@{#404526}

[jaydasika, 2016-07-08 00:43:37]

Description was changed from

==========
cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawProeprtyUtils with
ComputeTransforms and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
==========

to

==========
cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawProeprtyUtils with
ComputeTransforms and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-07-08 00:44:03]

Description was changed from

==========
cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawProeprtyUtils with
ComputeTransforms and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawPropertyUtils with
ComputeTransform and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-07-08 00:47:26]

jaydasika@chromium.org changed reviewers:
+ sunxd@chromium.org, weiliangc@chromium.org

[jaydasika, 2016-07-08 00:47:27]

PTAL

[weiliangc, 2016-07-08 19:19:03]

LGTM

https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:56: static void
AddSublayerScaleToTransform(const int effect_node_id,
nit: ApplySublayerScale?

https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:61: effect_node->data.has_render_surface
I think when we call this function, the effect_node should always
has_render_surface? Do you think it make sense to DCHECK that here?

[jaydasika, 2016-07-08 19:30:47]

https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:61: effect_node->data.has_render_surface
On 2016/07/08 19:19:03, weiliangc wrote:
> I think when we call this function, the effect_node should always
> has_render_surface? Do you think it make sense to DCHECK that here?

We may call this function when effect_node may not have render surface, like
here:
https://cs.chromium.org/chromium/src/cc/trees/draw_property_utils.cc?sq=packa...

I think its better to find the target effect node here rather than expect each
caller to do it on their own. WDYT ?

[jaydasika, 2016-07-08 19:44:25]

https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:56: static void
AddSublayerScaleToTransform(const int effect_node_id,
On 2016/07/08 19:19:03, weiliangc wrote:
> nit: ApplySublayerScale?

Done.

[jaydasika, 2016-07-08 19:45:22]

jaydasika@chromium.org changed reviewers:
+ jbroman@chromium.org

[jaydasika, 2016-07-08 19:45:23]

jbroman : Can you review changes to PaintArtifactCompositor ?

[jbroman, 2016-07-08 19:53:02]

PaintArtifactCompositor lgtm, but you'll have to fix the bot failures and
possibly resolve conflicts with trchen's patch
(https://codereview.chromium.org/2052763002), which may land soon

[weiliangc, 2016-07-08 19:55:35]

On 2016/07/08 19:30:47, jaydasika wrote:
>
https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
> File cc/trees/draw_property_utils.cc (right):
> 
>
https://codereview.chromium.org/2127323002/diff/20001/cc/trees/draw_property_...
> cc/trees/draw_property_utils.cc:61: effect_node->data.has_render_surface
> On 2016/07/08 19:19:03, weiliangc wrote:
> > I think when we call this function, the effect_node should always
> > has_render_surface? Do you think it make sense to DCHECK that here?
> 
> We may call this function when effect_node may not have render surface, like
> here:
>
https://cs.chromium.org/chromium/src/cc/trees/draw_property_utils.cc?sq=packa...
> 
> I think its better to find the target effect node here rather than expect each
> caller to do it on their own. WDYT ?

Probably no need to do it on this patch. Since sublayer scale is only applied at
RenderTarget I think it makes more since to make the function call with the
target id, rather than calling a effect tree id and get the target inside the
function. (Possibly should have a helper function to get target effect id)

[jaydasika, 2016-07-08 21:53:50]

The CQ bit was checked by jaydasika@chromium.org

[jaydasika, 2016-07-08 21:53:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
weiliangc@chromium.org, jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2127323002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-07-08 21:54:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-08 23:08:36]

Description was changed from

==========
cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawPropertyUtils with
ComputeTransform and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawPropertyUtils with
ComputeTransform and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-07-08 23:08:37]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-08 23:08:52]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-08 23:10:11]

Description was changed from

==========
cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawPropertyUtils with
ComputeTransform and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Use sublayer scale from effect tree (1)

This CL replaces all calls to
ComputeTransformWithDestinationSublayerScale in DrawPropertyUtils with
ComputeTransform and then applies the destination sublayer scale using
the effect tree. To do this, I added a target_effect_id to clip nodes.

BUG=622372
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5ab6dc318ba35577b1344437b78d6c1a1fc2fc11
Cr-Commit-Position: refs/heads/master@{#404526}
==========

[commit-bot: I haz the power, 2016-07-08 23:10:13]

Patchset 4 (id:??) landed as
https://crrev.com/5ab6dc318ba35577b1344437b78d6c1a1fc2fc11
Cr-Commit-Position: refs/heads/master@{#404526}