Open
Bug 430465
Opened 17 years ago
Updated 14 years ago
After scrolling down then up, some PNG tiled background images exhibit odd artefacts
Categories
(Core Graveyard :: GFX, defect)
Tracking
(Not tracked)
NEW
People
(Reporter: DBailey635, Unassigned)
References
()
Details
(Keywords: qawanted, regression)
Attachments
(5 files)
(deleted),
image/jpeg
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9b5) Gecko/2008032620 Firefox/3.0b5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9b5) Gecko/2008032620 Firefox/3.0b5
Odd artefacts appear in background PNG images after scrolling
Reproducible: Always
Steps to Reproduce:
1. Load long web page with a PNG background image file that is tiled
2. Scroll down then back up
3. Odd artefacts appear in image (see uploaded example)
Actual Results:
See uploaded sample file
Expected Results:
No artefacts should appear
Reporter | ||
Comment 1•17 years ago
|
||
Reporter | ||
Comment 2•17 years ago
|
||
Running on Windows XP SP2, fully patched. Pentium D processor 3GHz with 2GB RAM. NVIDIA Quadro FX560 graphics card.
Comment 3•17 years ago
|
||
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042223 Minefield/3.0pre
Regression window is
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1205098020&maxdate=1205103479
Bug 403181 seems the most likely cause.
Blocks: 403181
Status: UNCONFIRMED → NEW
Component: General → GFX
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
Assignee: nobody → roc
Flags: wanted1.9+
I have to zoom in to see this. David, do you see this without zooming?
Wait, sometimes I can reproduce it without zooming.
The key to reproducing this is to scroll *slowly*. It affects the grey bar on the right as well as the rightmost blue bar.
I'm pretty sure that when we scroll slowly, the dirty rect contains only one tile so we take the single-tile path through nsLayoutUtils::DrawImage. That appears to be scaling the image, which it probably shouldn't be since the image coordinates should have been snapped to device pixels in a way that leaves the image size intact.
The edges of the image are being antialiased in this case, but they really shouldn't.
This patch fixes the testcase.
The problem in the testcase, which I've seen elsewhere too, is that we're snapping the destination and dirty rects which makes their width slightly different to the source rect, so we end up scaling the image by some ratio very close to 1. For example if the source and destination widths start off as 99.5px then we might snap the destination width to 100px and scale the image up a tiny bit. This makes it look blurry and is slow too.
So my idea is to treat the source rect as defining two things:
1) which pixels we may sample
2) a transformation (translation+scale) between source coordinates and coordinates within aDestRect
1) becomes pxSubimage which we round out and pass down to gfx.
2) is used to reconstruct the source rectangle by applying the reverse transformation to aDirtyRect. In particular (unless source clipping is going on) scale factors are preserved through nsLayoutUtils::DrawImage; if the source width equals the destination width on entry, then the source width will equal the dirty rect width when we call into gfx.
HOWEVER, this does not fix this bug as reported.
The Bath site uses background-position:center. We make no attempt to snap *source* coordinates at all, so our source rect is not pixel aligned and we sample fuzzy edges. That's OK. The problem is that for consistency we should also make the *tile* path sample the same way.
This fixes the reported bug. I could never figure out what the rounding there was for, anyway.
Attachment #317460 -
Flags: review?(vladimir)
Whiteboard: [needs review]
Comment on attachment 317460 [details] [diff] [review]
fix
Looks fine; I can't quite remember what the rounding was for any more either.
Attachment #317460 -
Flags: review?(vladimir) → review+
Comment on attachment 317460 [details] [diff] [review]
fix
Fixes some visual artifacts scrolling certain pages. Fairly safe.
Attachment #317460 -
Flags: approval1.9?
Comment 15•17 years ago
|
||
Comment on attachment 317460 [details] [diff] [review]
fix
a1.9=beltzner
Attachment #317460 -
Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Blocks: 424672
Comment 16•17 years ago
|
||
I backed out the patch+bustage fix because of orange in tbox.
Not sure if the cause was the reftest or the "bustage fix" to reftest.list or
what.
Updated•17 years ago
|
Keywords: checkin-needed
Whiteboard: [needs review]
Sorry Olli, I was delinquent.
Bug 421069 failed on all three platforms, and the reftest in this bug failed on Mac. Which is strange since I manually checked the color values on Mac and they seemed right. I think we should just leave this unfixed for 1.9, it's a minor bug and not worth spending more energy/risk on.
Flags: wanted1.9-
Flags: wanted1.9+
Flags: wanted-next+
Comment 18•17 years ago
|
||
Comment on attachment 317460 [details] [diff] [review]
fix
Remove approval, since we don't want to land this.
Attachment #317460 -
Flags: approval1.9+
Comment 19•16 years ago
|
||
Is this ready to land again?
Needs retesting.
Assignee | ||
Updated•16 years ago
|
Product: Core → Core Graveyard
Assignee: roc → nobody
You need to log in
before you can comment on or make changes to this bug.
Description
•