1379029 - Implement GuaranteePersistance on SourceSurfaceD2D + SourceSurfaceSkia

Status: RESOLVED FIXED

(Core :: Graphics, enhancement)

Description

[Mason Chang [Inactive] [:mchang]]

DrawTargetCapture requires source surfaces to stay alive until playback time. It does this by calling SourceSurface::GuaranteePersistance when recording a moz2d command that requires a SourceSurface[1]. What was happening with some source surfaces was that we'd create a temporary draw target (mostly skia), and we'd free the SourceSurface before we could replay the captured contents. This implements GuaranteePersistance by making copies of the source surface and being sad as we just made recording slower :(.

[1] http://searchfox.org/mozilla-central/source/gfx/2d/DrawTargetCapture.cpp#88

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: Implement GuaranteePersistance for SourceSurfaceSKia + SourceSurfaceD2D

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8884117 [details] [diff] [review]
Implement GuaranteePersistance for SourceSurfaceSKia + SourceSurfaceD2D

Review of attachment 8884117 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/SourceSurfaceSkia.cpp
@@ +177,5 @@
>  
> +void
> +SourceSurfaceSkia::GuaranteePersistance()
> +{
> +  // Explicitly make a copy of the image since sometimes, SourceSurfaceSkia

Let's open a bug for this, maybe there is a way to transfer the ownership instead of always making a copy?

Comment 3

[Lee Salzman [:lsalzman]]

(In reply to Milan Sreckovic [:milan] from comment #2)
> Comment on attachment 8884117 [details] [diff] [review]
> Implement GuaranteePersistance for SourceSurfaceSKia + SourceSurfaceD2D
> 
> Review of attachment 8884117 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/2d/SourceSurfaceSkia.cpp
> @@ +177,5 @@
> >  
> > +void
> > +SourceSurfaceSkia::GuaranteePersistance()
> > +{
> > +  // Explicitly make a copy of the image since sometimes, SourceSurfaceSkia
> 
> Let's open a bug for this, maybe there is a way to transfer the ownership
> instead of always making a copy?

This shouldn't be necessary, as SourceSurfaceSkia, when used as a snapshot, contains a RefPtr to the owning DrawTargetSkia. And DrawTargetSkia::MarkChanged will take care of the snapshot being orphaned as well.

So long as you're holding a RefPtr to the SourceSurfaceSkia, it should basically always be safe, in principle. Thus, I don't think this bug is what it seems. It would be nice to know more about the surrounding circumstances so we could ascertain what's going on.

Comment 4

[Mason Chang [Inactive] [:mchang]]

Attachment: AddUserData to hold onto the calloc blur memory

Comment 5

[Mason Chang [Inactive] [:mchang]]

Attachment: AddUserData to hold onto the calloc blur memory

Comment 6

[Mason Chang [Inactive] [:mchang]]

Attachment: AddUserData to hold onto the calloc blur memory

Comment 7

[Mason Chang [Inactive] [:mchang]]

Attachment: AddUserData to hold onto the calloc blur memory

Comment 8

[Mason Chang [Inactive] [:mchang]]

Try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=e89d3637216f1099457ad477cd77aa26232a4ab9

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e897b12b2b63
AddUserData to hold onto the calloc blur memory. r=lsalzman

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e897b12b2b63