As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 651469 - Scrolling up renders page unreadable (X11 with GL layers)
: Scrolling up renders page unreadable (X11 with GL layers)
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86_64 Linux
: -- normal with 2 votes (vote)
: ---
Assigned To: Matt Woodrow (:mattwoodrow)
:
: Milan Sreckovic [:milan]
Mentors:
http://zrusin.blogspot.com/
: 651484 651485 (view as bug list)
Depends on:
Blocks: 715668 640082
  Show dependency treegraph
 
Reported: 2011-04-20 05:31 PDT by Benoit Jacob [:bjacob] (mostly away)
Modified: 2012-01-05 14:22 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add MovePixels support to gfxXlibSurface (2.70 KB, patch)
2011-05-03 17:11 PDT, Matt Woodrow (:mattwoodrow)
no flags Details | Diff | Splinter Review
Add FastMovePixels to gfxASurface and use it where appropriate (6.46 KB, patch)
2011-05-03 19:00 PDT, Matt Woodrow (:mattwoodrow)
roc: review+
Details | Diff | Splinter Review
Add new scrolling reftest that scrolls backwards (2.25 KB, patch)
2011-05-03 20:39 PDT, Matt Woodrow (:mattwoodrow)
roc: review+
Details | Diff | Splinter Review

Description User image Benoit Jacob [:bjacob] (mostly away) 2011-04-20 05:31:59 PDT
Here on linux x86-64, proprietary NVIDIA driver, with layers.acceleration.force-enabled=true, scrolling is completely broken on this page:

http://zrusin.blogspot.com/
Comment 1 User image Benoit Jacob [:bjacob] (mostly away) 2011-04-20 05:33:45 PDT
To be more precise, the problem seems to be only/mostly with scrolling up, as opposed to scrolling down.
Comment 2 User image Benoit Jacob [:bjacob] (mostly away) 2011-04-20 05:38:33 PDT
The timing makes it plausible that this might be a regression from bug 640082, but I haven't verified this.
Comment 3 User image Matt Woodrow (:mattwoodrow) 2011-05-03 17:11:09 PDT
Created attachment 529895 [details] [diff] [review]
Add MovePixels support to gfxXlibSurface

Interestingly this happens when scrolling up the page (moving pixels down on the backing surface). With Image surfaces copying up is the problem, so I guess this means our internal data is Y-inverted.

If this ends up being a bottleneck we could try detect the flip (is it always the case for X surface?) and only allocate a temporary if we need one.

XCopyArea can handle self copies without manual intervention, but it is apparently rather slow. See http://lists.freedesktop.org/archives/cairo/2009-June/017290.html
Comment 4 User image Matt Woodrow (:mattwoodrow) 2011-05-03 17:14:29 PDT
*** Bug 651485 has been marked as a duplicate of this bug. ***
Comment 5 User image Matt Woodrow (:mattwoodrow) 2011-05-03 17:15:15 PDT
*** Bug 651484 has been marked as a duplicate of this bug. ***
Comment 6 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-03 17:55:18 PDT
I actually think that gfxASurface::MovePixels should use the safe, copying version by default, and a "fast" self-copy should be used in each backend where we know it works. I think gfxWindowsSurface and gfxQuartzSurface can use the fast implementation; gfxImageSurface already has its own implementation.
Comment 7 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-03 17:55:58 PDT
Oh, I guess D2D can use the fast version too.

A protected helper method in gfxASurface could implement the direct self-copy so each subclass only needs a few lines of code to use it.
Comment 8 User image Matt Woodrow (:mattwoodrow) 2011-05-03 19:00:57 PDT
Created attachment 529911 [details] [diff] [review]
Add FastMovePixels to gfxASurface and use it where appropriate

Sure, works for me.
Comment 9 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-03 19:07:42 PDT
Comment on attachment 529911 [details] [diff] [review]
Add FastMovePixels to gfxASurface and use it where appropriate

Review of attachment 529911 [details] [diff] [review]:
Comment 10 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-03 19:22:20 PDT
Do you have a testcase for this that could be turned into a reftest?
Comment 11 User image Matt Woodrow (:mattwoodrow) 2011-05-03 20:39:27 PDT
Created attachment 529922 [details] [diff] [review]
Add new scrolling reftest that scrolls backwards

Fails without the patch, passes with.
Comment 12 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-03 20:53:19 PDT
Comment on attachment 529922 [details] [diff] [review]
Add new scrolling reftest that scrolls backwards

Review of attachment 529922 [details] [diff] [review]:

lovely!
Comment 14 User image Karl Tomlinson (back Feb 1 :karlt) 2012-01-05 14:05:29 PST
(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> XCopyArea can handle self copies without manual intervention, but it is
> apparently rather slow. See
> http://lists.freedesktop.org/archives/cairo/2009-June/017290.html

Quoting that post:
> When an overlapped XCopyArea is issued - as is typical for a short
> scroll in any direction - XCopyArea breaks up the request into
> individual scanline fetches and puts.  This produces correct results,
> but because each scanline is done via two full Pixman call chains, it is
> hideously slow.  By that I mean that it can be about a third of the
> speed of a full copy-out followed by copy-back.

FWIW it looks like this is describing the fb implementation, the one used when the ddx graphics driver doesn't provide it's own implementation.

Note You need to log in before you can comment on or make changes to this bug.