Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Committed: https://crrev.com/fc9d613c03b8e72ddfbc9f43d87119d29f96fdee
Cr-Original-Commit-Position: refs/heads/master@{#405507}
Cr-Commit-Position: refs/heads/master@{#409543}

[Tobias Sargeant, 2016-03-21 15:40:32]

Description was changed from

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=
==========

to

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Tobias Sargeant, 2016-03-21 15:41:31]

Description was changed from

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Tobias Sargeant, 2016-03-21 15:41:32]

tobiasjs@chromium.org changed reviewers:
+ liberato@chromium.org

[Tobias Sargeant, 2016-03-21 15:42:32]

PTAL. I decided that this was best done as a separate CL.

[Tobias Sargeant, 2016-03-21 15:57:27]

Patchset #1 (id:1) has been deleted

[liberato (no reviews please), 2016-03-21 16:21:57]

https://codereview.chromium.org/1818073002/diff/20001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1818073002/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7548: // Pre-multiply
|gl_matrix| by |post_transform|.
are you sure that it should be premultiplied?  if |gl_matrix| is a translation,
and |post_transform|| is a scale, then we don't want to scale the translation,
do we?  that should be applied last.

i think that's |gl_matrix| * |post_transform| * uv_vector_in_fragment_shader,
assuming that the copier shader is also M * v.

now, it should be said that i (somehow) have a track record of < 50% with matrix
order.

https://codereview.chromium.org/1818073002/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7550: for (int c = 0; c < 16; c
+= 4) {
maybe sievers has a way to avoid writing matrix multiplication in-line here. 
wrap in skmatrix?

[Tobias Sargeant, 2016-03-21 17:29:05]

On 2016/03/21 16:21:57, liberato wrote:
>
https://codereview.chromium.org/1818073002/diff/20001/gpu/command_buffer/serv...
> File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
> 
>
https://codereview.chromium.org/1818073002/diff/20001/gpu/command_buffer/serv...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:7548: // Pre-multiply
> |gl_matrix| by |post_transform|.
> are you sure that it should be premultiplied?  if |gl_matrix| is a
translation,
> and |post_transform|| is a scale, then we don't want to scale the translation,
> do we?  that should be applied last.
> 
> i think that's |gl_matrix| * |post_transform| * uv_vector_in_fragment_shader,
> assuming that the copier shader is also M * v.
> 
> now, it should be said that i (somehow) have a track record of < 50% with
matrix
> order.

It was definitely premultiplication beforehand:

https://code.google.com/p/chromium/codesearch#chromium/src/cc/layers/video_la...

But that doesn't preclude the possibility that it was originally also wrong.
I'll have a think through it. Either way, post_transform is a bad name, and I
should change that.

I went looking for a 4x4 matrix multiply that could be used with float[16] in
column major order, but didn't find anything.  It seemed like a using
gfx::Transform was a pretty heavyweight way of accomplishing this.

[Tobias Sargeant, 2016-03-22 14:26:43]

In the vertex shader we have:

      v_texCoord = vec2(texMatrix * vec4(a_texCoord.x, 1.0 - a_texCoord.y, 0.0,
1.0));

If we think that it should be |gl_matrix| * |post_transform| then this also
needs to change. I'm inclined to think that you are right, though; the sequence
of transforms should be:

normalized quad coordinates -> normalized texture coordinates -> stream texture
coordinates

The tricky thing is that the concatenation of the ST matrix and the y-flip is I,
and so the order of transformations doesn't matter, and this has probably been
subtly wrong forever, without any way to notice it.

[Tobias Sargeant, 2016-03-22 15:21:10]

tobiasjs@chromium.org changed reviewers:
+ sievers@chromium.org

[Tobias Sargeant, 2016-03-22 15:21:10]

This needs to land after https://codereview.chromium.org/1746983002 because the
correctness of this CL depends on not doing the matrix multiplication by the
surface texture matrix in video_layer_impl.cc (which that CL will enable us to
remove).

[liberato (no reviews please), 2016-03-22 15:29:11]

On 2016/03/22 14:26:43, Tobias Sargeant wrote:
> In the vertex shader we have:
> 
>       v_texCoord = vec2(texMatrix * vec4(a_texCoord.x, 1.0 - a_texCoord.y,
0.0,
> 1.0));
> 
> If we think that it should be |gl_matrix| * |post_transform| then this also
> needs to change. I'm inclined to think that you are right, though; the
sequence
> of transforms should be:
> 
> normalized quad coordinates -> normalized texture coordinates -> stream
texture
> coordinates
> 
> The tricky thing is that the concatenation of the ST matrix and the y-flip is
I,
> and so the order of transformations doesn't matter, and this has probably been
> subtly wrong forever, without any way to notice it.

we have some devices that have non-trivial ST matrices.  don't those tell us if
it works?

or, maybe, everything is currently row-major, and we're really doing what we
think we should be.  :)

