Fix the size of video textures uploaded to WebGL.

Fix the size of video textures uploaded to WebGL.

When the hardware-accelerated video decoding path is in use, the
expected size of the destination WebGL texture is the video's natural
width and height. However, the code was blindly copying the entire
video source texture, which might include padding rows from the video
encoder.

Change to using CopySubTextureCHROMIUM in these code paths, and rely on
the caller to allocate the destination texture with the expected size.

A regression test for this bug is being added to the WebGL 2.0 conformance
suite in https://github.com/KhronosGroup/WebGL/pull/2202 .

BUG=672895
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2562003003
Cr-Commit-Position: refs/heads/master@{#442745}
Committed: https://chromium.googlesource.com/chromium/src/+/4910ae5b242c441bd047b78bb5cb46ad954920fa

[Ken Russell (switch to Gerrit), 2016-12-10 09:57:14]

Description was changed from

==========
Fix the size of video textures uploaded to WebGL.

When the hardware-accelerated video decoding path is in use, the
expected size of the destination WebGL texture is the video's natural
width and height. However, the code was blindly copying the entire
video source texture, which might include padding rows from the video
encoder.

Change to using CopySubTextureCHROMIUM in these code paths, and rely on
the caller to allocate the destination texture with the expected size.

A regression test for this bug is being added to the WebGL 2.0 conformance
suite in https://github.com/KhronosGroup/WebGL/pull/2202 .

BUG=672895
==========

to

==========
Fix the size of video textures uploaded to WebGL.

When the hardware-accelerated video decoding path is in use, the
expected size of the destination WebGL texture is the video's natural
width and height. However, the code was blindly copying the entire
video source texture, which might include padding rows from the video
encoder.

Change to using CopySubTextureCHROMIUM in these code paths, and rely on
the caller to allocate the destination texture with the expected size.

A regression test for this bug is being added to the WebGL 2.0 conformance
suite in https://github.com/KhronosGroup/WebGL/pull/2202 .

BUG=672895
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Ken Russell (switch to Gerrit), 2016-12-10 10:06:21]

kbr@chromium.org changed reviewers:
+ chrishtr@chromium.org, dalecurtis@chromium.org, esprehn@chromium.org,
hubbe@chromium.org, jbauman@chromium.org

[Ken Russell (switch to Gerrit), 2016-12-10 10:06:22]

dalecurtis or hubbe: OWNERS review of content/renderer/media/ and media/ please.

esprehn or chrishtr: OWNERS review of WebMediaPlayer.h please.

zmo: please review everything.

jbauman, fmalita, kainino: any comments welcome.

Thanks.

[Ken Russell (switch to Gerrit), 2016-12-12 19:09:54]

+junov and senorblanco as FYI

[DaleCurtis, 2016-12-12 19:12:23]

lgtm

https://codereview.chromium.org/2562003003/diff/1/media/renderers/skcanvas_vi...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2562003003/diff/1/media/renderers/skcanvas_vi...
media/renderers/skcanvas_video_renderer.cc:838: gfx::Size natural_size =
video_frame->natural_size();
const& for consistency?

[chrishtr, 2016-12-12 19:40:35]

lgtm

[esprehn, 2016-12-12 22:13:45]

How often do we sync those tests?

lgtm

[Ken Russell (switch to Gerrit), 2016-12-20 04:45:34]

https://codereview.chromium.org/2562003003/diff/1/media/renderers/skcanvas_vi...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2562003003/diff/1/media/renderers/skcanvas_vi...
media/renderers/skcanvas_video_renderer.cc:838: gfx::Size natural_size =
video_frame->natural_size();
On 2016/12/12 19:12:23, DaleCurtis_OOO_Dec14_Jan6 wrote:
> const& for consistency?

Yes, thanks. Done.

[Ken Russell (switch to Gerrit), 2016-12-20 04:46:36]

On 2016/12/12 22:13:45, esprehn wrote:
> How often do we sync those tests?
> 
> lgtm

A few times per week, nowadays. It's still a manual process but there's a roll
script in tools/ which helps. It's often the case that suppressions for new test
failures have to be added along with each roll, and we don't want to automate
that process.

[Ken Russell (switch to Gerrit), 2016-12-20 04:46:40]

The CQ bit was checked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2016-12-20 04:46:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, esprehn@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2562003003/#ps20001
(title: "Fixed review feedback from dalecurtis. Rebased.")

[commit-bot: I haz the power, 2016-12-20 04:46:59]

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-12-20 05:08:48]

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

[commit-bot: I haz the power, 2016-12-20 05:08:50]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[esprehn, 2016-12-20 05:22:50]

On 2016/12/20 at 04:46:36, kbr wrote:
> On 2016/12/12 22:13:45, esprehn wrote:
> > How often do we sync those tests?
> > 
> > lgtm
> 
> A few times per week, nowadays. It's still a manual process but there's a roll
script in tools/ which helps. It's often the case that suppressions for new test
failures have to be added along with each roll, and we don't want to automate
that process.

How do you make sure that the tests for this patch don't actually fail and then
get suppressed? Might be nice to have a process for that.

[Ken Russell (switch to Gerrit), 2016-12-20 05:35:23]

On 2016/12/20 05:22:50, esprehn wrote:
> On 2016/12/20 at 04:46:36, kbr wrote:
> > On 2016/12/12 22:13:45, esprehn wrote:
> > > How often do we sync those tests?
> > > 
> > > lgtm
> > 
> > A few times per week, nowadays. It's still a manual process but there's a
roll
> script in tools/ which helps. It's often the case that suppressions for new
test
> failures have to be added along with each roll, and we don't want to automate
> that process.
> 
> How do you make sure that the tests for this patch don't actually fail and
then
> get suppressed? Might be nice to have a process for that.

