1107297 - D3D11 should only composite invalid rect

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Bas Schouten (:bas.schouten)]

Attachment: Only recomposite and present the damaged area of the compositor

We're currently always recomposing and representing the whole window when using the D3D11 compositor. This is wasteful. Let's not. This essentially is the same behavior as the Basic compositor.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8531752 [details] [diff] [review]
Only recomposite and present the damaged area of the compositor

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

::: gfx/layers/d3d11/CompositorD3D11.cpp
@@ +889,5 @@
> +    RefPtr<IDXGISwapChain1> chain;
> +    HRESULT hr = mSwapChain->QueryInterface((IDXGISwapChain1**)byRef(chain));
> +    if (SUCCEEDED(hr) && chain) {
> +      DXGI_PRESENT_PARAMETERS params;
> +      memset(&params, 0, sizeof(DXGI_PRESENT_PARAMETERS));

Use mfbt/PodZero

@@ +891,5 @@
> +    if (SUCCEEDED(hr) && chain) {
> +      DXGI_PRESENT_PARAMETERS params;
> +      memset(&params, 0, sizeof(DXGI_PRESENT_PARAMETERS));
> +      params.DirtyRectsCount = mInvalidRegion.GetNumRects();
> +      std::vector<RECT> rects;

rects.reserve(params.DirtyRectsCount)

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8531752 [details] [diff] [review]
Only recomposite and present the damaged area of the compositor

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

::: gfx/layers/d3d11/CompositorD3D11.cpp
@@ +621,5 @@
> +  if (mCurrentRT == mDefaultRT) {
> +    scissor.left = std::max(int32_t(aClipRect.x), mCurrentClip.x);
> +    scissor.right = std::min(int32_t(aClipRect.XMost()), mCurrentClip.XMost());
> +    scissor.top = std::max(int32_t(aClipRect.y), mCurrentClip.y);
> +    scissor.bottom = std::min(int32_t(aClipRect.YMost()), mCurrentClip.YMost());

I'd like this better if you intersected the rects and set from that. i.e. 

clipRect = aClipRect;
if (mCurrentRT == mDefaultRT) {
   clipRect = intersect(aClipRect, mCurrentClip);
}

scissor.left = clipRect.x
....

@@ +862,5 @@
>    if (aRenderBoundsOut) {
>      *aRenderBoundsOut = Rect(0, 0, mSize.width, mSize.height);
>    }
>  
>    D3D11_RECT scissor;

scissor is now unused?

Comment 3

[Carsten Book [:Tomcat]]

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

Comment 4

[Jacek Caban]

I landed a trivial crosscompilation fixup:

https://hg.mozilla.org/integration/mozilla-inbound/rev/16ea12664917

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/16ea12664917

Comment 6

[Alice0775 White]

This should be backed out.

Comment 7

[Alice0775 White]

@:bas seems busy.

@:jrmuizel], This should be backed out.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Yes, this should get backed out. I think we should also probably back out bug 1102499 as it depends on this for performance.

Comment 9

[Bas Schouten (:bas.schouten)]

We set a Friday deadline for fixing the regressions, otherwise we'll back this out on aurora.