Fix images drawn with the NineImagePainter class in high DPI.

Fix images drawn with the NineImagePainter class in high DPI.

This class is used for drawing custom buttons/scrollbars, etc. On Windows high DPI for
fractional scale factors like 1.25/1.5, etc these images appear distorted due to pixel loss
occuring while drawing. The loss happens due to the following reasons.

1. The class uses two different flavors of the Canvas::DrawImageInt function to draw. One flavor
   is invoked via a local helper called Fill and the other one is used to draw some images.
   The latter causes painting problems for fractional scales as it rounds differently via the
   skia helper SkScalarRoundToInt which internally ends up adding 0.5 to the value being ceiled
   and ceils it at times.

2. The NineImagePainter class uses ImageSkiaStorage.width and height to get the DIP width and height
   respectively. These methods divide the pixel width/height by the scale factor and return the
   floored results. This causes pixel loss. E.g. pixel width/height 8 and scale favtor 1.5.
   These methods return 5 (5.33 rounded to 5). This causes pixel loss eventually.

3. The bounds of the rect being painted to which is passed into the paint function contain width/height
   values which when scaled cause rounding problems. 

Fixes for the above issues are as below:-
1. Added a new function DrawImageIntInPixel to the gfx::Canvas class. This function ensures that the 
   underlying SkCanvas does not scale. It translates the destination rectangle to ensure that the image is 
   displayed at the desired location.

2. Changed the drawing code in the NineImagePainter class to pass in pixel locations/width/height etc for drawing.

BUG=367543
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268429

[sky, 2014-05-02 14:36:32]

Since you're trying to align images, which are pixel based, I think this code
would be less error prone if you inverted the scale and operate at a pixel
level. So, I'm saying you get the images you at the appropriate scale, invert
the scale in the canvas, and draw yourself so that scale is 1x. This way you get
pixel level precision.

[ananta, 2014-05-03 03:49:36]

Updated the NineImagePainter class to pass in raw pixel values  for drawing as
per discussion. PTAL

[sky, 2014-05-05 14:08:16]

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc
File ui/gfx/canvas.cc (right):

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode396
ui/gfx/canvas.cc:396: // 1. Translate the destination rectangle with using the
current translation
nit: remove with.

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode401
ui/gfx/canvas.cc:401: // 4. Set the scale in gfx::Canvas instance to 1.0, 1.0.
Ideally this should
I don't get the 'ideally this sould have been enough. but is not the case'. What
does that mean?

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode410
ui/gfx/canvas.cc:410: destination_rect.set(dest_x, dest_y, dest_x + dest_w,
dest_y + dest_h);
These should all be SkIntToScalar I bleieve.

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode423
ui/gfx/canvas.cc:423: DrawImageIntHelper(image, src_x, src_y, src_w, src_h,
destination_rect.x(),
SkScalarToInt I believe.

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode429
ui/gfx/canvas.cc:429: Scale(image_scale, image_scale);
Doesn't the save save the matrix and scale so that you don't need to restore it?

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/nine_image_painte...
File ui/gfx/nine_image_painter.cc (left):

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/nine_image_painte...
ui/gfx/nine_image_painter.cc:46: { 0, insets.left(), image.width() -
insets.right(), image.width() };
nit: keep the space here. It's generally what we do for code like this (it
matches that in the openning brace).

[ananta, 2014-05-05 18:53:27]

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc
File ui/gfx/canvas.cc (right):

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode396
ui/gfx/canvas.cc:396: // 1. Translate the destination rectangle with using the
current translation
On 2014/05/05 14:08:16, sky wrote:
> nit: remove with.

Done.

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode401
ui/gfx/canvas.cc:401: // 4. Set the scale in gfx::Canvas instance to 1.0, 1.0.
Ideally this should
On 2014/05/05 14:08:16, sky wrote:
> I don't get the 'ideally this sould have been enough. but is not the case'.
What
> does that mean?

I assumed that setting the scale to 1.0 in the SkCanvas would be enough. However
that is not the case as mentioned in point 3 above. 

