761397 - [Azure][D2D] Reduce churn on surface creation for PushGroup/PopGroup/Clipping

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

Surface creation is expensive for GPU drawing systems. I should do some work to reduce the amount of surface creations, for clipping that can be done simply by caching ID2D1Layer objects. For the Azure-Thebes wrapper it should probably be done inside the gfxContext wrapper.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Cache ID2D1Layers for D2D Azure backend

Comment 2

[Bas Schouten (:bas.schouten)]

The attached patch seems to reduce texture creation for clipping purposes by roughly 80% in a very naive test.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 632473 [details] [diff] [review]
Part 1: Cache ID2D1Layers for D2D Azure backend

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

::: gfx/2d/DrawTargetD2D.cpp
@@ +157,3 @@
>    , mPrivateData(NULL)
>  {
> +  mCachedLayers.resize(5);

Use a named constant instead of 5 and a comment about why you chose 5 even if that comment is "chosen arbitrarily"

@@ +968,5 @@
> +  } else {
> +    mRT->CreateLayer(byRef(clip.mLayer));
> +  }
> +  mCurrentCachedLayer++;
> +

Feels like this should be a separate function

::: gfx/2d/DrawTargetD2D.h
@@ +218,5 @@
>      D2D1_MATRIX_3X2_F mTransform;
>      RefPtr<PathD2D> mPath;
>    };
>    std::vector<PushedClip> mPushedClips;
> +  std::vector<RefPtr<ID2D1Layer>> mCachedLayers;

It looks like you're just using a std::vector here so that you raii destruction. If so, add a comment and we can switch it to something like what I'm using in this patch:
https://bug763119.bugzilla.mozilla.org/attachment.cgi?id=631822

That will save us the heap allocation.

@@ +219,5 @@
>      RefPtr<PathD2D> mPath;
>    };
>    std::vector<PushedClip> mPushedClips;
> +  std::vector<RefPtr<ID2D1Layer>> mCachedLayers;
> +  uint32_t mCurrentCachedLayer;

Add a comment about the rationale behind caching layers.

Comment 4

[Joe Drew (not getting mail)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> > +  std::vector<RefPtr<ID2D1Layer>> mCachedLayers;
                                    ^^
That's going to fail to build on non-C++11 compilers.

Comment 5

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Cache ID2D1Layers for D2D Azure backend v2

Comment 6

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Cache ID2D1Layers for D2D Azure backend v3

Better function and this also uses the cache for masking, providing further performance improvements.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 632979 [details] [diff] [review]
Part 1: Cache ID2D1Layers for D2D Azure backend v3

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

::: gfx/2d/DrawTargetD2D.cpp
@@ +939,5 @@
>    rt->PopLayer();
>  
>    FinalizeRTForOperation(aOptions.mCompositionOp, aSource, Rect(0, 0, (Float)mSize.width, (Float)mSize.height));
> +
> +  mCurrentCachedLayer--;

It feels like you should also move PopLayer(); mCurrentCacheLayer-- into a helper to make sure they always match.

Comment 8

[Bas Schouten (:bas.schouten)]

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

Comment 9

[Ed Morley [:emorley]]

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