[Tobias Sargeant, 2016-03-22 15:33:11]

> we have some devices that have non-trivial ST matrices.  don't those tell us
if
> it works?

What are those devices? I'll test, if I can get one.

[liberato (no reviews please), 2016-03-22 15:44:25]

On 2016/03/22 15:33:11, Tobias Sargeant wrote:
> > we have some devices that have non-trivial ST matrices.  don't those tell us
> if
> > it works?
> 
> What are those devices? I'll test, if I can get one.

rumor has it that the xoom and Galaxy S6 fit.  i've got an s6 in front of me,
can check under what circumstances it does this.

-fl

[Tobias Sargeant, 2016-03-31 14:46:41]

Description was changed from

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[liberato (no reviews please), 2016-03-31 15:03:16]

https://codereview.chromium.org/1818073002/diff/60001/cc/output/shader.cc
File cc/output/shader.cc (right):

https://codereview.chromium.org/1818073002/diff/60001/cc/output/shader.cc#new...
cc/output/shader.cc:756: vec2(1.0, -1.0) * (texMatrix * vec4(a_texCoord.xy, 0.0,
1.0)).xy;
is this different?

https://codereview.chromium.org/1818073002/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1818073002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7629:
image->GetTextureMatrix(gl_matrix);
don't we want this to be GetFlippedTextureMatrix?  that lets the shader be more
intuitive.

https://codereview.chromium.org/1818073002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7629:
image->GetTextureMatrix(gl_matrix);
maybe not for this CL, but GetFlippedTextureMatrix might not be the best name in
retrospect.  GetChromiumTextureMatrix or similar, since nobody will care what
changes are made to the original, only that it's now in chromium coordinates.

or, just GetTextureMatrix.  i think that was your original proposal, and i agree
that you were right.  :)

https://codereview.chromium.org/1818073002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7630: GLfloat tmp[16];
without this premultiply, does the y-flip without UMP come back?  if so, then it
raises the question why !UMP is sending down a y-flip.

i don't see what else in this CL could change the behavior, but i also don't see
why it would affect !UMP only.

[Tobias Sargeant, 2016-03-31 20:46:13]

Ok, I think what we need to do is even more subtle. The transform pipeline
should look like this:

[v]quad coords -> [A]SVDQ transform (chromium texcoords) -> [B]y-flip
(SurfaceTexture texcoords) -> [C]SurfaceTexture matrix (sampler coords).

So what this means is that we DEFINITELY have to remove the y-flip from the
shader.

Before this patch we're computing [v]*[B]*[C] which is wrong because we're
ignoring [A]. At PS3 we're computing [v]*[A]*[C]*[B] which is wrong in the case
where [B]*[C] != [I].

[liberato (no reviews please), 2016-03-31 21:41:40]

On 2016/03/31 20:46:13, Tobias Sargeant wrote:
> Ok, I think what we need to do is even more subtle. The transform pipeline
> should look like this:
> 
> [v]quad coords -> [A]SVDQ transform (chromium texcoords) -> [B]y-flip
> (SurfaceTexture texcoords) -> [C]SurfaceTexture matrix (sampler coords).
> 
> So what this means is that we DEFINITELY have to remove the y-flip from the
> shader.
> 
> Before this patch we're computing [v]*[B]*[C] which is wrong because we're
> ignoring [A]. At PS3 we're computing [v]*[A]*[C]*[B] which is wrong in the
case
> where [B]*[C] != [I].

sampler: i agree that we want no y-flip in every shader.  nothing outside of
StreamTextureImage should know that android does that.  further i agree that
GetFlippedTextureMatrix should be farther away than [A] from [v], which i think
is what you drew.  with paranthesis, i think:

([v] * [A]) * [GetFlippedTextureMatrix] => sampler coords.

whether it's pre- or post- multiplied, i cannot say.  but i'm sure that it's
either that or GFT * ([A] * v).

[Tobias Sargeant, 2016-04-01 11:59:09]

tobiasjs@chromium.org changed reviewers:
+ enne@chromium.org, sky@chromium.org

[Tobias Sargeant, 2016-04-01 11:59:10]

I think this is good to go now. PTAL.

+sky for mojo autogen stamp.

+enne for changes to shaders in cc/output and matrix transforms.