Removed the should have been enough part.

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode410
ui/gfx/canvas.cc:410: destination_rect.set(dest_x, dest_y, dest_x + dest_w,
dest_y + dest_h);
On 2014/05/05 14:08:16, sky wrote:
> These should all be SkIntToScalar I bleieve.

Done.

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode423
ui/gfx/canvas.cc:423: DrawImageIntHelper(image, src_x, src_y, src_w, src_h,
destination_rect.x(),
On 2014/05/05 14:08:16, sky wrote:
> SkScalarToInt I believe.

No function by that name. Replaced with SkScalarRoundToInt

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/canvas.cc#newcode429
ui/gfx/canvas.cc:429: Scale(image_scale, image_scale);
On 2014/05/05 14:08:16, sky wrote:
> Doesn't the save save the matrix and scale so that you don't need to restore
it?

Left the Restore call as is. Removed the rest.

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/nine_image_painte...
File ui/gfx/nine_image_painter.cc (left):

https://codereview.chromium.org/262773010/diff/30001/ui/gfx/nine_image_painte...
ui/gfx/nine_image_painter.cc:46: { 0, insets.left(), image.width() -
insets.right(), image.width() };
On 2014/05/05 14:08:16, sky wrote:
> nit: keep the space here. It's generally what we do for code like this (it
> matches that in the openning brace).

Done.

[sky, 2014-05-05 20:51:13]

https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc
File ui/gfx/canvas.cc (right):

https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc#newcode422
ui/gfx/canvas.cc:422: Scale(1.0f, 1.0f);
Is this needed? Haven't you effectively reset the scale on 420?

https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc#newcode440
ui/gfx/canvas.cc:440: image_scale_ = image_scale;
Is this needed?

[ananta, 2014-05-05 21:36:20]

https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc
File ui/gfx/canvas.cc (right):

https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc#newcode422
ui/gfx/canvas.cc:422: Scale(1.0f, 1.0f);
On 2014/05/05 20:51:14, sky wrote:
> Is this needed? Haven't you effectively reset the scale on 420?

Yeah. Not needed.

https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc#newcode440
ui/gfx/canvas.cc:440: image_scale_ = image_scale;
On 2014/05/05 20:51:14, sky wrote:
> Is this needed?

Have that for correctness in case the canvas is used later.

[sky, 2014-05-06 00:24:10]

https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc
File ui/gfx/canvas.cc (right):

https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc#newcode440
ui/gfx/canvas.cc:440: image_scale_ = image_scale;
On 2014/05/05 21:36:20, ananta wrote:
> On 2014/05/05 20:51:14, sky wrote:
> > Is this needed?
> 
> Have that for correctness in case the canvas is used later.

Where are you actually changing image_scale_ though?

[ananta, 2014-05-06 00:27:55]

On 2014/05/06 00:24:10, sky wrote:
> https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc
> File ui/gfx/canvas.cc (right):
> 
>
https://codereview.chromium.org/262773010/diff/70001/ui/gfx/canvas.cc#newcode440
> ui/gfx/canvas.cc:440: image_scale_ = image_scale;
> On 2014/05/05 21:36:20, ananta wrote:
> > On 2014/05/05 20:51:14, sky wrote:
> > > Is this needed?
> > 
> > Have that for correctness in case the canvas is used later.
> 
> Where are you actually changing image_scale_ though?

Doh. Sorry about that. No longer needed with the call to Scale removed. Should
have seen that.

[sky, 2014-05-06 00:31:27]

LGTM

[ananta, 2014-05-06 01:20:35]

The CQ bit was checked by ananta@chromium.org

[commit-bot: I haz the power, 2014-05-06 01:22:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ananta@chromium.org/262773010/130001

[commit-bot: I haz the power, 2014-05-06 05:11:01]

Change committed as 268429

[oshima, 2014-05-06 06:44:01]

drive by:

https://codereview.chromium.org/262773010/diff/130001/ui/gfx/canvas.cc
File ui/gfx/canvas.cc (right):

https://codereview.chromium.org/262773010/diff/130001/ui/gfx/canvas.cc#newcod...
ui/gfx/canvas.cc:413: matrix.mapRect(&destination_rect, destination_rect);
This doesn't seem to work if the canvas is rotated, say 45 degree. Am I wrong?