Move copy-surface-as-draw fallback to GrGLGpu.

Move copy-surface-as-draw fallback to GrGLGpu.

Committed: https://skia.googlesource.com/skia/+/5df6fee929823f50c55cc50f7c882a309c1b1de9

[bsalomon, 2015-05-13 17:57:34]

bsalomon@google.com changed reviewers:
+ egdaniel@google.com, joshualitt@google.com

[egdaniel, 2015-05-13 19:04:42]

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp
File src/gpu/gl/GrGLGpu.cpp (right):

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp#...
src/gpu/gl/GrGLGpu.cpp:204: this->createCopyProgram();
you call this twice?

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp#...
src/gpu/gl/GrGLGpu.cpp:2806: auto attribs =
fHWGeometryState.bindArrayAndBufferToDraw(this, fCopyProgram.fArrayBuffer);
I think this is not the best use case for auto since there really is no way to
know what attribs is here.

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp#...
src/gpu/gl/GrGLGpu.cpp:2814: GrGLfloat dx0 = 2.f * dstPoint.fX / dw - 1.f;
little confused at what these equations are doing

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLVertexAr...
File src/gpu/gl/GrGLVertexArray.h (right):

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLVertexAr...
src/gpu/gl/GrGLVertexArray.h:79: 
extra \n?

[joshualitt, 2015-05-13 19:23:17]

lgtm + nit

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp
File src/gpu/gl/GrGLGpu.cpp (right):

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp#...
src/gpu/gl/GrGLGpu.cpp:2715: "void main() {"
Add comment to vshader and fshader so we know when we see in the spew where
these come from

[bsalomon, 2015-05-14 21:27:14]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2015-05-14 21:27:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1144433002/#ps100001
(title: "more")

[commit-bot: I haz the power, 2015-05-14 21:27:38]

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

[bsalomon, 2015-05-14 21:32:46]

The CQ bit was unchecked by bsalomon@google.com

[bsalomon, 2015-05-15 17:56:31]

Can you guys take a look at the changes between what you reviewed (PS#3) and the
most recent PS?

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp
File src/gpu/gl/GrGLGpu.cpp (right):

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp#...
src/gpu/gl/GrGLGpu.cpp:204: this->createCopyProgram();
On 2015/05/13 19:04:42, egdaniel wrote:
> you call this twice?

Done.

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp#...
src/gpu/gl/GrGLGpu.cpp:2715: "void main() {"
On 2015/05/13 19:23:17, joshualitt wrote:
> Add comment to vshader and fshader so we know when we see in the spew where
> these come from

Done.

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp#...
src/gpu/gl/GrGLGpu.cpp:2806: auto attribs =
fHWGeometryState.bindArrayAndBufferToDraw(this, fCopyProgram.fArrayBuffer);
On 2015/05/13 19:04:42, egdaniel wrote:
> I think this is not the best use case for auto since there really is no way to
> know what attribs is here.

Done.

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLGpu.cpp#...
src/gpu/gl/GrGLGpu.cpp:2814: GrGLfloat dx0 = 2.f * dstPoint.fX / dw - 1.f;
On 2015/05/13 19:04:42, egdaniel wrote:
> little confused at what these equations are doing

Done.

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLVertexAr...
File src/gpu/gl/GrGLVertexArray.h (right):

https://codereview.chromium.org/1144433002/diff/40001/src/gpu/gl/GrGLVertexAr...
src/gpu/gl/GrGLVertexArray.h:79: 
On 2015/05/13 19:04:42, egdaniel wrote:
> extra \n?

Done.

[egdaniel, 2015-05-15 19:36:05]

looks good

[bsalomon, 2015-05-18 13:20:05]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2015-05-18 13:20:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1144433002/#ps220001
(title: "remove incorrect assert")

[commit-bot: I haz the power, 2015-05-18 13:20:13]

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

[commit-bot: I haz the power, 2015-05-18 13:26:22]

Committed patchset #12 (id:220001) as
https://skia.googlesource.com/skia/+/5df6fee929823f50c55cc50f7c882a309c1b1de9

[mtklein, 2015-05-18 16:28:33]

A revert of this CL (patchset #12 id:220001) has been created in
https://codereview.chromium.org/1142003003/ by mtklein@google.com.

The reason for reverting is: Many GMs changed, some totally busted..

[qiankun, 2015-05-22 12:54:45]

On 2015/05/18 16:28:33, mtklein wrote:
> A revert of this CL (patchset #12 id:220001) has been created in
> https://codereview.chromium.org/1142003003/ by mailto:mtklein@google.com.
> 
> The reason for reverting is: Many GMs changed, some totally busted..

Hi brian, will you try to land this CL again with GMs fixed?

[bsalomon, 2015-05-22 14:39:34]

On 2015/05/22 12:54:45, qiankun wrote:
> On 2015/05/18 16:28:33, mtklein wrote:
> > A revert of this CL (patchset #12 id:220001) has been created in
> > https://codereview.chromium.org/1142003003/ by mailto:mtklein@google.com.
> > 
> > The reason for reverting is: Many GMs changed, some totally busted..
> 
> Hi brian, will you try to land this CL again with GMs fixed?

Hi, yes I'm working on it. I'm not exactly clear on what went wrong on that
device but once I have it sorted out, I'll reland this.

[joshualitt, 2015-06-01 17:20:07]

On 2015/05/22 14:39:34, bsalomon wrote:
> On 2015/05/22 12:54:45, qiankun wrote:
> > On 2015/05/18 16:28:33, mtklein wrote:
> > > A revert of this CL (patchset #12 id:220001) has been created in
> > > https://codereview.chromium.org/1142003003/ by mailto:mtklein@google.com.
> > > 
> > > The reason for reverting is: Many GMs changed, some totally busted..
> > 
> > Hi brian, will you try to land this CL again with GMs fixed?
> 
> Hi, yes I'm working on it. I'm not exactly clear on what went wrong on that
> device but once I have it sorted out, I'll reland this.

lgtm

[baleboy, 2015-06-05 05:16:43]

On 2015/05/22 14:39:34, bsalomon wrote:
> On 2015/05/22 12:54:45, qiankun wrote:
> > On 2015/05/18 16:28:33, mtklein wrote:
> > > A revert of this CL (patchset #12 id:220001) has been created in
> > > https://codereview.chromium.org/1142003003/ by mailto:mtklein@google.com.
> > > 
> > > The reason for reverting is: Many GMs changed, some totally busted..
> > 
> > Hi brian, will you try to land this CL again with GMs fixed?
> 
> Hi, yes I'm working on it. I'm not exactly clear on what went wrong on that
> device but once I have it sorted out, I'll reland this.

Hello, I was wondering whether there were any clearer idea on when the change
could re-land? I'm part of the Crosswalk team and many users are asking for a
resolution. Thanks!

[bsalomon, 2015-06-10 15:59:12]

On 2015/06/05 05:16:43, baleboy wrote:
> On 2015/05/22 14:39:34, bsalomon wrote:
> > On 2015/05/22 12:54:45, qiankun wrote:
> > > On 2015/05/18 16:28:33, mtklein wrote:
> > > > A revert of this CL (patchset #12 id:220001) has been created in
> > > > https://codereview.chromium.org/1142003003/ by
mailto:mtklein@google.com.
> > > > 
> > > > The reason for reverting is: Many GMs changed, some totally busted..
> > > 
> > > Hi brian, will you try to land this CL again with GMs fixed?
> > 
> > Hi, yes I'm working on it. I'm not exactly clear on what went wrong on that
> > device but once I have it sorted out, I'll reland this.
> 
> Hello, I was wondering whether there were any clearer idea on when the change
> could re-land? I'm part of the Crosswalk team and many users are asking for a
> resolution. Thanks!

H,i it was relanded here: https://codereview.chromium.org/1161063003/