Closed Bug 479852 Opened 15 years ago Closed 14 years ago

very slow scrolling with zoom on pages that have 1px tall or wide images tiled

Categories

(Core :: Graphics, defect, P1)

Product:
Core
Component:
Graphics
Version:
1.9.1 Branch
Platform:
x86
macOS
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: lilly, Assigned: roc)

References

(Depends on 1 open bug,
URL
)

Details

(Keywords: fixed1.9.1)

Attachments

(2 files, 1 obsolete file)

updated patch
3.26 KB, patch
roc
: review+
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
standalone testcase
2.15 KB, text/html
Details 
on certain pages (including this one: http://www.politico.com/politico44/) if i have it scaled up or down, performance on page scrolling is very very degraded. unscaled doesn't exhibit the slowdown.
Hmm.. will get a Shark profile in a bit, it might be that we're doing something stupid like copying data around all the time to scale it up/down.
I can confirm this on Linux (Ubuntu) as well. I'm surfing on a 12" laptop where I often zoom in on websites to make it a more comfortable read, and anything other than 100% normal/default zoom makes scrolling at least four times slower.

Text Zoom is not affected; only full page zoom has these perf issues. 

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20081211 Minefield/3.2a1pre
Looks like a dupe of bug 425234 btw.
http://www.politico.com/politico44/ works OK for me although it's not fast enough.  bug 425234 seems to exhibit the severity of the performance issue of the zoomed page scrolling more obviously - http://ajaxian.com/archives/tracetool-now-with-open-source-javascript-client
I searched for similar bug reports and here are my findings:

* Bug 479852
* Bug 480806
* Bug 425234
* Bug 426388

And here's another easy way to reproduce the problem:

1) Load your mailbox in Google Mail
2) Full-zoom to 200%
3) Enable the 'Tasks' labs feature
4) Login again to your GMail account with the 'Tasks' labs feature enabled.
5) Scroll now becomes seriously slow.
Let's not confuse this bug -- leave this as OSX only, and file a separate bug for any Linux/Win32 issues.

The problem here seems to be completely due to background images; 60% of the rendering time is spent under PaintBackgroundWithSC -> nsLayoutUtils::DrawImage.  

Inside nsThebesImage::Draw, we have:
  30% in cairo_paint()
  17% in cairo_fill()
  12% in gfxPlatformMac::CreateOffscreenSurface

so it looks like we're hitting one of the paths that require us to create a temporary surface.  The two main things that cause that are: padding/partial decode, or a non-integer translation.  I'm not sure which one we're hitting; need to check in the debugger to find out...  but, the Paint() is then the paint into the temporary image, and the Fill() is the actual real final fill.

There's a number of images on that page that are repeat-x or repeat-y with a 800x1 or 1x800 source image; if we somehow end up with a non-integer position, this could easily be the cause.
Why does non-integer position trigger a temporary surface? We shouldn't have a non-integer position anyway, though.
Oh, so this is the second case.  We have this code:

http://mxr.mozilla.org/mozilla-central/source/gfx/src/thebes/nsThebesImage.cpp#603

But userSpaceToImage space obviously doesn't have an integer scale, because the page is scaled.  And since we're tiling, we go into the horrible path down below.

