Fix some problems with scaled icons. This allows the TabIconView to automati...

Fix some problems with scaled icons.  This allows the TabIconView to automatically scale the throbber and favicon to whatever size the View itself is.  Notably, this does NOT change tab_renderer.cc, which _also_ draws favicons -- the former is used by app windows and the latter by standard tabs.  Argh!  I'm not going to bother unifying these for now even though it'd be nice, since we never scale up our tabs, just our app titlebars.

BUG=5054
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=8348

[sky, 2009-01-21 00:53:22]

http://codereview.chromium.org/18392/diff/1/2
File chrome/browser/views/tab_icon_view.cc (right):

http://codereview.chromium.org/18392/diff/1/2#newcode93
Line 93: PaintIcon(canvas, bitmap, 0, 0, bitmap.width(), bitmap.height(), true);
To match the old code you should only pass in true as filter if you're going to
scale. Perhaps you should just put this functionality into your PaintIcon
method.

http://codereview.chromium.org/18392/diff/1/5
File chrome/common/gfx/chrome_canvas.cc (right):

http://codereview.chromium.org/18392/diff/1/5#newcode195
Line 195: shader_scale.postTranslate(SkIntToScalar(dest_x),
SkIntToScalar(dest_y));
I've no idea on this code, I'm assuming you know what you're donig.

[Peter Kasting, 2009-01-21 01:00:36]

http://codereview.chromium.org/18392/diff/1/2
File chrome/browser/views/tab_icon_view.cc (right):

http://codereview.chromium.org/18392/diff/1/2#newcode93
Line 93: PaintIcon(canvas, bitmap, 0, 0, bitmap.width(), bitmap.height(), true);
On 2009/01/21 00:53:22, sky wrote:
> To match the old code you should only pass in true as filter if you're going
to
> scale.

If we aren't scaling, we won't be resampling, and thus the "filter" argument
will be ignored.

http://codereview.chromium.org/18392/diff/1/5
File chrome/common/gfx/chrome_canvas.cc (right):

http://codereview.chromium.org/18392/diff/1/5#newcode195
Line 195: shader_scale.postTranslate(SkIntToScalar(dest_x),
SkIntToScalar(dest_y));
On 2009/01/21 00:53:22, sky wrote:
> I've no idea on this code, I'm assuming you know what you're donig.

Pre-translate means "do this before the existing matrix takes effect",
post-translate means "do after".  If we have a source offset, dest offset, and
scaling factor, what we want is:
* Get desired subimage from source image at source offset
* Scale subimage to desired size
* Display scaled subimage at appropriate dest offset

The new code does this.  The old code scaled first:
* Scale source image up
* Get subimage from within source image (at unscaled offset!)
* Display scaled subimage at appropriate dest offset

As a result we would pick the wrong pixels out of the subimage.  I could have
manually scaled the |src_x| and |src_y| factors in the single postTranslate()
call, but I think that is less clear and may also be less precise.

[sky, 2009-01-21 01:03:59]

Thanks for the clarification.
LGTM