310630 - scaled-down PNG favicon (site icon) is very blurry

Status: RESOLVED INCOMPLETE

(Camino Graveyard :: General, defect)

Description

[[e-mail]]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050930 Camino/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050930 Camino/1.0+

When a site uses a PNG image larger than the standard 16x16 size, Camino scales
it down for displaying in location bar, tab title, and bookmarks. The scaled
version looks really fuzzy compared to FireFox's display.

Reproducible: Always

Steps to Reproduce:
1. Go to the example URL and wait for the page to load.
2. Note the favicon displayed in the location bar.


Actual Results:  
The scaled down display of the favicon is fuzzy and aliased.

Expected Results:  
Favicon should be scaled smoothly and anti-aliased. Refer to Firefox's display
of the same favicon.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Looks exactly the same as in Safari, so this is an OS issue.  We use the OS
library to draw favicons, so there's nothing we can do here.

Comment 2

[[e-mail]]

Attachment: PNG favicon rendered in Firefox

Yes, it looks the same as in Safari. But Firefox renders it much better. See
attachment.

Comment 3

[[e-mail]]

Attachment: PNG favicon rendered in Camino

Comment 4

[Samuel Sidler (old account; do not CC)]

I'm guessing we use system calls to display it while FF uses straight XUL (or
something in that realm at least). As such, we're not doing anything "wrong".
This is an OS X bug, I would guess.

CCing Simon... any thoughts on if we can do this better?

Comment 5

[Wevah (Nate Weaver)]

I'm pretty sure this can be fixed by adding the line

[[NSGraphicsContext currentContext] setImageInterpolation:NSImageInterpolationHigh];

before we composite the image to our custom cell.

Comment 6

[Ulrik Sverdrup]

Attachment: Patch using Wevah's snippet

Patch to run just the code Wevah posted in the appropriate place. Rendering is
not much better for the example favicon, but perhaps is it too blurry for
others?

Comment 7

[Ulrik Sverdrup]

Attachment: Rendering with the patch

Comment 8

[Simon Fraser [no longer active]]

Don't we need this on the tab icon too? I also forget what NSImage does for
caching imageReps, and whether this might affect other places where we display
the same NSImage.

Comment 9

[Ulrik Sverdrup]

(In reply to comment #6)

> Patch to run just the code Wevah posted in the appropriate place. Rendering is
> not much better for the example favicon, but perhaps is it too blurry for
> others?
not -> now


(In reply to comment #8)
> Don't we need this on the tab icon too? I also forget what NSImage does for
> caching imageReps, and whether this might affect other places where we display
> the same NSImage.
The same view class is used to paint the icon  in both the location bar and in
the tab view items.
Not affected are the drawing in menus and drag icons.
It seems there are scaling regressions in the history view for some icons. Could
that be a caching issue.
So investigation of side effects is needed.

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Current location bar and tab favicon rendering

The current tab favicon rendering is *much* worse than the current location bar
rendering (extra blurry?), yet (I think) both are the same size.  I notice
similar, though slightly less noticeable, issues with normal favicons.

Comment 11

[Wevah (Nate Weaver)]

(In reply to comment #10)
> yet (I think) both are the same size.

They aren't; the icon on the tabs is 1 or 2 px smaller in each dimension.

It looks much better with the high interpolation, though.

Comment 12

[Samuel Sidler (old account; do not CC)]

Simon, have you had a chance to look at this?

Comment 13

[Simon Fraser [no longer active]]

The tabs don't use PageProxyIcon; they use TruncatingImageAndTextCell. I think what the patch is doing is causing a smoothed version of the image to be cached, which then draws smoothed in other places (including the bookmarks bar). Although this gives the intended outcome, I'm not sure it's the most precise way to do it.

Comment 14

[Samuel Sidler (old account; do not CC)]

Ping.

If there's not a better way to do this, would the current patch be better than our current situation? If so, we should checkin this fix and leave the bug open for a better solution for 1.1. Either way, is a fix plausible for 1.0?

Comment 15

[Simon Fraser [no longer active]]

Patch is not correct (see my last comment).

Comment 16

[[e-mail]]

Changed sample URL. Need to place pre-scaled version to my site. Sorry.

Comment 17

[Michael Stillwell]

*** Bug 323998 has been marked as a duplicate of this bug. ***

Comment 18

[Michael Stillwell]

Attachment: screenshot showing BBC news favicon in location bar (16x16, readable) and tab bar (15x15, unreadable)

Is it not possible to simply increase the size of the favicons in the tab bar by 1 pixel?  A lot of favicons include a few characters of text, and they're often unreadable at 15x15.  (See BBC screenshot for an example.)

Comment 19

[Simon Fraser [no longer active]]

(In reply to comment #18)
> Is it not possible to simply increase the size of the favicons in the tab bar
> by 1 pixel?  A lot of favicons include a few characters of text, and they're
> often unreadable at 15x15.  (See BBC screenshot for an example.)

I think we'd have to increase the height of the tab bar by a pixel to keep the icon looking vertically centered.

Comment 20

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I filed bug 326872 on the issue of making tab favicons the same size as location bar favicons (comment 10, comment 11, comment 18, comment 19), so this bug can focus on better scaling down large (>16px) favicons.

Also fixing the spelling of "favicon" in the summary so this bug shows up in searches ;)

Comment 21

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 198538 [details] [diff] [review]
Patch using Wevah's snippet

r-'ing this based on comment 15; no reason to keep this in the queue.

Comment 22

[Christopher Henderson]

The referenced website (ju-ju.com) now has a 16x16 favicon, so is no longer usable for testing. Does anyone have a new example?

Comment 23

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Approximation of the old ju-ju.com site icon for testcase

I can't remember what the original size of the PNG was when Stephen filed this bug, but this is the 64x64px version returned by his Gravatar on the site.

Comment 24

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: testcase using the 64x64px PNG

Testcase

Comment 25

[[e-mail]]

Sorry guys. I reorganized the size some time ago and forgot to keep the test case file. I updated the location to another version of the test file.

Comment 26

[Chris Lawson (gone)]

Removing [good first bug] designator since we don't know a) what the correct fix is or b) what exactly the problems with the solutions proposed so far were.