Apply the correct transform_to_root_target DelegatedRendererLayerImpl::AppendRenderPassQuads().

Apply the correct transform_to_root_target DelegatedRendererLayerImpl::AppendRenderPassQuads().

render_pass->transform_to_root_target should've been
delegated_render_pass->transform_to_root_target.

BUG=406274
Committed: https://crrev.com/53af0fedc3d008f46221224043a41555e5d0e347
Cr-Commit-Position: refs/heads/master@{#296036}

[hartmanng, 2014-09-05 21:34:05]

hartmanng@chromium.org changed reviewers:
+ danakj@chromium.org

[hartmanng, 2014-09-05 21:34:05]

Dana, please take a look. I did my best to justify the updated unit test
expectations here:
https://docs.google.com/document/d/1R_PAjzXgA06CPeuGFP0oyfn8fseTywqVhl5EDxHPe....
Please let me know if I'm still misunderstanding the situation.

[danakj, 2014-09-09 19:16:34]

https://codereview.chromium.org/546993002/diff/1/cc/layers/delegated_renderer...
File cc/layers/delegated_renderer_layer_impl_unittest.cc (right):

https://codereview.chromium.org/546993002/diff/1/cc/layers/delegated_renderer...
cc/layers/delegated_renderer_layer_impl_unittest.cc:1396:
AddRenderPass(&delegated_render_passes, pass2_id, quad_rect, transform);
and this should be gfx::Rect(11, 0, 899, 1000) instead of quad_rect

then the transform matches the rect. and you get a quad at 200,300 inside the
surface which is offset by 11, putting the quad at 211,300 on the screen.

https://codereview.chromium.org/546993002/diff/1/cc/layers/delegated_renderer...
cc/layers/delegated_renderer_layer_impl_unittest.cc:1397: AddQuad(pass2,
gfx::Rect(quad_rect.size()), SK_ColorRED);
i think this should be quad_rect instead of dropping the offset

[hartmanng, 2014-09-22 15:57:40]

On 2014/09/09 19:16:34, danakj wrote:
>
https://codereview.chromium.org/546993002/diff/1/cc/layers/delegated_renderer...
> File cc/layers/delegated_renderer_layer_impl_unittest.cc (right):
> 
>
https://codereview.chromium.org/546993002/diff/1/cc/layers/delegated_renderer...
> cc/layers/delegated_renderer_layer_impl_unittest.cc:1396:
> AddRenderPass(&delegated_render_passes, pass2_id, quad_rect, transform);
> and this should be gfx::Rect(11, 0, 899, 1000) instead of quad_rect
> 
> then the transform matches the rect. and you get a quad at 200,300 inside the
> surface which is offset by 11, putting the quad at 211,300 on the screen.
> 
>
https://codereview.chromium.org/546993002/diff/1/cc/layers/delegated_renderer...
> cc/layers/delegated_renderer_layer_impl_unittest.cc:1397: AddQuad(pass2,
> gfx::Rect(quad_rect.size()), SK_ColorRED);
> i think this should be quad_rect instead of dropping the offset

Sorry for the long delay here.


I made your suggestions, and am getting various failures now:

[==========] Running 3 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 3 tests from DelegatedRendererLayerImplTest
[ RUN      ] DelegatedRendererLayerImplTest.InvalidRenderPassDrawQuad
[       OK ] DelegatedRendererLayerImplTest.InvalidRenderPassDrawQuad (2 ms)
[ RUN      ] DelegatedRendererLayerImplTest.Occlusion
../../cc/test/layer_test_common.cc:54: Failure
Value of: rect.Contains(quad_rect)
  Actual: false
Expected: true
    Quad: 0 rect: 211,300 400x500 quad: 22,0 889x1000
Google Test trace:
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1418: Root render pass
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1414: No occlusion
../../cc/test/layer_test_common.cc:57: Failure
Value of: remaining.Contains(quad_rect)
  Actual: false
Expected: true
    Quad: 0 remaining: 211,300 400x500 quad: 22,0 889x1000
Google Test trace:
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1418: Root render pass
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1414: No occlusion
../../cc/test/layer_test_common.cc:54: Failure
Value of: rect.Contains(quad_rect)
  Actual: false
Expected: true
    Quad: 0 rect: 0,0 400x500 quad: 200,300 400x500
Google Test trace:
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1427: Contributing
render pass
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1414: No occlusion
../../cc/test/layer_test_common.cc:57: Failure
Value of: remaining.Contains(quad_rect)
  Actual: false
Expected: true
    Quad: 0 remaining: 0,0 400x500 quad: 200,300 400x500


and so on.


Also, the partial occlusion tests are failing due to no occlusion:

Google Test trace:
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1454: Contributing
render pass
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1438: Full occlusion
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1497: Failure
Value of: partially_occluded_count
  Actual: 0
Expected: 1u
Which is: 1
Google Test trace:
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1486: Contributing
render pass
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1464: Partial
occlusion
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1502: Failure
Value of: impl.quad_list()[0]->visible_rect.ToString()
  Actual: "200,300 400x500"
Expected: gfx::Rect(300 - 22, 0, 100 + 22, 500).ToString()
Which is: "278,0 122x500"
Google Test trace:
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1486: Contributing
render pass
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1464: Partial
occlusion
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1525: Failure
Value of: partially_occluded_count
  Actual: 0
Expected: 1u
Which is: 1
Google Test trace:
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1510: Contributing
render pass with transformed root
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1530: Failure
Value of: impl.quad_list()[0]->visible_rect.ToString()
  Actual: "200,300 400x500"
Expected: gfx::Rect(300 - 33, 0, 100 + 33, 500).ToString()
Which is: "267,0 133x500"
Google Test trace:
../../cc/layers/delegated_renderer_layer_impl_unittest.cc:1510: Contributing
render pass with transformed root



I'm confused by a few things here... First of all I'm not seeing why quad is
sometimes (22,0 889x1000). These results:

  Actual: false
Expected: true
    Quad: 0 rect: 0,0 400x500 quad: 200,300 400x500

seem to make slightly more sense to me, but the transform of 11 is missing.

But I also am not really seeing why there is no partial occlusion. The occlusion
rect is defined as gfx::Rect occluded(0, 0, 500, 1000) in those cases, which
certainly should intersect with the quad if it's at (211,300 400x500), (200,300
400x500), or even (22,0 889x1000), but that doesn't seem to be the case?