We add bug IDs to any suppressions in e.g.
src/content/test/gpu/gpu_tests/webgl_conformance_expectations.py and make sure
that these are resolved before closing the bug.

[Ken Russell (switch to Gerrit), 2016-12-20 05:42:06]

On 2016/12/20 05:35:23, Ken Russell wrote:
> On 2016/12/20 05:22:50, esprehn wrote:
> > On 2016/12/20 at 04:46:36, kbr wrote:
> > > On 2016/12/12 22:13:45, esprehn wrote:
> > > > How often do we sync those tests?
> > > > 
> > > > lgtm
> > > 
> > > A few times per week, nowadays. It's still a manual process but there's a
> roll
> > script in tools/ which helps. It's often the case that suppressions for new
> test
> > failures have to be added along with each roll, and we don't want to
automate
> > that process.
> > 
> > How do you make sure that the tests for this patch don't actually fail and
> then
> > get suppressed? Might be nice to have a process for that.
> 
> We add bug IDs to any suppressions in e.g.
> src/content/test/gpu/gpu_tests/webgl_conformance_expectations.py and make sure
> that these are resolved before closing the bug.

Note: sometimes we incorporate a WebGL conformance roll into a CL in order to
pull in a test without a suppression. However it's usually necessary to do these
out of sync. Since new tests are usually added to the WebGL conformance suite
because of bugs seen on one platform or another, it's usually the case that the
tests are written and landed before the Chromium CL fixing them is finished with
code review. Doing changes in lock step is impossible.

It's clear there are some compilation problems with this patch on Android so I'm
going to work on resolving those. If nobody's rolled the WebGL conformance suite
by that point I'll do so and make sure the new test's passing everywhere.

[Ken Russell (switch to Gerrit), 2017-01-10 22:37:39]

The CQ bit was checked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2017-01-10 22:37:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, esprehn@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2562003003/#ps40001
(title: "Rebased. Fixed Android build.")

[commit-bot: I haz the power, 2017-01-10 22:38:27]

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

[Ken Russell (switch to Gerrit), 2017-01-10 23:37:57]

The CQ bit was unchecked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2017-01-10 23:38:53]

Description was changed from

==========
Fix the size of video textures uploaded to WebGL.

When the hardware-accelerated video decoding path is in use, the
expected size of the destination WebGL texture is the video's natural
width and height. However, the code was blindly copying the entire
video source texture, which might include padding rows from the video
encoder.

Change to using CopySubTextureCHROMIUM in these code paths, and rely on
the caller to allocate the destination texture with the expected size.

A regression test for this bug is being added to the WebGL 2.0 conformance
suite in https://github.com/KhronosGroup/WebGL/pull/2202 .

BUG=672895
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Fix the size of video textures uploaded to WebGL.

When the hardware-accelerated video decoding path is in use, the
expected size of the destination WebGL texture is the video's natural
width and height. However, the code was blindly copying the entire
video source texture, which might include padding rows from the video
encoder.

Change to using CopySubTextureCHROMIUM in these code paths, and rely on
the caller to allocate the destination texture with the expected size.

A regression test for this bug is being added to the WebGL 2.0 conformance
suite in https://github.com/KhronosGroup/WebGL/pull/2202 .

BUG=672895
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

[Ken Russell (switch to Gerrit), 2017-01-10 23:40:05]

ynovikov: FYI, added android_optional_gpu_tests_rel to the tryservers manually.
If that tryserver's reliable enough now, let's add it to the default set for
those directories that add the optional GPU tryservers.

[Ken Russell (switch to Gerrit), 2017-01-10 23:40:25]

The CQ bit was checked by kbr@chromium.org

[commit-bot: I haz the power, 2017-01-10 23:41:05]

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, 2017-01-11 00:52:33]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484091625006450,
"parent_rev": "d6539dc7b85800f035f1a14e06653497a45aab0c", "commit_rev":
"4910ae5b242c441bd047b78bb5cb46ad954920fa"}

[commit-bot: I haz the power, 2017-01-11 00:53:04]

Description was changed from

==========
Fix the size of video textures uploaded to WebGL.

When the hardware-accelerated video decoding path is in use, the
expected size of the destination WebGL texture is the video's natural
width and height. However, the code was blindly copying the entire
video source texture, which might include padding rows from the video
encoder.

Change to using CopySubTextureCHROMIUM in these code paths, and rely on
the caller to allocate the destination texture with the expected size.

A regression test for this bug is being added to the WebGL 2.0 conformance
suite in https://github.com/KhronosGroup/WebGL/pull/2202 .

BUG=672895
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

to

==========
Fix the size of video textures uploaded to WebGL.

When the hardware-accelerated video decoding path is in use, the
expected size of the destination WebGL texture is the video's natural
width and height. However, the code was blindly copying the entire
video source texture, which might include padding rows from the video
encoder.

Change to using CopySubTextureCHROMIUM in these code paths, and rely on
the caller to allocate the destination texture with the expected size.

A regression test for this bug is being added to the WebGL 2.0 conformance
suite in https://github.com/KhronosGroup/WebGL/pull/2202 .

BUG=672895
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2562003003
Cr-Commit-Position: refs/heads/master@{#442745}
Committed:
https://chromium.googlesource.com/chromium/src/+/4910ae5b242c441bd047b78bb5cb...
==========

[commit-bot: I haz the power, 2017-01-11 00:53:06]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/4910ae5b242c441bd047b78bb5cb...