Fixed YUV Video quads when their visible_content_rect != their rect.

Fixed YUV Video quads when their visible_content_rect != their rect.

BUG=467673
Committed: https://crrev.com/4596599a1f99c0459bc4adc9251d4c1d25a426ca
Cr-Commit-Position: refs/heads/master@{#320916}

[awoloszyn, 2015-03-16 21:50:15]

awoloszyn@chromium.org changed reviewers:
+ enne@chromium.org

[danakj, 2015-03-16 23:08:48]

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

[danakj, 2015-03-16 23:08:49]

https://codereview.chromium.org/1007383004/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/1007383004/diff/1/cc/output/gl_renderer.cc#ne...
cc/output/gl_renderer.cc:2076: gfx::RectF tile_rect = quad->rect;
This means we overdraw when the video is occluded tho? Maybe it's a corner
case.. but would it be difficult to still use the visible rect?

[awoloszyn, 2015-03-16 23:15:55]

https://codereview.chromium.org/1007383004/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/1007383004/diff/1/cc/output/gl_renderer.cc#ne...
cc/output/gl_renderer.cc:2076: gfx::RectF tile_rect = quad->rect;
On 2015/03/16 23:08:49, danakj wrote:
> This means we overdraw when the video is occluded tho? Maybe it's a corner
> case.. but would it be difficult to still use the visible rect?

This matches the original behavior before my splitting patch went in, (as in we
used to use the quad->rect instead of the visible_rect).
It may be possible to use the visible_rect instead, but that would require a
change to how the shader operates.

[enne (OOO), 2015-03-16 23:16:55]

https://codereview.chromium.org/1007383004/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/1007383004/diff/1/cc/output/gl_renderer.cc#ne...
cc/output/gl_renderer.cc:2076: gfx::RectF tile_rect = quad->rect;
On 2015/03/16 at 23:08:49, danakj wrote:
> This means we overdraw when the video is occluded tho? Maybe it's a corner
case.. but would it be difficult to still use the visible rect?

I think this is fine as-is.

You'd have to scale both the uv and and the ya texture coordinates differently,
I think.  It's somewhat complicated, so possible to get wrong (or break easily).
 Watching videos that are partially offscreen seems like a bit of a corner case
to me.  If I were going to optimize videos, I'd look at a number of other things
first.

https://codereview.chromium.org/1007383004/diff/1/cc/output/renderer_pixeltes...
File cc/output/renderer_pixeltest.cc (right):

https://codereview.chromium.org/1007383004/diff/1/cc/output/renderer_pixeltes...
cc/output/renderer_pixeltest.cc:981: gfx::Rect
visible_rect(this->device_viewport_size_.width() * 0.25,
Can you make this test more robust? Visible rect means it's equally valid to
only draw that rect, and I think this test would fail if that optimization were
made.

[awoloszyn, 2015-03-17 02:26:01]

https://codereview.chromium.org/1007383004/diff/1/cc/output/renderer_pixeltes...
File cc/output/renderer_pixeltest.cc (right):

https://codereview.chromium.org/1007383004/diff/1/cc/output/renderer_pixeltes...
cc/output/renderer_pixeltest.cc:981: gfx::Rect
visible_rect(this->device_viewport_size_.width() * 0.25,
On 2015/03/16 23:16:55, enne wrote:
> Can you make this test more robust? Visible rect means it's equally valid to
> only draw that rect, and I think this test would fail if that optimization
were
> made.

This test now has the visible_rect as the viewport, so even if it we optimize,
the test should not change. This also exhibits the same incorrect behavior for
scaling before the patch.

[enne (OOO), 2015-03-17 02:42:35]

The CQ bit was checked by enne@chromium.org

[enne (OOO), 2015-03-17 02:42:37]

lgtm

[commit-bot: I haz the power, 2015-03-17 02:42:42]

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

[commit-bot: I haz the power, 2015-03-17 07:42:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-17 07:42:11]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[awoloszyn, 2015-03-17 13:41:39]

The CQ bit was checked by awoloszyn@chromium.org

[awoloszyn, 2015-03-17 13:41:39]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org
Link to the patchset: https://codereview.chromium.org/1007383004/#ps40001
(title: "Added correct yuv_striped_clipped.png")

[commit-bot: I haz the power, 2015-03-17 13:41:54]

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

[commit-bot: I haz the power, 2015-03-17 15:13:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-17 15:15:06]

Patchset 3 (id:??) landed as
https://crrev.com/4596599a1f99c0459bc4adc9251d4c1d25a426ca
Cr-Commit-Position: refs/heads/master@{#320916}