add SkImage::NewFromBitmap

add SkImage::NewFromBitmap

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/5617900063f69754de62b8b12d32f6e36df14104

[reed2, 2015-07-04 04:47:52]

reed@chromium.org changed reviewers:
+ bsalomon@google.com, robertphillips@google.com

[reed2, 2015-07-04 04:47:53]

TODO:
- add explicit tests
- picture's hasBitmaps should be extended to also consider images

[robertphillips, 2015-07-06 20:43:47]

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp
File gm/colormatrix.cpp (right):

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:31: 
set_color_matrix ?

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:35: 
set_array ?

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:41: SkDoOnce fOnce;
Can this not occur in onOnceBeforeDraw ?

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:54: protected:
override(s) ?

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:62: 
Can createSolidBitmap & createTransparentBitmap be static ?

https://codereview.chromium.org/1217053003/diff/80001/include/core/SkImage.h
File include/core/SkImage.h (right):

https://codereview.chromium.org/1217053003/diff/80001/include/core/SkImage.h#...
include/core/SkImage.h:65: *  Construct a new image from the specified bitmap.
If the bitmap is marked immutable, and
Supposedly "shareable" prevails by a 2:1 margin in American texts

https://codereview.chromium.org/1217053003/diff/80001/src/core/SkRecorder.cpp
File src/core/SkRecorder.cpp (left):

https://codereview.chromium.org/1217053003/diff/80001/src/core/SkRecorder.cpp...
src/core/SkRecorder.cpp:181: TRY_MINIRECORDER(drawBitmapRectToRect, bitmap, src,
dst, paint, flags);
What happened to this code?

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImagePriv.h
File src/image/SkImagePriv.h (right):

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImagePriv.h...
src/image/SkImagePriv.h:29: */
Why Raster when it can be GPU backed ?

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImage_Gpu.c...
src/image/SkImage_Gpu.cpp:251: // need to be a rendertarget for readpixels to
work, instead of kNone_GrSurfaceFlags
Should the copied texture inherit the src's kCheckAllocation_GrSurfaceFlag ?

[mtklein, 2015-07-06 21:02:26]

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

[mtklein, 2015-07-06 21:02:27]

https://codereview.chromium.org/1217053003/diff/80001/src/core/SkRecorder.cpp
File src/core/SkRecorder.cpp (right):

https://codereview.chromium.org/1217053003/diff/80001/src/core/SkRecorder.cpp...
src/core/SkRecorder.cpp:196: TRY_MINIRECORDER(drawBitmapRectToRect, bitmap, src,
dst, paint, flags);
I support this approach both as a hack and a long term change to pictures.  That
said, if we do it for real, we'll need to implement drawImageRect() on
SkMiniRecorder to keep up the status quo here.  (It's a near trivial change.) 
Otherwise we'll use SkBigPictures for all those singleton drawBitmapRect()
calls.

[reed2, 2015-07-07 01:18:03]

ptal

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp
File gm/colormatrix.cpp (right):

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:31: 
On 2015/07/06 20:43:47, robertphillips wrote:
> set_color_matrix ?

Done.

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:35: 
On 2015/07/06 20:43:47, robertphillips wrote:
> set_array ?

Done.

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:41: SkDoOnce fOnce;
On 2015/07/06 20:43:47, robertphillips wrote:
> Can this not occur in onOnceBeforeDraw ?

Done.

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:54: protected:
On 2015/07/06 20:43:47, robertphillips wrote:
> override(s) ?

Done.

https://codereview.chromium.org/1217053003/diff/80001/gm/colormatrix.cpp#newc...
gm/colormatrix.cpp:62: 
On 2015/07/06 20:43:47, robertphillips wrote:
> Can createSolidBitmap & createTransparentBitmap be static ?

Done.

https://codereview.chromium.org/1217053003/diff/80001/include/core/SkImage.h
File include/core/SkImage.h (right):

https://codereview.chromium.org/1217053003/diff/80001/include/core/SkImage.h#...
include/core/SkImage.h:65: *  Construct a new image from the specified bitmap.
If the bitmap is marked immutable, and
On 2015/07/06 20:43:47, robertphillips wrote:
> Supposedly "shareable" prevails by a 2:1 margin in American texts

Done.

https://codereview.chromium.org/1217053003/diff/80001/src/core/SkRecorder.cpp
File src/core/SkRecorder.cpp (left):

https://codereview.chromium.org/1217053003/diff/80001/src/core/SkRecorder.cpp...
src/core/SkRecorder.cpp:181: TRY_MINIRECORDER(drawBitmapRectToRect, bitmap, src,
dst, paint, flags);
On 2015/07/06 20:43:47, robertphillips wrote:
> What happened to this code?

Twas lost in a manual merge. Restored.

https://codereview.chromium.org/1217053003/diff/80001/src/core/SkRecorder.cpp
File src/core/SkRecorder.cpp (right):

https://codereview.chromium.org/1217053003/diff/80001/src/core/SkRecorder.cpp...
src/core/SkRecorder.cpp:196: TRY_MINIRECORDER(drawBitmapRectToRect, bitmap, src,
dst, paint, flags);
On 2015/07/06 21:02:27, mtklein wrote:
> I support this approach both as a hack and a long term change to pictures. 
That
> said, if we do it for real, we'll need to implement drawImageRect() on
> SkMiniRecorder to keep up the status quo here.  (It's a near trivial change.) 
> Otherwise we'll use SkBigPictures for all those singleton drawBitmapRect()
> calls.

Gotcha. Yes, this is just a testing-experiment for now.

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImagePriv.h
File src/image/SkImagePriv.h (right):

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImagePriv.h...
src/image/SkImagePriv.h:29: */
On 2015/07/06 20:43:47, robertphillips wrote:
> Why Raster when it can be GPU backed ?

This is part of the cascade of dissecting a generic bitmap. This can only be
called if the bitmap is NOT texture-backed.

updating dox (there is already an assert in the impl).

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImage_Gpu.c...
src/image/SkImage_Gpu.cpp:251: // need to be a rendertarget for readpixels to
work, instead of kNone_GrSurfaceFlags
On 2015/07/06 20:43:47, robertphillips wrote:
> Should the copied texture inherit the src's kCheckAllocation_GrSurfaceFlag ?

You tell me. I didn't know about that flag. What is its intent?

[reed2, 2015-07-07 01:18:18]

The CQ bit was checked by reed@chromium.org

[commit-bot: I haz the power, 2015-07-07 01:18:37]

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

[commit-bot: I haz the power, 2015-07-07 01:18:39]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-07-07 07:18 UTC

[commit-bot: I haz the power, 2015-07-07 01:19:48]

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

[commit-bot: I haz the power, 2015-07-07 01:19:49]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[reed2, 2015-07-07 11:58:28]

The CQ bit was checked by reed@chromium.org

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

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

[commit-bot: I haz the power, 2015-07-07 11:58:43]

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

[commit-bot: I haz the power, 2015-07-07 11:58:44]

No LGTM from a valid reviewer yet. Please ask for an LGTM from a
full Skia committer from
https://skia.googlesource.com/skia/+/master/CQ_COMMITTERS

[robertphillips, 2015-07-07 12:23:59]

lgtm + nits

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.chromium.org/1217053003/diff/80001/src/image/SkImage_Gpu.c...
src/image/SkImage_Gpu.cpp:251: // need to be a rendertarget for readpixels to
work, instead of kNone_GrSurfaceFlags
On 2015/07/07 01:18:03, reed2 wrote:
> On 2015/07/06 20:43:47, robertphillips wrote:
> > Should the copied texture inherit the src's kCheckAllocation_GrSurfaceFlag ?
> 
> You tell me. I didn't know about that flag. What is its intent?

Tracing through the code it looks like this should be fine. I wonder if we can
remove the kCheckAllocation flag from the public API though.

https://codereview.chromium.org/1217053003/diff/140001/gm/colormatrix.cpp
File gm/colormatrix.cpp (right):

https://codereview.chromium.org/1217053003/diff/140001/gm/colormatrix.cpp#new...
gm/colormatrix.cpp:15: 
Can we delete this guy now?

https://codereview.chromium.org/1217053003/diff/140001/src/image/SkImage.cpp
File src/image/SkImage.cpp (right):

https://codereview.chromium.org/1217053003/diff/140001/src/image/SkImage.cpp#...
src/image/SkImage.cpp:238: 
These two consts should probably go inside the SK_SUPPORT_GPU block

[reed2, 2015-07-07 12:58:24]

https://codereview.chromium.org/1217053003/diff/140001/gm/colormatrix.cpp
File gm/colormatrix.cpp (right):

https://codereview.chromium.org/1217053003/diff/140001/gm/colormatrix.cpp#new...
gm/colormatrix.cpp:15: 
On 2015/07/07 12:23:59, robertphillips wrote:
> Can we delete this guy now?

Done.

https://codereview.chromium.org/1217053003/diff/140001/src/image/SkImage.cpp
File src/image/SkImage.cpp (right):

https://codereview.chromium.org/1217053003/diff/140001/src/image/SkImage.cpp#...
src/image/SkImage.cpp:238: 
On 2015/07/07 12:23:59, robertphillips wrote:
> These two consts should probably go inside the SK_SUPPORT_GPU block

Done.

[reed2, 2015-07-07 13:01:37]

The CQ bit was checked by reed@chromium.org

[reed2, 2015-07-07 13:01:38]

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

[commit-bot: I haz the power, 2015-07-07 13:01:48]

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

[reed2, 2015-07-07 13:04:02]

The CQ bit was unchecked by reed@chromium.org

[reed2, 2015-07-07 13:04:09]

The CQ bit was checked by reed@chromium.org

[commit-bot: I haz the power, 2015-07-07 13:04:16]

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

[commit-bot: I haz the power, 2015-07-07 13:11:22]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/5617900063f69754de62b8b12d32f6e36df14104