Scrolling up renders page unreadable (X11 with GL layers)

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: bjacob, Assigned: mattwoodrow)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

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/
(Reporter)

Comment 1

6 years ago
To be more precise, the problem seems to be only/mostly with scrolling up, as opposed to scrolling down.
Summary: Scrolling renders page unreadable (X11 with GL layers) → Scrolling up renders page unreadable (X11 with GL layers)
(Reporter)

Comment 2

6 years ago
The timing makes it plausible that this might be a regression from bug 640082, but I haven't verified this.
(Reporter)

Updated

6 years ago
Blocks: 640082
(Assignee)

Comment 3

6 years ago
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
Attachment #529895 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Duplicate of this bug: 651485
(Assignee)

Updated

6 years ago
Duplicate of this bug: 651484
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.
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.
(Assignee)

Comment 8

6 years ago
Created attachment 529911 [details] [diff] [review]
Add FastMovePixels to gfxASurface and use it where appropriate

Sure, works for me.
Attachment #529895 - Attachment is obsolete: true
Attachment #529895 - Flags: review?(roc)
Attachment #529911 - Flags: review?(roc)
Comment on attachment 529911 [details] [diff] [review]
Add FastMovePixels to gfxASurface and use it where appropriate

Review of attachment 529911 [details] [diff] [review]:
Attachment #529911 - Flags: review?(roc) → review+
Do you have a testcase for this that could be turned into a reftest?
(Assignee)

Comment 11

6 years ago
Created attachment 529922 [details] [diff] [review]
Add new scrolling reftest that scrolls backwards

Fails without the patch, passes with.
Attachment #529922 - Flags: review?(roc)
Comment on attachment 529922 [details] [diff] [review]
Add new scrolling reftest that scrolls backwards

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

lovely!
Attachment #529922 - Flags: review?(roc) → review+
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/mozilla-central/rev/a0fd33b098f6
http://hg.mozilla.org/mozilla-central/rev/e861aabf1631
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Assignee: nobody → matt.woodrow+bugzilla
(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.
Blocks: 715668
You need to log in before you can comment on or make changes to this bug.