https://codereview.chromium.org/1818073002/diff/60001/cc/output/shader.cc
File cc/output/shader.cc (right):

https://codereview.chromium.org/1818073002/diff/60001/cc/output/shader.cc#new...
cc/output/shader.cc:756: vec2(1.0, -1.0) * (texMatrix * vec4(a_texCoord.xy, 0.0,
1.0)).xy;
On 2016/03/31 15:03:16, liberato wrote:
> is this different?

It was different (y flip before matrix multiplication vs. y flip after). It's
gone completely now though.

https://codereview.chromium.org/1818073002/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1818073002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7629:
image->GetTextureMatrix(gl_matrix);
On 2016/03/31 15:03:16, liberato wrote:
> maybe not for this CL, but GetFlippedTextureMatrix might not be the best name
in
> retrospect.  GetChromiumTextureMatrix or similar, since nobody will care what
> changes are made to the original, only that it's now in chromium coordinates.
> 
> or, just GetTextureMatrix.  i think that was your original proposal, and i
agree
> that you were right.  :)

Done. On reflection I decided against GetChromiumTextureMatrix in favour of
requiring GetTextureMatrix to return a matrix that converts chromium orientation
texel coordinates into whatever coordinates are appropriate for whatever
GLStreamTextureImage wraps. For ADVA and StreamTextureAndroid this involves a
y-flip, so I made both use a convenience function for that explicitly.

[liberato (no reviews please), 2016-04-01 14:07:13]

lgtm, nice!

-fl

https://codereview.chromium.org/1818073002/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1818073002/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7629:
image->GetTextureMatrix(gl_matrix);
On 2016/04/01 11:59:10, Tobias Sargeant wrote:
> On 2016/03/31 15:03:16, liberato wrote:
> > maybe not for this CL, but GetFlippedTextureMatrix might not be the best
name
> in
> > retrospect.  GetChromiumTextureMatrix or similar, since nobody will care
what
> > changes are made to the original, only that it's now in chromium
coordinates.
> > 
> > or, just GetTextureMatrix.  i think that was your original proposal, and i
> agree
> > that you were right.  :)
> 
> Done. On reflection I decided against GetChromiumTextureMatrix in favour of
> requiring GetTextureMatrix to return a matrix that converts chromium
orientation
> texel coordinates into whatever coordinates are appropriate for whatever
> GLStreamTextureImage wraps. For ADVA and StreamTextureAndroid this involves a
> y-flip, so I made both use a convenience function for that explicitly.

makes sense.

[no sievers, 2016-04-01 21:39:01]

sievers@chromium.org changed reviewers:
+ ccameron@chromium.org

[no sievers, 2016-04-01 21:39:54]

This looks good, but +ccameron to help with checking this for Mac. I didn't know
StreamVideoDrawQuad ended up being used there too.

https://codereview.chromium.org/1818073002/diff/100001/gpu/command_buffer/ser...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1818073002/diff/100001/gpu/command_buffer/ser...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7632: for (int r = 0; r < 4;
++r) {
Can you use gfx::Transform for the multiplication?

[no sievers, 2016-04-01 21:46:01]

> [v]quad coords -> [A]SVDQ transform (chromium texcoords) -> [B]y-flip
(SurfaceTexture texcoords) -> [C]SurfaceTexture matrix (sampler coords).


Oh could you also clarify where the extra y-flip outside of the ST matrix comes
from? Is this for non-Android platforms?
I only see the scale being applied in the code.

[Tobias Sargeant, 2016-04-08 17:45:41]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 17:45:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1818073002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1818073002/140001

[commit-bot: I haz the power, 2016-04-08 17:49:01]

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

[commit-bot: I haz the power, 2016-04-08 17:49:02]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Tobias Sargeant, 2016-04-08 17:52:57]

On 2016/04/01 21:46:01, sievers wrote:
> > [v]quad coords -> [A]SVDQ transform (chromium texcoords) -> [B]y-flip
> (SurfaceTexture texcoords) -> [C]SurfaceTexture matrix (sampler coords).
> 
> 
> Oh could you also clarify where the extra y-flip outside of the ST matrix
comes
> from? Is this for non-Android platforms?
> I only see the scale being applied in the code.

Sorry, the y-flip marked by [B] was in the shader where it transforms
vec4(a_texCoord.x, 1.0 - a_texCoord.y, 0.0, 1.0)

I've checked WebView, Chrome on Android +/- ADVA, and ChromeOS.

[no sievers, 2016-04-11 18:08:50]

