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) (PTO until 27 June)
:
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) (PTO until 27 June)
no flags Details | Diff | Review
Add FastMovePixels to gfxASurface and use it where appropriate (6.46 KB, patch)
2011-05-03 19:00 PDT, Matt Woodrow (:mattwoodrow) (PTO until 27 June)
roc: review+
Details | Diff | Review
Add new scrolling reftest that scrolls backwards (2.25 KB, patch)
2011-05-03 20:39 PDT, Matt Woodrow (:mattwoodrow) (PTO until 27 June)
roc: review+
Details | Diff | Review

Description 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 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 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 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 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 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2011-05-03 17:14:29 PDT
*** Bug 651485 has been marked as a duplicate of this bug. ***
Comment 5 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2011-05-03 17:15:15 PDT
*** Bug 651484 has been marked as a duplicate of this bug. ***
Comment 6 Robert O'Callahan (:roc) (Exited; 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 Robert O'Callahan (:roc) (Exited; 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 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 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 Robert O'Callahan (:roc) (Exited; 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 Robert O'Callahan (:roc) (Exited; 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 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 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 Robert O'Callahan (:roc) (Exited; 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 Karl Tomlinson (ni?: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.