Allow client to force an SkImage snapshot to be unique (and uniquely own its backing store).

Allow client to force an SkImage snapshot to be unique (and uniquely own its backing store).
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1686163002

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

[bsalomon, 2016-02-10 18:42:09]

Description was changed from

==========
Allow client to force an SkImage snapshot to be unique (and uniquely own its
backing store).
==========

to

==========
Allow client to force an SkImage snapshot to be unique (and uniquely own its
backing store).
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2016-02-12 21:57:22]

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

[bsalomon, 2016-02-12 22:17:16]

Follow up that fixes the added TODO here:
https://codereview.chromium.org/1694943002

[reed1, 2016-02-15 21:32:14]

https://codereview.chromium.org/1686163002/diff/40001/include/core/SkSurface.h
File include/core/SkSurface.h (right):

https://codereview.chromium.org/1686163002/diff/40001/include/core/SkSurface....
include/core/SkSurface.h:248: SkImage* newImageSnapshot(Budgeted =
kYes_Budgeted, ForceUnique = kNo_ForceUnique);
Crud. I think its cruddy when we have more than one default arg. I had forgotten
that we had budgeted. I wonder if we should just have a 2nd version that
explicitly requires both enums..?

[bsalomon, 2016-02-16 14:25:22]

On 2016/02/15 21:32:14, reed1 wrote:
> https://codereview.chromium.org/1686163002/diff/40001/include/core/SkSurface.h
> File include/core/SkSurface.h (right):
> 
>
https://codereview.chromium.org/1686163002/diff/40001/include/core/SkSurface....
> include/core/SkSurface.h:248: SkImage* newImageSnapshot(Budgeted =
> kYes_Budgeted, ForceUnique = kNo_ForceUnique);
> Crud. I think its cruddy when we have more than one default arg. I had
forgotten
> that we had budgeted. I wonder if we should just have a 2nd version that
> explicitly requires both enums..?

I agree, will do.

[bsalomon, 2016-02-16 15:26:37]

PTAL, new version adds a overload that requires both enum values be specified.

[reed1, 2016-02-16 16:01:51]

lgtm

[reed1, 2016-02-16 16:03:16]

Actually, I think we should consider augmenting the dox to reference the exact
bug we're working around... In the absence of that bug (which at some point will
be fixed or unimportant I presume) would we offer this enum at all? I'd just
like to have a reminder that this may be a complication we can remove from the
API in the future.

[bsalomon, 2016-02-22 16:27:38]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-02-22 16:27:38]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1686163002/#ps80001
(title: "add bug number ot comment")

[commit-bot: I haz the power, 2016-02-22 16:27:44]

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

[commit-bot: I haz the power, 2016-02-22 16:30:23]

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

[commit-bot: I haz the power, 2016-02-22 16:30:23]

Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[bsalomon, 2016-02-22 18:34:59]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-02-22 18:35:00]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1686163002/#ps100001
(title: "Fix enum to bool warning")

[commit-bot: I haz the power, 2016-02-22 18:35:07]

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

[commit-bot: I haz the power, 2016-02-22 19:03:00]

Description was changed from

==========
Allow client to force an SkImage snapshot to be unique (and uniquely own its
backing store).
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Allow client to force an SkImage snapshot to be unique (and uniquely own its
backing store).
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/f47b9a3b88a037a481eb70f01a4cf9f5be34dc28
==========

[commit-bot: I haz the power, 2016-02-22 19:03:01]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/f47b9a3b88a037a481eb70f01a4cf9f5be34dc28

[brucedawson, 2016-02-25 01:27:25]

brucedawson@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2016-02-25 01:27:26]

https://codereview.chromium.org/1686163002/diff/100001/src/image/SkSurface_Gp...
File src/image/SkSurface_Gpu.cpp (right):

https://codereview.chromium.org/1686163002/diff/100001/src/image/SkSurface_Gp...
src/image/SkSurface_Gpu.cpp:88: desc.fFlags = desc.fFlags &
!kRenderTarget_GrSurfaceFlag;
This line looks odd (/analyze flagged it as odd). Why is a logical not of an
enum value being done? Should this be ~ instead of !?

This works out to "& 0;"

The actual warning was:
    third_party\skia\include\gpu\grtypes.h(410) : warning C6323: Use of
arithmetic operator on Boolean type(s).
with a pointer back to this line of code.

[bsalomon, 2016-02-25 01:37:41]

https://codereview.chromium.org/1686163002/diff/100001/src/image/SkSurface_Gp...
File src/image/SkSurface_Gpu.cpp (right):

https://codereview.chromium.org/1686163002/diff/100001/src/image/SkSurface_Gp...
src/image/SkSurface_Gpu.cpp:88: desc.fFlags = desc.fFlags &
!kRenderTarget_GrSurfaceFlag;
On 2016/02/25 01:27:26, brucedawson wrote:
> This line looks odd (/analyze flagged it as odd). Why is a logical not of an
> enum value being done? Should this be ~ instead of !?
> 
> This works out to "& 0;"
> 
> The actual warning was:
>     third_party\skia\include\gpu\grtypes.h(410) : warning C6323: Use of
> arithmetic operator on Boolean type(s).
> with a pointer back to this line of code.

d'oh. Yes, should be ~, will fix.