Add snapshot entry point to SkPictureRecorder

Add snapshot entry point to SkPictureRecorder

Unfortunately it appears that the Android public API requires this capability. That said, it doesn't seem like an unreasonable thing to support.

[reed1, 2014-04-24 13:28:51]

Can you document *what* exactly is the android behavior? Is it at the Java
level?

[robertphillips, 2014-04-25 13:48:29]

There are two points in Android's java layer where the old Picture's
make-an-in-progress-copy functionality is/was used: in the Picture copy ctor and
the Picture::writeToStream entry point.

The Picture copy ctor is documented as:

Create a picture by making a copy of what has already been recorded in src. The
contents of src are unchanged, and if src changes later, those changes will not
be reflected in this picture.

So Android is relying on the capability there. The writeToStream entry point has
been deprecated and its documentation doesn't mention what happens to the source
picture.

From what I can tell Android has no unit tests that cover the behavior of the
src Picture in either of these cases.

[scroggo, 2014-04-25 14:16:51]

https://codereview.chromium.org/253443003/diff/1/include/core/SkPictureRecord...
File include/core/SkPictureRecorder.h (right):

https://codereview.chromium.org/253443003/diff/1/include/core/SkPictureRecord...
include/core/SkPictureRecorder.h:91: return SkNEW_ARGS(SkPicture, (*fPicture));
I haven't followed along with all the picture changes. Will this allow the
caller to continue recording after they've made the snapshot?

[robertphillips, 2014-04-25 14:55:28]

https://codereview.chromium.org/253443003/diff/1/include/core/SkPictureRecord...
File include/core/SkPictureRecorder.h (right):

https://codereview.chromium.org/253443003/diff/1/include/core/SkPictureRecord...
include/core/SkPictureRecorder.h:91: return SkNEW_ARGS(SkPicture, (*fPicture));
Currently - yes. I believe the desire is to not assume the burden of carrying
such a capability going forward.

[scroggo, 2014-04-25 15:00:44]

https://codereview.chromium.org/253443003/diff/1/include/core/SkPictureRecord...
File include/core/SkPictureRecorder.h (right):

https://codereview.chromium.org/253443003/diff/1/include/core/SkPictureRecord...
include/core/SkPictureRecorder.h:91: return SkNEW_ARGS(SkPicture, (*fPicture));
On 2014/04/25 14:55:28, robertphillips wrote:
> Currently - yes. I believe the desire is to not assume the burden of carrying
> such a capability going forward.

When that day comes, what will the Android approach be? Deprecate the Picture
copy constructor?

Could we have a function in picture recorder that copies the picture recorder?
Or is that a price we don't want to pay?

[robertphillips, 2014-04-25 15:47:05]

Another way around this is to add a specialized picture playback. We could then
do:

int numTrailingRestores = c1->getSaveCount()
p1 = recorder->endRecording()
// p1 is the snapped picture and has the usual extra save at its start and the
usual trailing restoreToCount restores
c2 = recorder->beginRecording()
// c2 has started a picture with the usual leading save
p1->drawSpecial(c2, numTrailingRestores)
// drawSpecial doesn't replay the initial save and the last numTrailingRestores
restores
// c2 is now up to date with c1's state before it was snapped and can accept
additional commands

A problem with this is that the caller would now have to use c2 instead of c1
(the canvas used to create p1).
This could be remedied by tracking the java Canvas associated with the Picture
and swapping out its SkCanvas behind the scenes.

[scroggo, 2014-04-25 17:50:28]

On 2014/04/25 15:47:05, robertphillips wrote:
> Another way around this is to add a specialized picture playback. We could
then
> do:
> 
> int numTrailingRestores = c1->getSaveCount()
> p1 = recorder->endRecording()
> // p1 is the snapped picture and has the usual extra save at its start and the
> usual trailing restoreToCount restores
> c2 = recorder->beginRecording()
> // c2 has started a picture with the usual leading save
> p1->drawSpecial(c2, numTrailingRestores)
> // drawSpecial doesn't replay the initial save and the last
numTrailingRestores
> restores
> // c2 is now up to date with c1's state before it was snapped and can accept
> additional commands

It seems like we would need to do this trick twice - once for the Picture being
copied, and once for the Picture being created - so that both Pictures can still
be recorded to. That seems like a reasonable approach to me.

> 
> A problem with this is that the caller would now have to use c2 instead of c1
> (the canvas used to create p1).
> This could be remedied by tracking the java Canvas associated with the Picture
> and swapping out its SkCanvas behind the scenes.

I think swapping out the SkCanvas internally is fine.

[tomhudson, 2014-04-28 11:07:41]

> There are two points in Android's java layer where the old Picture's
make-an-in-progress-copy functionality is/was used: in the Picture copy ctor and
the Picture::writeToStream entry point.


+Ben: I thought WebView already had a workaround for this? Would it be
generalizable to the Android API? Or am I misremembering?

[benm (inactive), 2014-04-28 11:55:04]

On 2014/04/28 11:07:41, tomhudson wrote:
> > There are two points in Android's java layer where the old Picture's
> make-an-in-progress-copy functionality is/was used: in the Picture copy ctor
and
> the Picture::writeToStream entry point.
> 
> 
> +Ben: I thought WebView already had a workaround for this? Would it be
> generalizable to the Android API? Or am I misremembering?

Sorry, not sure I follow the question .. workaround for what exactly? Derek may
be able to answer better.

[robertphillips, 2014-04-28 17:29:39]

This CL is being abandoned in favor of (add replay entry point to
SkPictureRecorder for Android - https://codereview.chromium.org/252873005/)