[webkit-reviews] review granted: [Bug 169547] REGRESSION (r213764): Background image from sprite sheet incorrectly scaled : [Attachment 305870] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 169547: REGRESSION (r213764): Background image from sprite sheet
incorrectly scaled
https://bugs.webkit.org/show_bug.cgi?id=169547

Attachment 305870: Patch

https://bugs.webkit.org/attachment.cgi?id=305870&action=review




--- Comment #66 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 305870
  --> https://bugs.webkit.org/attachment.cgi?id=305870
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305870&action=review

> Source/WebCore/ChangeLog:15
> +	   To fix this issue, we must use the size of the image and not
destRect.size().

You say "size of the image", but the srcRect parameter to
nativeImageDrawingScale() isn't necessarily the bounds of the whole image. We
may be drawing a subsection of the image.

> Source/WebCore/platform/graphics/cg/NativeImageCG.cpp:83
> -float subsamplingScale(GraphicsContext& context, const FloatRect& destRect,
const FloatRect& srcRect)
> +FloatSize nativeImageDrawingScale(GraphicsContext& context, const FloatRect&
destRect, const FloatRect& srcRect)
>  {
>      // Never use subsampled images for drawing into PDF contexts.
>      if (wkCGContextIsPDFContext(context.platformContext()))
> -	   return 1;
> +	   return { 1, 1 };
>  
>      CGRect transformedDestinationRect = CGRectApplyAffineTransform(destRect,
CGContextGetCTM(context.platformContext()));
> -    return std::min<float>(1, std::max(transformedDestinationRect.size.width
/ srcRect.width(), transformedDestinationRect.size.height / srcRect.height()));
> +    return { static_cast<float>(transformedDestinationRect.size.width /
srcRect.width()), static_cast<float>(transformedDestinationRect.size.height /
srcRect.height()) };

It's weird to have this bare function for each platform, but it's mostly cross
platform code (you can call getCTM on the GraphicsContext). This should become
a function on maybe GraphicsContext at some point.