We shouldn't need this, especially not on OSX, and I'd argue that we shouldn't use it anywhere despite the potential for some rendering artifacts.  Repeating 1xN or Nx1 images is pretty common, and this ensures that it will be very slow when zooming.  The artifact potential is that at the edges we might see bleed from the opposite part of the image where it's not expected.  But I think that's better than having things be as slow as they are.
I don't think we need to argue about tiling artifacts here. Vertical-only tiling of an Nx1 image can't produce any artifacts because the first row of the next tile is the same as the last row of the current tile... We should just optimize that case (and the 1xN case) to not need the temporary surface.
Yeah, good point.  I'll do that in the next few days.
Assignee: nobody → vladimir
Flags: wanted1.9.1+
Priority: -- → P1
Resolved Bug 484572 as a dupe of this bug since they're both OSX.  Of note from that conversation was that this issue appears to have been introduced somewhere between the 12/26/2008 and 1/1/2009 nightly builds.  This cnet link: http://news.cnet.com/8301-17938_105-10203342-1.html?tag=xlr8yourmac is also the worst page I've seen.
+1 vote
The same issue for Windows XP for URL( http://news.cnet.com/8301-17938_105-10203342-1.html?tag=xlr8yourmac ) of Comment#12.

Please see https://bugzilla.mozilla.org/show_bug.cgi?id=480806#c4
The above ( https://bugzilla.mozilla.org/show_bug.cgi?id=480806#c4 )shows regression range and changesets between the regression range.
Now try scrolling this site. Then try again with font size increased (Cmd-+):

http://www.mac-tv.de/index.lasso?Rubrik=Filme

Again, a breeze with Safari/Webkit, so I'm not sure one can blame this on OSX somehow.
(In reply to comment #14)
> http://www.mac-tv.de/index.lasso?Rubrik=Filme
. . . and a breeze for OmniWeb as well.
Attached patch potential patch (obsolete) — — Splinter Review 
Here's a potential patch here, after discussion with roc.. this fixes the politico case, by special-casing 1px wide/tall repeated images which are pretty common.  But there's an underlying problem that we'll end up creating a large temporary surface when I don't think we need to... that's not handled here, but putting this up for review before I look at that.

Note that this fixes the politico case, and doesn't affect others, which need to be different bugs with different analysis.
Attachment #370742 - Flags: review?(roc)
My survey of several sites vs several browsers implies that there are two issues shown by Firefox and Camino.

Slow scrolling of certain sites as found: Firefox = Yes; Camino = Yes.

Slow scrolling triggered by increase in font size (Cmd-+): Firefox = Yes; Camino = No.

On the acid test: http://www.mac-tv.de/index.lasso?Rubrik=Filme
Camino isn't affected at all by increasing font size! Is one bug in the Gecko rendering engine (same for Camino & FF) and the other in FF somewhere else?
Comment on attachment 370742 [details] [diff] [review]
potential patch

This patch is right, except for this:

-        if (!doTile && !mSinglePixel) {
+        if (!doTileX && !doTileY && !mSinglePixel) {

It's not correct to replace !doTile with !doTileX && !doTileY here. The reason is that the first thing this path does is replace the fill rect with the image-space-to-user-space transformed "available" rect. If the bottom row of pixels is available (for example) this means we're basically undoing the pixel-snapping of the bottom edge of the image. In the current code, we won't take this path if the bottom edge of the fill area was moved down by pixel snapping, since imageRect.Contains(sourceRect) won't be true in that case; with your code we would.

So we should just preserve behaviour here by testing imageRect.Contains(sourceRect) instead of !doTileX && !doTileY.
Attachment #370742 - Flags: review?(roc) → review+
Then you can move the declaration of doTileX and doTileY further down the function.
Attached patch updated patch — — Splinter Review 
Previous patch had a bug -- it wasn't actually setting EXTEND_REPEAT for the 1px case, so nothing was getting tiled.  This fixes that.
Attachment #370742 - Attachment is obsolete: true
Attachment #370951 - Flags: review?(roc)
Attached file standalone testcase — 
An isolated testcase for this -- just tiles a 998x1 image in the y direction in a 15,000px tall div.  This is basically what was happening in the politico case; the entire content had this image tiled around the background, and similar images tiled down the boxes of content.  Someone should really teach them about borders.
Attachment #370951 - Flags: review?(roc) → review+
Hrm, that patch broke all sorts of reftests -- the tests for bug 403181, bug 414123, bug 446100 all broke, among others.  Will look into it.
Hm, I take that back -- my tree was dirty, all reftests pass now.  Will land on trunk shortly.
http://hg.mozilla.org/mozilla-central/rev/836d5856f1e5
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 370951 [details] [diff] [review]
updated patch

Requesting a1.9.1; this optimizes rendering a pretty common pattern on the web.
Attachment #370951 - Flags: approval1.9.1?
Comment on attachment 370951 [details] [diff] [review]
updated patch

a191=beltzner, noting that this might fix bug 488186 as well
Comment on attachment 370951 [details] [diff] [review]
updated patch

(flag fixup, think it got midaired somewhere)
Attachment #370951 - Flags: approval1.9.1? → approval1.9.1+
As it looks like a tiling bug, could this have caused bug 488685?
maybe this is a regression, but on today's (and yesterday's) nightlies of Shiretoko, zoomed pages are scrolling terribly slowly.

see, http://ajaxian.com/ for an example page.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090421 Shiretoko/3.5b4pre ID:20090421030848
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
If this stays REOPENED, someone should also remove the fixed1.9.1. Nominating for blocking.
Flags: blocking1.9.1?
It's clearly regressed on 1.9.1, so removing that.  Can someone check trunk and see if this should be REOPENED or FIXED-sans-fixed1.9.1?
Keywords: fixed1.9.1
No, this did not regress; testing with the URL in the bug is fine.  A related/similar bug exists that also manifests itself as slow scrolling when the page is zoomed, which is the problem that's seen on ajaxian.  That's not a regression in the last few days.  This bug is bug 489389 for now (can't find the original bug for it).
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Flags: blocking1.9.1?
Keywords: fixed1.9.1
Resolution: --- → FIXED
Summary: very very slow scrolling when page zoomed in or out → very slow scrolling with zoom on pages that have 1px tall or wide images tiled
Using the latest nightly on Shiretoko and Trunk on the following pages doesn't show the slow scrolling that was reported in this bug. Also, I'm going to disregard the ajaxian.com comments as bug 489389 (per comment #34) has been created for the issue and no responses have been made after Vladimir's comments. 

http://www.politico.com/politico44/

and

the attached testcase

verified FIXED on builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b5pre) Gecko/20090504 Shiretoko/3.5b5pre ID:20090504031110

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090504 Minefield/3.6a1pre ID:20090504031236
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
We should back this out due to regressions in bug 488671 and probably bug 489642.
Depends on: 489642
Whiteboard: [needs landing]
Backed out due to regressions:
http://hg.mozilla.org/mozilla-central/rev/c9db288cd5cc

Still need to back out on branch.

We can refix this bug properly, although the new patch will be more complex. But now that bug 489389 has also landed, the problems here might not be so bad, though.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Whiteboard: [needs landing] → [needs 191 landing]
Assignee: vladimir → roc
Now that this has been backed out and 489389 landed, I'd like to know whether people think that special-casing the one-pixel-wide/high image case is still warranted. In other words, are the pages that this patch originally improved bad again, now that this patch has been reverted? Or are they still OK (presumably because bug 489389 is helping)?
Backed out on branch.
Keywords: verified1.9.1 → 4xp
Whiteboard: [needs 191 landing]
Keywords: 4xp
John tells me, and I can reproduce, that the page in question is much better than before (though still not perfect). I'll leave it to you, Rob, to close or leave open as you desire.
Marking fixed1.9.1 as per comment 40
Keywords: fixed1.9.1
So why is this page still hopelessly crippled (another cnet page)?

http://news.cnet.com/8301-13924_3-10252818-64.html

2-3 second delays when clicking in the scroll bar on a 2008 Intel MacPro. Smooth sailing in Safari/WebKit.
What version of Firefox are you using?  It's not *perfectly* smooth for me but even when zoomed up once using a 1.9.1 nightly (basically the "b99" preview) I don't get delays of even half a second on that page.  2008 MBP here, 2.33GHz, 3GB RAM.
Sorry. I just now downloaded and tried 3.6a1pre, and the problem is solved there, thanks. I was using 3.5b4. I hope this fix works its way into the release of 3.5!

That said, http://www.mac-tv.de/index.lasso?Rubrik=Filme still causes 3.6a1 to stumble. (It should open on a huge array on color+gray panels with text, spread three across.)
I'm seeing this problem with FireFox 3.5.

http://www.politico.com/politico44/ exhibits choppy scrolling when zoomed in or out. The greater the zoom, the more choppy the behavior.
I don't think I meant to reopen this. If there are still problems like this, please file a new bug.
Status: REOPENED → RESOLVED
Closed: 15 years ago → 14 years ago
Flags: wanted1.9.1+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: