GrContext::copyTexture->GrContext::copySurface. Add a flush writes pixel ops flag.

GrContext::copyTexture->GrContext::copySurface.

Add a flush writes pixel ops flag.

Add an explicit flush writes for GrSurface.

BUG=skia:2977
Committed: https://skia.googlesource.com/skia/+/cf99b00980b6c9c557e71abf1a7c9f9b21217262

Committed: https://skia.googlesource.com/skia/+/f80bfedc42fde88ae3f8dbd7b21950a22258fd22

[reed1, 2014-10-01 19:42:23]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2014-10-01 19:42:24]

fixes SampleApp for me. lgtm

[bsalomon, 2014-10-01 19:45:54]

bsalomon@google.com changed reviewers:
+ robertphillips@google.com

[bsalomon, 2014-10-01 19:45:54]

The new flag is now used in SampleApp which fixes a bug where SampleApp's raster
results weren't flushed to GrRenderTarget before swap buffers was called.

[robertphillips, 2014-10-02 15:21:53]

Initial set of comments - I would like to take another pass through.

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrContext.h#...
include/gpu/GrContext.h:700: /**
Fix this comment ?

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrContext.h#...
include/gpu/GrContext.h:718: bool copySurface(GrSurface* dst, GrSurface* src) {
Missing ' ' ?

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrContext.h#...
include/gpu/GrContext.h:728: /**
will _have been_ ??

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrSurface.h
File include/gpu/GrSurface.h (right):

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrSurface.h#...
include/gpu/GrSurface.h:115: /**
will have been ?

[robertphillips, 2014-10-02 15:40:41]

lgtm + some suggestions

https://codereview.chromium.org/622663002/diff/20001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/622663002/diff/20001/src/gpu/GrContext.cpp#ne...
src/gpu/GrContext.cpp:1756: 
It seems a bit odd to pass in the surface to check whether to flush the
GrContext. Would it make more sense to alter the GrSurface method that calls
this to check if it has pending writes and then call flush?

https://codereview.chromium.org/622663002/diff/20001/src/gpu/GrSurface.cpp
File src/gpu/GrSurface.cpp (right):

https://codereview.chromium.org/622663002/diff/20001/src/gpu/GrSurface.cpp#ne...
src/gpu/GrSurface.cpp:51: void GrSurface::flushWrites() {
Have this guy check if he has writes and then just straight out flush the
context ?

https://codereview.chromium.org/622663002/diff/20001/src/gpu/SkGrPixelRef.cpp
File src/gpu/SkGrPixelRef.cpp (right):

https://codereview.chromium.org/622663002/diff/20001/src/gpu/SkGrPixelRef.cpp...
src/gpu/SkGrPixelRef.cpp:66: 
It would be nice to have a SkIPoint::Zero or Origin. 
Barring that how about moving the creation of dstPoint to right before the
copySurface call?

[bsalomon, 2014-10-02 17:34:06]

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrContext.h#...
include/gpu/GrContext.h:700: /**
On 2014/10/02 15:21:53, robertphillips wrote:
> Fix this comment ?

Done.

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrContext.h#...
include/gpu/GrContext.h:718: bool copySurface(GrSurface* dst, GrSurface* src) {
On 2014/10/02 15:21:53, robertphillips wrote:
> Missing ' ' ?

Done.

https://codereview.chromium.org/622663002/diff/20001/include/gpu/GrContext.h#...
include/gpu/GrContext.h:728: /**
On 2014/10/02 15:21:53, robertphillips wrote:
> will _have been_ ??

Done.

https://codereview.chromium.org/622663002/diff/20001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/622663002/diff/20001/src/gpu/GrContext.cpp#ne...
src/gpu/GrContext.cpp:1756: 
On 2014/10/02 15:40:40, robertphillips wrote:
> It seems a bit odd to pass in the surface to check whether to flush the
> GrContext. Would it make more sense to alter the GrSurface method that calls
> this to check if it has pending writes and then call flush?

If we ever want to support partial flushing then GrContext will need to know
which object requires the flushing.

https://codereview.chromium.org/622663002/diff/20001/src/gpu/GrSurface.cpp
File src/gpu/GrSurface.cpp (right):

https://codereview.chromium.org/622663002/diff/20001/src/gpu/GrSurface.cpp#ne...
src/gpu/GrSurface.cpp:51: void GrSurface::flushWrites() {
On 2014/10/02 15:40:40, robertphillips wrote:
> Have this guy check if he has writes and then just straight out flush the
> context ?

I think we will want to eventually communicate which object needs to be flushed
down to the flush mechanism so it can do a more minimal flush.

https://codereview.chromium.org/622663002/diff/20001/src/gpu/SkGrPixelRef.cpp
File src/gpu/SkGrPixelRef.cpp (right):

https://codereview.chromium.org/622663002/diff/20001/src/gpu/SkGrPixelRef.cpp...
src/gpu/SkGrPixelRef.cpp:66: 
On 2014/10/02 15:40:41, robertphillips wrote:
> It would be nice to have a SkIPoint::Zero or Origin. 
> Barring that how about moving the creation of dstPoint to right before the
> copySurface call?

Done. (Make(0,0) in call site)

[bsalomon, 2014-10-02 17:34:23]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-10-02 17:35:27]

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

[commit-bot: I haz the power, 2014-10-02 17:42:29]

Committed patchset #3 (id:40001) as cf99b00980b6c9c557e71abf1a7c9f9b21217262

[bsalomon, 2014-10-02 18:22:44]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/621073002/ by bsalomon@google.com.

The reason for reverting is: Breaking GMs on some bots.

[bsalomon, 2014-10-07 02:48:16]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-10-07 02:48:47]

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

[commit-bot: I haz the power, 2014-10-07 11:09:50]

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

[commit-bot: I haz the power, 2014-10-07 11:09:51]

Failed to apply patch for samplecode/SampleApp.cpp:
While running git apply --index -3 -p1;
  error: patch failed: samplecode/SampleApp.cpp:298
  Falling back to three-way merge...
  Applied patch to 'samplecode/SampleApp.cpp' with conflicts.
  U samplecode/SampleApp.cpp

Patch:       samplecode/SampleApp.cpp
Index: samplecode/SampleApp.cpp
diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp
index
1b766b021b2c68c0d86b055ab62043d9d9e68180..e8146f058276ee012ced90ef50f6a0e50662259c
100644
--- a/samplecode/SampleApp.cpp
+++ b/samplecode/SampleApp.cpp
@@ -298,7 +298,8 @@ public:
                                             
SkImageInfo2GrPixelConfig(bm.colorType(),
                                                                       
bm.alphaType()),
                                              bm.getPixels(),
-                                             bm.rowBytes());
+                                             bm.rowBytes(),
+                                             GrContext::kFlushWrites_PixelOp);
             }
         }
 #endif

[bsalomon, 2014-10-07 12:48:15]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-10-07 12:48:25]

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

[commit-bot: I haz the power, 2014-10-07 12:56:06]

Committed patchset #5 (id:80001) as f80bfedc42fde88ae3f8dbd7b21950a22258fd22