Add support for serializing/deserializing of SkDrawable

Add support for serializing/deserializing of SkDrawable

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1837913003

[msarett, 2016-03-28 21:15:09]

Description was changed from

==========
Add support for serializing/deserializing of SkDrawable

BUG=skia:
==========

to

==========
Add support for serializing/deserializing of SkDrawable

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[msarett, 2016-03-28 21:29:30]

Patchset #1 (id:1) has been deleted

[msarett, 2016-03-28 21:32:07]

Patchset #2 (id:40001) has been deleted

[msarett, 2016-03-28 21:32:13]

Patchset #1 (id:20001) has been deleted

[msarett, 2016-03-28 21:41:08]

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

[msarett, 2016-03-28 21:41:08]

Here's a first attempt.

I'm not really thrilled about using SkTRegistry, but I haven't found another way
to do this without giving SkDrawable knowledge of its subclasses.

It would be nice to use SkFlattenable, but SkFlattenable is set up to require an
enum for every Factory that is registered with it - I don't think we want
classes that are external to Skia needing to be named in this enum.

Also, I'm assuming that this needs to be hooked into skp
serializing/deserializing somehow?

[msarett, 2016-03-28 23:29:40]

Patchset #2 (id:80001) has been deleted

[msarett, 2016-03-28 23:31:35]

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

[msarett, 2016-03-28 23:31:36]

On second look, we can use SkFlattenable (when I use it correctly)...

Thanks to Mike for pushing this.

[msarett, 2016-03-29 15:29:18]

This version adds serialize/deserialize entry points to a header file in
src/core and improves testing to confirm that we can handle drawables within
drawables etc.

https://codereview.chromium.org/1837913003/diff/120001/tests/SerializeDrawabl...
File tests/SerializeDrawableTest.cpp (right):

https://codereview.chromium.org/1837913003/diff/120001/tests/SerializeDrawabl...
tests/SerializeDrawableTest.cpp:192:
SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkIntDrawable)
Still need to think about how to register drawables from Android...

[msarett, 2016-03-30 19:31:59]

Patchset #5 (id:160001) has been deleted

[msarett, 2016-03-30 20:20:14]

Patchset #5 (id:180001) has been deleted

[msarett, 2016-03-30 20:26:09]

Patchset #5 (id:200001) has been deleted

[msarett, 2016-03-30 20:34:11]

Please take a look.  This has gotten more complicated, but I think it gives us
what we want.

I think the next step will just be registering the drawables in Android (and
adding flatten/CreateProc overrides).

I think it's worth mentioning that there's no need to add serialize/deserialize
APIs.  We can use what is already on SkReadBuffer/SkWriteBuffer.
Ex:
writeBuffer.writeFlattenable(/*drawable*/);
...
drawable = readBuffer.readFlattenable();

https://codereview.chromium.org/1837913003/diff/220001/tests/SerializeDrawabl...
File tests/SerializeDrawableTest.cpp (right):

https://codereview.chromium.org/1837913003/diff/220001/tests/SerializeDrawabl...
tests/SerializeDrawableTest.cpp:288: * If we include SkRecordedDrawable.h and
make all of the fields on
I'll remove this code block before submitting.

I didn't really know how to test the playback of canvas commands.  Maybe I'll
write a gm?

[djsollen, 2016-03-31 12:54:00]

I'll defer to mtklein on if this is a good direction to go with the SkPicture
code.

I would like to consider how we could support the following 2 scenarios...

1. have a default factory for SkDrawable that just writes out the drawing
commands at serialization time.  This means that unless you want the subclasses
don't need to do anything special.

2. have a way to register a flattenable factory for just one instance of the
SkReadBuffers.  This would allow the callers to inject custom functions that
would enable the following pattern.

static SkFlattenable* customProc(SkReadBuffer& rb) {
  MyDrawable* md = MyDrawable::CreateProc(rb);
  // custom client logic to track drawable for out-of-band use
  return md;
}

writeBuffer.registerFlattenable(customProc, myDrawable::GetFlattenableType());

[msarett, 2016-03-31 15:52:58]

Patchset #6 (id:240001) has been deleted

[msarett, 2016-03-31 15:58:29]

On 2016/03/31 12:54:00, djsollen wrote:
> I'll defer to mtklein on if this is a good direction to go with the SkPicture
> code.
> 

