Closed Bug 532732 Opened 15 years ago Closed 15 years ago

Scrolling when zoomed out with fixed image backgrounds is slow on OS X

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jrmuizel, Assigned: roc)

References

Details

Attachments

(1 file, 1 obsolete file)

This is presumably because we rescale the background image on every paint. We shouldn't do this, and I'm not yet sure why we do.
Blocks: 527728
This is because we call createOffscreenSurface() every time we draw the image and so we don't hit the destination scaled image cache. createOffscreenSurface() is called because the sourceRect is much larger then we actually need and so we think that the image needs tiling. Even still we shouldn't call createOffscreenSurface() for tiled images.
What testcase are you looking at? nsThebesImage::Draw could cache the temporary surface it creates for tiled images. That's probably the best thing to do, short of actually being able to limit the area we sample from in tiled image space.
This page simulates the effect using 'background-size': http://people.mozilla.com/~jmuizelaar/scroll/fixed-downscale.html I don't actually understand why we need a temporary surface at all.
I'm surprised, because we've discussed it several times.
But to recap: suppose we have a 20x20 image whose left half is black and whose right half is white. Suppose we tile that image to cover a 100x100 area. Suppose there is a non-1.0 zoom, which triggers bilinear sampling. cairo with EXTEND_REPEAT will draw the rightmost column of pixels by sampling from the rightmost row of pixels of the image *and* the leftmost row of pixels of the image, producing a gray line. This is an artifact because if there's no zoom, the area around and containing that line is solid white.
(assuming a white background color for the page)
(In reply to comment #5) > But to recap: suppose we have a 20x20 image whose left half is black and whose > right half is white. Suppose we tile that image to cover a 100x100 area. > Suppose there is a non-1.0 zoom, which triggers bilinear sampling. cairo with > EXTEND_REPEAT will draw the rightmost column of pixels by sampling from the > rightmost row of pixels of the image *and* the leftmost row of pixels of the > image, producing a gray line. This is an artifact because if there's no zoom, > the area around and containing that line is solid white. I understand that scenario. However, I was expecting a page with a fixed background to be indistinguishable from one with a scrolling background provided the content size was less than window size. This currently isn't the case, and in thinking it over I'm not sure which makes more sense. I also didn't realize some of the implications of our current design. For example, http://people.mozilla.com/~jmuizelaar/scale/test.html is quite slow. No good way to make it fast, well keeping the same rendering, using CoreGraphics comes immediately to mind.
(In reply to comment #7) > I understand that scenario. However, I was expecting a page with a fixed > background to be indistinguishable from one with a scrolling background > provided the content size was less than window size. This currently isn't the > case, and in thinking it over I'm not sure which makes more sense. I see your point. What's happening here is simply that background-attachment:fixed doesn't change the "fill area" over which the background is drawn. In this case, that is the whole document, i.e., logically the background is tiled over the whole document. In practice that doesn't really matter since you can only ever see the one tile. In fact it does matter a tiny bit: for example it implies that the pixels at the bottom of the window can/should be sampled using the first row of the next tile as well as the last row of the first tile. However, in this case I think it would be quite justified to forget about that and limit the fill area for fixed backgrounds to the viewport, always. I'll write a patch for that.
That will mean that drawWindow calls or other operations that try to render content outside the viewport won't get the background. Too bad!
Attached patch fix (obsolete) — Splinter Review
This causes all background-attachment:fixed backgrounds to be clipped to the viewport. Probably the right thing to do even though it's technically incorrect.
Assignee: nobody → roc
Attachment #416253 - Flags: review?(dbaron)
Are you changing the clip area just to avoid hitting a codepath that will trigger the extra surface? Could we pass a boolean to do that instead, and not break drawWindow?
We could condition this change on FLAG_DESTINED_FOR_SCREEN, I guess. We can't just "disable the extra surface", since that would lead to some kind of incorrect results when using drawWindow, e.g. we'd trigger artifacts such as those described in comment #5 in some situations.
(In reply to comment #12) > We could condition this change on FLAG_DESTINED_FOR_SCREEN, I guess. If that's not too hard, it sounds like a good idea.
Attached patch fix v2Splinter Review
It's easy.
Attachment #416253 - Attachment is obsolete: true
Attachment #416525 - Flags: review?(dbaron)
Attachment #416253 - Flags: review?(dbaron)
Whiteboard: [needs landing]
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: