1647877 - Remote GDI presentation path always updates the entire window, should use dirty region

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Markus Stange [:mstange]]

CompositorWidget::StartRemoteDrawingInRegion has a region parameter that lets it optimize the window update region. But the remote GDI drawing path does not make use of that region and always updates the entire window, which is rather wasteful.

Comment 1

[Sotaro Ikeda [:sotaro]]

:cmartin, can you comment to the bug?

Comment 2

[Chris Martin [:cmartin]]

I agree it seems like this could be done more efficiently. Bug 1604412 was already getting quite complex, and so I took a "make it work, then make it fast" approach. It seems like it's time to do some work towards the second part of that :)

I can take a crack at implementing this when I get some free cycles, but in case anyone else is inclined to take it on - Probably the solution will involve "remembering" the update region from StartRemoteDrawingInRegion() and then sending it to the remote_backbuffer::Provider via the shared memory region during the EndRemoteDrawing() call.

I'm more inclined to think of this as an "enhancement" rather than a regression, as this is IMO new and unprecedented behaviour. We knew we were going to have to accept some amount of GDI perf hit for this GPU sandbox, and so this seems like an improvement on that initial feature.

Comment 3

[Markus Stange [:mstange]]

I agree with the "make it work, then make it fast" approach. But please do treat this with a high priority. We've seen cases where the memory bandwidth overhead from updating the whole window can affect browser performance in unexpected ways. See bug 1406414 for example: The throbber animation during page load can starve resources from the rest of the browser so that page load takes a very long time.

Comment 4

[Chris Martin [:cmartin]]

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

I agree with the "make it work, then make it fast" approach. But please do treat this with a high priority. We've seen cases where the memory bandwidth overhead from updating the whole window can affect browser performance in unexpected ways. See bug 1406414 for example: The throbber animation during page load can starve resources from the rest of the browser so that page load takes a very long time.

Fair enough - I will put it near the top of my todo list.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Seems unlikely the fix would be simple enough we'd take it in 78.

Comment 6

[Chris Martin [:cmartin]]

Attachment: Bug 1647877 - Remote Backbuffer: Only BitBlt dirty region

For initial implementation of remote backbuffer, I took a simple approach of
just BitBlt-ing the entire backbuffer contents to the window.

This now only updates the bounding rectangle of the dirty region.

Comment 7

[Chris Martin [:cmartin]]

Changing bug to "leave-open" because Markus pointed out that there is already machinery in place to reduce a region to a maximum number of rectangles, which is much simpler to pass over IPC. I will follow this first changeset up with another that allows up to 8 update rectangles, as Markus suggested that seems like a good "sweet spot"

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d861a4e70043
Remote Backbuffer: Only BitBlt dirty region r=mstange

Comment 9

[Chris Martin [:cmartin]]

Removed leave-open tag - Will file a new bug for the follow-up instead.

Comment 10

[Atila Butkovits]

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