Open
Bug 933389
Opened 11 years ago
Updated 2 years ago
For swipe snapshots, consider getting the snapshots from the window server using the private API CGSCaptureWindowsContentsToRectWithOptions
Categories
(Core :: Widget: Cocoa, defect, P2)
Tracking
()
NEW
People
(Reporter: mstange, Unassigned)
References
Details
(Whiteboard: tpi:+)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
Starting a swipe is janky at the moment because we repaint the whole window synchronously in order to get the snapshot.
I wondered why Safari is so much smoother and profiled it, and it turns out that Safari is using the private function CGSCaptureWindowsContentsToRect in order to get the snapshot directly from the window server.
Here's a patch that lets us do the same, and it reduces swipe start jank by about 50% for me. The rest of the jank can now be attributed to either making unnecessary copies of the pixel data or to scaling down and up for HiDPI displays.
Stephen, can you test how much of a difference this patch makes for non-HiDPI displays?
The right way to fix the capturing jank would be to create a new snapshotting graphics API (which I know is planned) and use that, but if we don't want to wait for it, we could use this approach instead, for now.
Comment 1•11 years ago
|
||
Nice! Thanks, Markus!
(In reply to Markus Stange [:mstange] from comment #0)
> Stephen, can you test how much of a difference this patch makes for
> non-HiDPI displays?
Did you have a specific method of testing in mind? Is your 50% improvement number based on profiling?
> The right way to fix the capturing jank would be to create a new
> snapshotting graphics API (which I know is planned) and use that, but if we
> don't want to wait for it, we could use this approach instead, for now.
I think this would be a great alternative until we have a snapshotting API.
Comment 2•11 years ago
|
||
Markus, could you do a tryserver build of your patch, so that people can test with it?
Comment 3•11 years ago
|
||
n-i for questions in comment 1. I can kick off a try build for you if you'd like, Markus.
Flags: needinfo?(mstange)
Comment 4•11 years ago
|
||
FWIW with that patch applied I can't use history swipes at all.
Reporter | ||
Comment 5•11 years ago
|
||
(In reply to Stephen Pohl [:spohl] from comment #1)
> Nice! Thanks, Markus!
>
> (In reply to Markus Stange [:mstange] from comment #0)
> > Stephen, can you test how much of a difference this patch makes for
> > non-HiDPI displays?
>
> Did you have a specific method of testing in mind?
Not really, but attaching a profiler, swiping back and forth a few times, and looking at the time spent in canvas.drawWindow should give us a good idea.
> Is your 50% improvement number based on profiling?
It's based on profiling and subjective experience. The 50% number is not accurate at all, it's just what it felt like to me during my brief testing. But we should try to get some more accurate numbers before seriously considering this ;)
A try build would be much appreciated!
Flags: needinfo?(mstange)
Reporter | ||
Comment 6•11 years ago
|
||
(In reply to Reuben Morais [:reuben] from comment #4)
> FWIW with that patch applied I can't use history swipes at all.
Which version of OS X are you on? I've only tested this on 10.8.5.
Comment 7•11 years ago
|
||
Try build will become available here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/spohl@mozilla.com-e723c5868874
Comment 8•11 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #6)
> (In reply to Reuben Morais [:reuben] from comment #4)
> > FWIW with that patch applied I can't use history swipes at all.
>
> Which version of OS X are you on? I've only tested this on 10.8.5.
10.9
Comment 9•11 years ago
|
||
(In reply to Reuben Morais [:reuben] from comment #4)
> FWIW with that patch applied I can't use history swipes at all.
Is it possible that you didn't resolve the rejects when applying the patch? I'll upload a patch shortly that's updated for current trunk (plus some known, broken behavior at the moment).
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/spohl@mozilla.com-
> e723c5868874
This build displays swipe animations for me on 10.9.
Comment 10•11 years ago
|
||
Updated for current trunk. I will need to revisit this patch to make sure that the changes from earlier bugs around page zoom get applied again. Zooming in on a page currently creates black borders when swiping. Indexing of snapshots also seems to be broken because the current page gets displayed twice (instead of the next page) when swiping forward. But this patch should unblock anyone wanting to apply the patch locally for performance testing.
Attachment #825446 -
Attachment is obsolete: true
Updated•11 years ago
|
Blocks: history-swipe
Comment 11•11 years ago
|
||
Comment on attachment 830541 [details] [diff] [review]
proof of concept
Review of attachment 830541 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/interfaces/canvas/nsIDOMCanvasRenderingContext2D.idl
@@ +31,5 @@
> // Don't synchronously decode images - draw what we have
> const unsigned long DRAWWINDOW_ASYNC_DECODE_IMAGES = 0x10;
> + // Avoid any rendering if possible and re-use recent existing snapshot of
> + // the window, for example directly from the window server.
> + const unsigned long DRAWWINDOW_USE_RECENT_WINDOW_SNAPSHOT = 0x20;
Nit: please remember to update the uuid of this interface, otherwise doing this may cause non-clobber builds to not properly update their xpt files.
Updated•7 years ago
|
Priority: -- → P2
Whiteboard: tpi:+
Updated•6 years ago
|
No longer blocks: history-swipe
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•