[hartmanng, 2014-09-22 18:09:40]

Patchset #3 (id:40001) has been deleted

[hartmanng, 2014-09-22 18:11:48]

Patchset #3 (id:60001) has been deleted

[hartmanng, 2014-09-22 18:11:54]

Patchset #3 (id:80001) has been deleted

[hartmanng, 2014-09-22 18:20:35]

Patchset #4 (id:120001) has been deleted

[hartmanng, 2014-09-22 18:30:43]

Ok, I think the test is all fixed up now (thanks for all the help!)

PTAL

[danakj, 2014-09-22 18:55:07]

LGTM

https://codereview.chromium.org/546993002/diff/140001/cc/layers/delegated_ren...
File cc/layers/delegated_renderer_layer_impl_unittest.cc (right):

https://codereview.chromium.org/546993002/diff/140001/cc/layers/delegated_ren...
cc/layers/delegated_renderer_layer_impl_unittest.cc:1452: // Move the occlusion
to where it is in the contributing surface.
leftover

https://codereview.chromium.org/546993002/diff/140001/cc/layers/delegated_ren...
cc/layers/delegated_renderer_layer_impl_unittest.cc:1513:
delegated_renderer_layer_impl->SetTransform(layer_transform);
can you do occlusion_in_target_of_delegated_quad -= vector2d(11,0) down here
instead so it's more clear where that 11 is coming from?

[hartmanng, 2014-09-22 19:02:04]

Patchset #5 (id:160001) has been deleted

[hartmanng, 2014-09-22 19:03:00]

Thanks!

https://codereview.chromium.org/546993002/diff/140001/cc/layers/delegated_ren...
File cc/layers/delegated_renderer_layer_impl_unittest.cc (right):

https://codereview.chromium.org/546993002/diff/140001/cc/layers/delegated_ren...
cc/layers/delegated_renderer_layer_impl_unittest.cc:1452: // Move the occlusion
to where it is in the contributing surface.
On 2014/09/22 18:55:06, danakj wrote:
> leftover

Done.

https://codereview.chromium.org/546993002/diff/140001/cc/layers/delegated_ren...
cc/layers/delegated_renderer_layer_impl_unittest.cc:1513:
delegated_renderer_layer_impl->SetTransform(layer_transform);
On 2014/09/22 18:55:06, danakj wrote:
> can you do occlusion_in_target_of_delegated_quad -= vector2d(11,0) down here
> instead so it's more clear where that 11 is coming from?

Done.

[hartmanng, 2014-09-22 19:03:04]

The CQ bit was checked by hartmanng@chromium.org

[commit-bot: I haz the power, 2014-09-22 19:04:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/546993002/180001

[commit-bot: I haz the power, 2014-09-22 20:26:18]

Committed patchset #5 (id:180001) as 0c6cd5a41b22cea7a49061051c2ee2259727e60c

[commit-bot: I haz the power, 2014-09-22 20:26:58]

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