688368 - [Azure] Make SourceSurfaceSkia copy-on-write

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: Add Copy-on-write support

Instead of always copying pixel data from DrawTargets into SourceSurfaces, we should allow these to share memory until the DrawTarget is modified.

Comment 1

[Joe Drew (not getting mail)]

Comment on attachment 561659 [details] [diff] [review]
Add Copy-on-write support

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

::: gfx/2d/DrawTargetSkia.cpp
@@ +190,3 @@
>      return NULL;
>    }
> +  source->mDrawTarget = this;

I don't think this is still necessary, is it?

@@ +714,5 @@
> +void
> +DrawTargetSkia::RemoveSnapshot(SourceSurfaceSkia* aSnapshot)
> +{
> +  std::vector<SourceSurfaceSkia*>::iterator iter = std::find(mSnapshots.begin(), mSnapshots.end(), aSnapshot);
> +  mSnapshots.erase(iter);

Should this be if (iter != mSnapshots.end())? I thought that was causing a crash before.

::: gfx/2d/SourceSurfaceSkia.h
@@ +67,5 @@
>                      SurfaceFormat aFormat);
>  
> +  bool InitWithBitmap(const SkBitmap& aBitmap,
> +                      SurfaceFormat aFormat,
> +                      DrawTargetSkia* aOwner);

You should probably add documentation saying that if aOwner is NULL here, we'll copy the data, and otherwise we'll just copy-on-write reference it.

@@ +84,5 @@
>    SkBitmap mBitmap;
>    SurfaceFormat mFormat;
>    IntSize mSize;
>    int32_t mStride;
> +  RefPtr<DrawTargetSkia> mDrawTarget;

I made this a raw pointer and made sure that I called MarkIndependent() in the destructor of both SourceSurfaceCairo and DrawTargetCairo; I was a little worried about things being kept alive longer than they should be. I haven't thought long about what's right, though.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Make SourceSurfaceSkia data surfaces copy-on-write

Comment 3

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a204fcf29e60

Comment 4

[Marco Bonardo [:mak]]

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