https://codereview.chromium.org/1818073002/diff/100001/gpu/command_buffer/ser...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1818073002/diff/100001/gpu/command_buffer/ser...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7632: for (int r = 0; r < 4;
++r) {
On 2016/04/01 21:39:53, sievers wrote:
> Can you use gfx::Transform for the multiplication?

^^ what about this?

[Tobias Sargeant, 2016-04-12 11:05:24]

https://codereview.chromium.org/1818073002/diff/100001/gpu/command_buffer/ser...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1818073002/diff/100001/gpu/command_buffer/ser...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7632: for (int r = 0; r < 4;
++r) {
On 2016/04/11 18:08:50, sievers wrote:
> On 2016/04/01 21:39:53, sievers wrote:
> > Can you use gfx::Transform for the multiplication?
> 
> ^^ what about this?

I've reimplemented with gfx::Transform. WDYT?

[Tobias Sargeant, 2016-04-12 11:05:41]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-12 11:05:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1818073002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1818073002/160001

[commit-bot: I haz the power, 2016-04-12 11:22:48]

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

[commit-bot: I haz the power, 2016-04-12 11:22:49]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Tobias Sargeant, 2016-04-12 12:48:02]

On 2016/04/12 11:22:49, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
>
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

Those link errors don't happen locally, but I guess it's indicative of the fact
that code in gpu/ probably shouldn't be relying on skia in this way.

[no sievers, 2016-04-12 18:12:02]

Yea the failed Android builds seem to be component builds, so it looks like
there is a missing dependency. If gfx::Transform uses skia, it seems to me like
then that component should depend on skia.

https://codereview.chromium.org/1818073002/diff/160001/gpu/command_buffer/ser...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1818073002/diff/160001/gpu/command_buffer/ser...
gpu/command_buffer/service/gles2_cmd_decoder.cc:7637:
image->GetTextureMatrix(gl_matrix);
nit: Maybe GLStreamTextureImage can then even fill in the gfx::Transform
directly?

[Tobias Sargeant, 2016-04-12 18:43:36]

On 2016/04/12 18:12:02, sievers wrote:
> Yea the failed Android builds seem to be component builds, so it looks like
> there is a missing dependency. If gfx::Transform uses skia, it seems to me
like
> then that component should depend on skia.

Actually, they're only component gyp builds. Component gn builds work fine it
seems.

> nit: Maybe GLStreamTextureImage can then even fill in the gfx::Transform
> directly?

Could do, but then the texture copy path would get more verbose. It seems like
it would be advantageous if gfx::Transform had a constructor from float[16]
(probably with an enum to mark it as column- or row-major).

[Tobias Sargeant, 2016-05-24 10:48:59]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-24 10:49:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1818073002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1818073002/180001

[commit-bot: I haz the power, 2016-05-24 11:18:45]

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

[commit-bot: I haz the power, 2016-05-24 11:18:46]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[no sievers, 2016-07-01 20:44:51]

lgtm

[Tobias Sargeant, 2016-07-11 11:15:53]

Enne, are you ok with the cc/ changes?

[enne (OOO), 2016-07-11 18:05:12]

lgtm

[Tobias Sargeant, 2016-07-14 08:24:05]

tobiasjs@chromium.org changed reviewers:
+ jam@chromium.org

[Tobias Sargeant, 2016-07-14 08:34:42]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-14 08:35:08]

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

[Tobias Sargeant, 2016-07-14 08:38:19]

tobiasjs@chromium.org changed reviewers:
- jam@chromium.org, sky@chromium.org

[commit-bot: I haz the power, 2016-07-14 10:16:22]

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

[commit-bot: I haz the power, 2016-07-14 10:16:23]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Tobias Sargeant, 2016-07-14 10:30:09]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2016-07-14 10:30:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
liberato@chromium.org, sievers@chromium.org, enne@chromium.org
Link to the patchset: https://codereview.chromium.org/1818073002/#ps220001
(title: "Re-run build_gles2_cmd_buffer.py")

[commit-bot: I haz the power, 2016-07-14 10:30:24]

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-14 11:31:25]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.win
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-14 13:45:57]

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

[commit-bot: I haz the power, 2016-07-14 13:45:59]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Tobias Sargeant, 2016-07-14 15:35:06]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2016-07-14 15:35:33]

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-14 15:35:37]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.win
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-14 17:08:08]

Description was changed from

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-07-14 17:08:10]

Committed patchset #11 (id:220001)

[commit-bot: I haz the power, 2016-07-14 17:08:14]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 17:10:15]

