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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jrmuizel, Assigned: roc)
References
Details
Attachments
(1 file, 1 obsolete file)
4.99 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•15 years ago
|
||
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.
Assignee | ||
Comment 2•15 years ago
|
||
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.
Reporter | ||
Comment 3•15 years ago
|
||
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.
Assignee | ||
Comment 4•15 years ago
|
||
I'm surprised, because we've discussed it several times.
Assignee | ||
Comment 5•15 years ago
|
||
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.
Assignee | ||
Comment 6•15 years ago
|
||
(assuming a white background color for the page)
Reporter | ||
Comment 7•15 years ago
|
||
(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.
Assignee | ||
Comment 8•15 years ago
|
||
(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.
Assignee | ||
Comment 9•15 years ago
|
||
That will mean that drawWindow calls or other operations that try to render content outside the viewport won't get the background. Too bad!
Assignee | ||
Comment 10•15 years ago
|
||
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?
Assignee | ||
Comment 12•15 years ago
|
||
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.
Assignee | ||
Comment 14•15 years ago
|
||
It's easy.
Attachment #416253 -
Attachment is obsolete: true
Attachment #416525 -
Flags: review?(dbaron)
Attachment #416253 -
Flags: review?(dbaron)
Comment on attachment 416525 [details] [diff] [review]
fix v2
r=dbaron
Attachment #416525 -
Flags: review?(dbaron) → review+
Assignee | ||
Updated•15 years ago
|
Whiteboard: [needs landing]
Assignee | ||
Comment 16•15 years ago
|
||
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.
Description
•