Add SkEncodedFormat, used by SkCodec.

Add SkEncodedFormat, used by SkCodec.

Needed by Android to determine the format.

Committed: https://skia.googlesource.com/skia/+/1dd3ea9d4523436578c86dc8d8f43d63fa188c01

[scroggo, 2015-03-18 16:59:49]

scroggo@google.com changed reviewers:
+ djsollen@google.com, msarett@google.com

[djsollen, 2015-03-18 17:35:55]

lgtm

[scroggo, 2015-03-18 17:57:42]

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

[scroggo, 2015-03-18 17:57:42]

Some questions brought up in a live conversation with reed@:

> Why enums and not strings?

The Java API actually wants Java Strings. I chose to use enums because I thought
it would be simpler than converting a C string into a Java String. Further, if
someone wants to make a decision of what to do based on the format, an enum
seems to be a more efficient/clearer way to do that. (It's also how
SkImageDecoder returns this information.) That said, using an enum has a
separate problem, which is that we need to keep the enum in sync with the Java
code, which uses the enum as an index into an array of strings.

Some advantages reed@ brought up for strings:
> We can add new ones without updating the header with an ever expanding
> enum.
> A string can have more fine tuned information like versioning.

> Why do we need this at all?

The main motivation is that the API we are replacing provides this information.
https://googleplex-android-review.git.corp.google.com/#/c/610752/1/graphics/j...
has some (slightly) more specific reasons for including it.

> Why not share enums (i.e. with SkImageEncoder)?

That seems a fine approach to me. That said, if you'd prefer strings, I think
that's fine, too.

[scroggo, 2015-03-19 20:00:21]

> Why not share enums (i.e. with SkImageEncoder)?

Done.

The interesting bits in patch set 3 are in include (not to say there couldn't be
any errors in the other files...)

https://codereview.chromium.org/1018953003/diff/40001/include/core/SkEncodedF...
File include/core/SkEncodedFormat.h (right):

https://codereview.chromium.org/1018953003/diff/40001/include/core/SkEncodedF...
include/core/SkEncodedFormat.h:14: enum SkEncodedFormat {
New shared enum. I placed it in core, since SkImageEncoder (and SkImageDecoder)
is in core, which depends on it now.

https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageDec...
File include/core/SkImageDecoder.h (left):

https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageDec...
include/core/SkImageDecoder.h:29: enum Format {
I could leave this unchanged, since we're removing the class anyway...

https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageEnc...
File include/core/SkImageEncoder.h (left):

https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageEnc...
include/core/SkImageEncoder.h:29: kKTX_Type,
There's a slight difference between this and the new version. A couple of Types
precede kKTX in SkEncodedFormat. I don't think anyone depends on these values
(Android doesn't use kKTX). The ones that Android cares about have the same
order.

https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageEnc...
File include/core/SkImageEncoder.h (right):

https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageEnc...
include/core/SkImageEncoder.h:21: static SkImageEncoder*
Create(SkEncodedFormat);
Now that we've changed this (and SkImageDecoder), we'll need a (simple) change
to Android to update to the new name.

[scroggo, 2015-03-20 18:31:42]

On 2015/03/19 20:00:21, scroggo wrote:
> > Why not share enums (i.e. with SkImageEncoder)?
> 
> Done.
> 
> The interesting bits in patch set 3 are in include (not to say there couldn't
be
> any errors in the other files...)
> 
>
https://codereview.chromium.org/1018953003/diff/40001/include/core/SkEncodedF...
> File include/core/SkEncodedFormat.h (right):
> 
>
https://codereview.chromium.org/1018953003/diff/40001/include/core/SkEncodedF...
> include/core/SkEncodedFormat.h:14: enum SkEncodedFormat {
> New shared enum. I placed it in core, since SkImageEncoder (and
SkImageDecoder)
> is in core, which depends on it now.
> 
>
https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageDec...
> File include/core/SkImageDecoder.h (left):
> 
>
https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageDec...
> include/core/SkImageDecoder.h:29: enum Format {
> I could leave this unchanged, since we're removing the class anyway...
> 
>
https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageEnc...
> File include/core/SkImageEncoder.h (left):
> 
>
https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageEnc...
> include/core/SkImageEncoder.h:29: kKTX_Type,
> There's a slight difference between this and the new version. A couple of
Types
> precede kKTX in SkEncodedFormat. I don't think anyone depends on these values
> (Android doesn't use kKTX). The ones that Android cares about have the same
> order.
> 
>
https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageEnc...
> File include/core/SkImageEncoder.h (right):
> 
>
https://codereview.chromium.org/1018953003/diff/40001/include/core/SkImageEnc...
> include/core/SkImageEncoder.h:21: static SkImageEncoder*
> Create(SkEncodedFormat);
> Now that we've changed this (and SkImageDecoder), we'll need a (simple) change
> to Android to update to the new name.

For simplicity, patch set 5 just changes SkCodec to use the common one, with
TODOs for the others. When I have some more time to handle the three way merge,
I'll take rebase patch set 4 and get it in as a cleanup.

[scroggo, 2015-03-20 18:31:46]

The CQ bit was checked by scroggo@google.com

[scroggo, 2015-03-20 18:31:47]

The patchset sent to the CQ was uploaded after l-g-t-m from djsollen@google.com
Link to the patchset: https://codereview.chromium.org/1018953003/#ps80001
(title: "Just change SkCodec, with TODO for the others.")

[commit-bot: I haz the power, 2015-03-20 18:32:10]

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

[commit-bot: I haz the power, 2015-03-20 18:55:59]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/1dd3ea9d4523436578c86dc8d8f43d63fa188c01