Closed Bug 516665 Opened 15 years ago Closed 15 years ago

distorted images with moz-icon://*?size=dialog

Categories

(Core :: Graphics: ImageLib, defect)

x86
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta1+

People

(Reporter: bugzilla.i.sekler, Assigned: ginnchen+exoracle)

References

()

Details

(Keywords: regression, Whiteboard: [decodeondraw])

Attachments

(4 files)

User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090915 Firefox/3.7a1pre Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090915 Firefox/3.7a1pre The display of images like moz-icon://stock/gtk-dialog-warning?size=dialog shows pixel noise in the lower part of the image (screenshots will follow). The same image over file:// displays fine. This happend after <http://hg.mozilla.org/mozilla-central/rev/ff3496b1f6c7> and at or before <http://hg.mozilla.org/mozilla-central/rev/3655f5de6392> Maybe something in connection with the new decode-on-draw? Reproducible: Always Steps to Reproduce: 1. Load moz-icon://stock/gtk-dialog-warning?size=dialog Actual Results: The lower part of the image is broken (contains a blank stripe and pixel noise below). The images that show the bug belong to the "Human" GNOME theme of Ubuntu 9.04. Can't tell whether they are required to be able to reproduce the bug yet.
Keywords: regression
Version: unspecified → Trunk
what's the equivalent of the gtk-dialog-warning string for other platforms? Joe - does the icon url mean that this somehow gets decoded with the nsIconDecoder? If so, then it makes sense that this would be broken, since I rewrote the icon decoder. Otherwise, I'm not sure why loading it with a different URL makes a difference. Normally it might have to do with our use of the false/false configuration for chrome images, but _everything_ is false/false right now.
Whiteboard: [decodeondraw]
yeah so at first glance it looks like this goes through the nsIconDecoder, meaning it's almost definitely a decode-on-draw regression. On the list.
Severity: normal → major
blocking2.0: --- → ?
Since I just tested, this regressed between Linux x86-64 nightlies 2009-09-13-03-mozilla-central and 2009-09-14-03-mozilla-central, which is the range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bf0fdec8f43b&tochange=7f0988f042e9
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reverting the changes to modules/libpr0n/src/imgRequest.cpp from the Bug 516311 http://hg.mozilla.org/mozilla-central/rev/f34cc41267d8 fixes the issue. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090920 Firefox/3.7a1pre SourceStamp=9061f43b86bd+
(In reply to comment #7) > Reverting the changes to modules/libpr0n/src/imgRequest.cpp from the Bug 516311 > http://hg.mozilla.org/mozilla-central/rev/f34cc41267d8 fixes the issue. > > Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090920 > Firefox/3.7a1pre > > SourceStamp=9061f43b86bd+ I have exactly the same results. It seems to me that code checked in to fix a performance only regression on one platform, that causes a real non-performance issue on another platform needs to be backed out immediately.
Blocks: 516311
(In reply to comment #8) > (In reply to comment #7) > I have exactly the same results. It seems to me that code checked in to fix a > performance only regression on one platform, that causes a real non-performance > issue on another platform needs to be backed out immediately. I don't see why this is necessarily the case. Regardless, that isn't actually the issue here. This bug was most certainly caused by my rewrite of the icon decoder in bug 435296. The checkin mentioned here was just toggling a global variable that switched a code patch to a more known one, but apparently one that didn't quite work properly with the icon decoder rewrite. Joe's going to look into this, it will almost certainly be a simple fix once we figure out what's going on, and we of course wouldn't ship a production browser with this bug unresolved.
(In reply to comment #9) > Regardless, that isn't actually the issue here. This bug was most certainly > caused by my rewrite of the icon decoder in bug 435296. The checkin mentioned > here was just toggling a global variable that switched a code patch to a more > known one, but apparently one that didn't quite work properly with the icon > decoder rewrite. Joe's going to look into this, it will almost certainly be a > simple fix once we figure out what's going on, and we of course wouldn't ship a > production browser with this bug unresolved. Yes, Sorry, I did not realize that the patch in questions was merely intended to turn off the decode on draw optimization, So, they images in question draw correctly either without the decode-on-draw code int he build, or with it in the build and enabled, but not in the case where it is in the build but disabled. This is obviously something that needs to be corrected. I should have actually looked at the patch rather than just doing an hg bisect to identify it.
From all the dupe reports we've been getting, it looks like this is probably linux specific.
Blocks: 435296
Attached patch patch (deleted) — Splinter Review
I think the typo here is the problem.
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
Attachment #406996 - Flags: review?(bobbyholley)
Comment on attachment 406996 [details] [diff] [review] patch r=bholley. Does this fix the issue, if so, this can land right away. Ginn - do you have commit access? If not joe or I can push this today (probably joe since I'll be out and about until 7pm).
Attachment #406996 - Flags: review?(bobbyholley) → review+
FWIW -- this patch doesn't apply cleanly to mozilla-central -- it looks like it was generated from a checkout that lacked Bug 512269's changes. But luckily, this patch is just a 3-character change, so it's easy to update to tip. :) > r=bholley. Does this fix the issue I've just tested (an un-bitrotted version of) Ginn's patch on a Linux mozilla-central debug build, and I can confirm that it fixes this bug for moz-icon://stock/gtk-dialog-warning?size=dialog > Ginn - do you have commit access? Yeah, he does. (see bug 514632 comment 6)
Same patch. This one applies cleanly to current mozilla-central tip.;
Thanks, I forgot to switch to HEAD after tracing the regression. It was a 2-character change actually. :) Pushed: http://hg.mozilla.org/mozilla-central/rev/09905e5b1f98
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
blocking2.0: ? → beta1
Depends on: 520504
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: