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

RESOLVED FIXED in mozilla16

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bas, Assigned: bas)

Tracking

unspecified
mozilla16
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Snappy:P1])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
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.

Updated

5 years ago
Whiteboard: [Snappy:P1]
(Assignee)

Comment 1

5 years ago
Created attachment 632473 [details] [diff] [review]
Part 1: Cache ID2D1Layers for D2D Azure backend
Attachment #632473 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

5 years ago
The attached patch seems to reduce texture creation for clipping purposes by roughly 80% in a very naive test.
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.
Attachment #632473 - Flags: review?(jmuizelaar) → review-
(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.
(Assignee)

Comment 5

5 years ago
Created attachment 632896 [details] [diff] [review]
Part 1: Cache ID2D1Layers for D2D Azure backend v2
Attachment #632473 - Attachment is obsolete: true
Attachment #632896 - Flags: review?(jmuizelaar)
(Assignee)

Comment 6

5 years ago
Created attachment 632979 [details] [diff] [review]
Part 1: Cache ID2D1Layers for D2D Azure backend v3

Better function and this also uses the cache for masking, providing further performance improvements.
Attachment #632896 - Attachment is obsolete: true
Attachment #632896 - Flags: review?(jmuizelaar)
Attachment #632979 - Flags: review?(jmuizelaar)
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.
Attachment #632979 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 8

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

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/f328118d86b9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.