Description was changed from

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Cr-Commit-Position: refs/heads/master@{#405507}
==========

[commit-bot: I haz the power, 2016-07-14 17:10:17]

Patchset 11 (id:??) landed as
https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Cr-Commit-Position: refs/heads/master@{#405507}

[pdr., 2016-07-14 17:35:44]

On 2016/07/14 at 17:10:17, commit-bot wrote:
> Patchset 11 (id:??) landed as
https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
> Cr-Commit-Position: refs/heads/master@{#405507}

I'm getting a linker error due to this patch:
FAILED: libgpu.dylib libgpu.dylib.TOC 
...
Undefined symbols for architecture x86_64:
  "SkMatrix44::setColMajorf(float const*)", referenced from:
     
gpu::gles2::GLES2DecoderImpl::DoUniformMatrix4fvStreamTextureMatrixCHROMIUM(int,
unsigned char, float const*) in gpu.gles2_cmd_decoder.o
  "SkMatrix44::setConcat(SkMatrix44 const&, SkMatrix44 const&)", referenced
from:
      SkMatrix44::SkMatrix44(SkMatrix44 const&, SkMatrix44 const&) in
gpu.gles2_cmd_decoder.o
  "SkMatrix44::asColMajorf(float*) const", referenced from:
     
gpu::gles2::GLES2DecoderImpl::DoUniformMatrix4fvStreamTextureMatrixCHROMIUM(int,
unsigned char, float const*) in gpu.gles2_cmd_decoder.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

[pdr., 2016-07-14 17:37:36]

Looks like the debug bot is failing as well:
https://build.chromium.org/p/chromium.mac/waterfall?builder=Mac%20GYP%20(dbg)

[wjmaclean, 2016-07-14 17:40:30]

A revert of this CL (patchset #11 id:220001) has been created in
https://codereview.chromium.org/2150993002/ by wjmaclean@chromium.org.

The reason for reverting is: Broke the build on Mac GYP (dbg):

https://build.chromium.org/p/chromium.mac/builders/Mac%20GYP%20(dbg).

[Tobias Sargeant, 2016-07-25 14:44:34]

Description was changed from

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Cr-Commit-Position: refs/heads/master@{#405507}
==========

to

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Cr-Commit-Position: refs/heads/master@{#405507}
==========

[Tobias Sargeant, 2016-08-03 13:29:02]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 13:29:16]

Dry run: 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-08-03 13:29:22]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.win
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-08-03 13:34:04]

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

[commit-bot: I haz the power, 2016-08-03 13:34:05]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Tobias Sargeant, 2016-08-03 13:41:19]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 13:41:34]

Dry run: 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-08-03 13:41:37]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.win
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-08-03 16:42:45]

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

[commit-bot: I haz the power, 2016-08-03 16:42:47]

Dry run: This issue passed the CQ dry run.

[Tobias Sargeant, 2016-08-03 16:50:56]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2016-08-03 16:50:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
sievers@chromium.org, enne@chromium.org, liberato@chromium.org
Link to the patchset: https://codereview.chromium.org/1818073002/#ps240001
(title: "rebase [If you wait by the river long enough, the bodies of your gyp
files will float by]")

[commit-bot: I haz the power, 2016-08-03 16:51:34]

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-08-03 16:51:38]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.win
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-08-03 16:56:26]

Description was changed from

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Cr-Commit-Position: refs/heads/master@{#405507}
==========

to

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Cr-Commit-Position: refs/heads/master@{#405507}
==========

[commit-bot: I haz the power, 2016-08-03 16:56:28]

Committed patchset #12 (id:240001)

[commit-bot: I haz the power, 2016-08-03 16:58:30]

Description was changed from

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Cr-Commit-Position: refs/heads/master@{#405507}
==========

to

==========
Always apply UniformMatrix4fvStreamTextureMatrixCHROMIUM matrix argument.

Regardless of whether we have a stream texture matrix, we should apply
the transformation matrix held by the StreamTextureDrawQuad instance.

Change the implementation of the extension to always premultiply either
the stream texture matrix or the identity matrix by the passed
transformation matrix.

BUG=597681
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/5897d75b89294c388df2ec16b7519ecd635cc148
Committed: https://crrev.com/fc9d613c03b8e72ddfbc9f43d87119d29f96fdee
Cr-Original-Commit-Position: refs/heads/master@{#405507}
Cr-Commit-Position: refs/heads/master@{#409543}
==========

[commit-bot: I haz the power, 2016-08-03 16:58:32]

Patchset 12 (id:??) landed as
https://crrev.com/fc9d613c03b8e72ddfbc9f43d87119d29f96fdee
Cr-Commit-Position: refs/heads/master@{#409543}