Closed Bug 1575159 Opened 5 months ago Closed 3 months ago

Partial Invalidation on Windows

Categories

(Core :: Graphics: WebRender, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

(Blocks 3 open bugs)

Details

(Whiteboard: [wr-q41])

Attachments

(2 files, 7 obsolete files)

Once we have the infrastructure in WR for partial rects. We could implement partial invalidation on Windows. Partial invalidation is also necessary for android.

Depends on: 1536360
Priority: -- → P3
Blocks: 1546823
Blocks: 1575765
OS: Unspecified → Windows
  • It seems reasonable to use infrastructure of WR Picture caching to calculate invalidated rects for Partial Invalidation. But current picture caching does not cover whole window rendering. Bug 1575767 might be related.
  • WR partial rendering should update only within invalidated rects.
  • Invalidated rects might needs to be simple if we want to get performance. Chromium uses only one rect for partial invalidation.
  • When window is resized, WR needs to render whole window.

:kamidphish, do you have an interest to this bug? It is also necessary for get performance with DirectComposition.

Flags: needinfo?(dglastonbury)
OS: Windows → Unspecified
Flags: needinfo?(dglastonbury)
Depends on: 1581650

The patch uses WR dirty rects(Bug 1581650 ) for present swap chain. But it does not expect to reduce GPU task with the current patch.

Depends on: 1582624
No longer blocks: 1546823
Whiteboard: [wr-q41]

Attachment 9105152 [details] [diff] has2 problems.
-[1] CopySubresourceRegion() seemed not to work as expected

  • But error did not reported with pref gfx.direct3d11.enable-debug-layer : true

-[2] Black square appeared when content showed a scrollable web page and mouse was around browser UI and content.

  • The black square did not appear during showing about:support or about:config.

The patch enables partial present in WR. But it does full Present() at RenderCompositorANGLE::EndFrame(). It should work, since RenderCompositorANGLE allocates one buffer SwapChain with DXGI_SWAP_EFFECT_SEQUENTIAL. The SwapChain works as Bitblt model swap chain.
https://docs.microsoft.com/en-us/windows/win32/direct3ddxgi/dxgi-1-2-presentation-improvements#bitblt-model-swap-chain-with-dirty-rectangles

But with the patch, [2] in Comment 9 still happened. With it, WR's partial rendering might have a problem. The problem could happen more frequently with the followings.

  • "Bookmarks Toolbar" is enabled and has links.
  • Browser shows a web site.
  • Move mouse around the links and the web site.

:gw, can you comment to Comment 10? Do you have an idea about how the black square could happen?

Flags: needinfo?(gwatson)

I don't quite understand what you're doing here. Why is the CopySubresourceRegion call needed? According to the partial present documentation, Present1 should be doing all the necessary copying automatically. You just need to make sure to give it the right dirty rects and draw to those same dirty rects. (Maybe WebRender is drawing to the wrong dirty rects? Maybe the coordinates are upside down?)

Flags: needinfo?(sotaro.ikeda.g)

You just need to make sure to give it the right dirty rects and draw to those same dirty rects.

Yea. You are right. I am going to check the coordinates. CopySubresourceRegion is necessary when WR uses partial present, but RenderCompositorANGLE does not do partial present.

Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(gwatson)

(In reply to Sotaro Ikeda [:sotaro] from comment #13)

CopySubresourceRegion is necessary when WR uses partial present,

Why?

Attached patch wip patch (obsolete) — Splinter Review

Removed CopySubresourceRegion usage.

Attachment #9105152 - Attachment is obsolete: true

(In reply to Markus Stange [:mstange] from comment #14)

(In reply to Sotaro Ikeda [:sotaro] from comment #13)

CopySubresourceRegion is necessary when WR uses partial present,

Why?

For example, in CompositorD3D11, partial present is disabled for NVIDIA GPU, but compositor renders only part of back buffer.

Attachment #9105242 - Attachment is obsolete: true

(In reply to Sotaro Ikeda [:sotaro] from comment #16)

(In reply to Markus Stange [:mstange] from comment #14)

(In reply to Sotaro Ikeda [:sotaro] from comment #13)

CopySubresourceRegion is necessary when WR uses partial present,

Why?

For example, in CompositorD3D11, partial present is disabled for NVIDIA GPU, but compositor renders only part of back buffer.

In the case that partial present is disabled, could you initialize WR with max_partial_presents = 0, and then behavior is the same? Or is it likely faster to emulate partial present with the CopySubResourceRegion ?

(In reply to Glenn Watson [:gw] from comment #17)

In the case that partial present is disabled, could you initialize WR with max_partial_presents = 0, and then behavior is the same? Or is it likely faster to emulate partial present with the CopySubResourceRegion ?

It seems likely faster than simple full present.

I might find the cause of rendering problem. There might be 2 problems.

:gw, can you comment to Comment 19 ?

Flags: needinfo?(gwatson)

(In reply to Sotaro Ikeda [:sotaro] from comment #19)

I might find the cause of rendering problem. There might be 2 problems.

Nice work!

Thanks for investigating, I'll take a look at this today!

Thanks!

Attached patch 0001-WIP.patchSplinter Review

There is a patch up to fix the partial present semantics in WR (https://bugzilla.mozilla.org/show_bug.cgi?id=1593154).

This WIP patch builds on top of this - it skips Present1() call completely if WR returns no dirty rects at all.

From what I can see, this fixes the issues with partial present. I didn't notice any glitches during browsing. I also saw from logging that we are often presenting very small rects, or none at all (completely skipping Present1). It also fixes the error code returns we were getting from Present1() when WR was supplying an empty dirty rect.

Flags: needinfo?(gwatson)
Depends on: 1593154
No longer depends on: 1536360
Blocks: 1593179
No longer depends on: 1593154
Attachment #9105468 - Attachment is obsolete: true
Pushed by sikeda.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e151275792d
Implement partial invalidation on Windows r=gw
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.