Sounds good.  This was a first attempt to share all of the SkPicture logic with
drawables.  It is a bit invasive on SkPictures.  I'll definitely discuss with
Mike and iterate on this.

> I would like to consider how we could support the following 2 scenarios...
> 
> 1. have a default factory for SkDrawable that just writes out the drawing
> commands at serialization time.  This means that unless you want the
subclasses
> don't need to do anything special.
> 

Per our discussion in person, let's hold off on this until we are asked for it.

> 2. have a way to register a flattenable factory for just one instance of the
> SkReadBuffers.  This would allow the callers to inject custom functions that
> would enable the following pattern.
> 
> static SkFlattenable* customProc(SkReadBuffer& rb) {
>   MyDrawable* md = MyDrawable::CreateProc(rb);
>   // custom client logic to track drawable for out-of-band use
>   return md;
> }
> 
> writeBuffer.registerFlattenable(customProc, myDrawable::GetFlattenableType());

Added this in the latest Patch Set.

[reed1, 2016-03-31 18:42:04]

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

[reed1, 2016-03-31 18:42:05]

This is a big interesting idea. Lets talk about it in person next Monday or so.

https://codereview.chromium.org/1837913003/diff/260001/include/core/SkDrawable.h
File include/core/SkDrawable.h (right):

https://codereview.chromium.org/1837913003/diff/260001/include/core/SkDrawabl...
include/core/SkDrawable.h:63: virtual Factory getFactory() const { return
nullptr; }
Is this an override of the method in SkFlattenable?

https://codereview.chromium.org/1837913003/diff/260001/include/core/SkFlatten...
File include/core/SkFlattenable.h (right):

https://codereview.chromium.org/1837913003/diff/260001/include/core/SkFlatten...
include/core/SkFlattenable.h:74: kSkDrawable_Type,
do we ever serialize this enum, in which case renumbering them would be tricky?

[msarett, 2016-04-01 14:11:44]

"This is a big interesting idea. Lets talk about it in person
next Monday or so."

Sounds good see you Monday!

------------------------------------

I've added support for custom create procs when the
SkFlattenable has *not* been registered.  As an alternative
to registering these flattenables will need to override
getTypeName() (since the registration function is what
gives flattenables a name).

This is maybe a little awkward, but works.

https://codereview.chromium.org/1837913003/diff/260001/include/core/SkDrawable.h
File include/core/SkDrawable.h (right):

https://codereview.chromium.org/1837913003/diff/260001/include/core/SkDrawabl...
include/core/SkDrawable.h:63: virtual Factory getFactory() const { return
nullptr; }
On 2016/03/31 18:42:05, reed1 wrote:
> Is this an override of the method in SkFlattenable?

Yes!  Fixed.

https://codereview.chromium.org/1837913003/diff/260001/include/core/SkFlatten...
File include/core/SkFlattenable.h (right):

https://codereview.chromium.org/1837913003/diff/260001/include/core/SkFlatten...
include/core/SkFlattenable.h:74: kSkDrawable_Type,
On 2016/03/31 18:42:05, reed1 wrote:
> do we ever serialize this enum, in which case renumbering them would be
tricky?

No we don't serialize the enums.  They don't do much... I actually considered a
CL to delete them.
https://codereview.chromium.org/1834303003/

But they seem pretty harmless, and can be used to suggest casts or for
validation.

[mtklein, 2016-04-01 14:14:53]

> But they seem pretty harmless, and can be used to suggest casts or for
validation.

I'm having a hard time thinking of a scenario where we want to deserialize
something but don't know what type we want to cast it to... I'd be happy to
delete.

[msarett, 2016-04-05 19:12:47]

Patchset #8 (id:300001) has been deleted

[msarett, 2016-04-05 19:41:28]

Patchset #8 (id:320001) has been deleted

[msarett, 2016-04-06 18:30:19]

Patchset #8 (id:340001) has been deleted

[msarett, 2016-04-20 21:42:32]

Patchset #9 (id:380001) has been deleted

[msarett, 2016-04-22 20:16:27]

Patchset #10 (id:420001) has been deleted

[msarett, 2016-04-29 16:53:17]

Description was changed from

==========
Add support for serializing/deserializing of SkDrawable

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Add support for serializing/deserializing of SkDrawable

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========