[webkit-reviews] review granted: [Bug 67415] Text drawn via -webkit-background-clip:text is blurry at device scale factors >1.0 : [Attachment 108254] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Beth Dakin <bdakin at apple.com>'s
request for review:
Bug 67415: Text drawn via -webkit-background-clip:text is blurry at device
scale factors >1.0
https://bugs.webkit.org/show_bug.cgi?id=67415

Attachment 108254: Patch
https://bugs.webkit.org/attachment.cgi?id=108254&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=108254&action=review


r=me as is even though I have some suggestions for improvement

Is there a good way to make a test for this?

> Source/WebCore/rendering/RenderBoxModelObject.cpp:692
> +	   float deviceScaleFactor = Page::deviceScaleFactor(frame());

I’m surprised we made this helper a Page static member, even though I think I
possibly suggested putting it in Page.h. I think it could instead just be a
namespace-level function that takes a Frame*.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:705
> +	   // To create a mask image of the appropriate resolution, we need to
scale maskRect's size
> +	   // by the device scale factor.
> +	   float deviceScaleFactor = Page::deviceScaleFactor(frame());
> +	   IntSize scaledMaskSize = maskRect.size();
> +	   scaledMaskSize.scale(deviceScaleFactor);
>	   
>	   // Now create the mask.
> -	   OwnPtr<ImageBuffer> maskImage =
ImageBuffer::create(maskRect.size());
> +	   OwnPtr<ImageBuffer> maskImage = ImageBuffer::create(scaledMaskSize);

>	   if (!maskImage)
>	       return;
>	   
>	   GraphicsContext* maskImageContext = maskImage->context();
> +
> +	   // Scale the whole context by the device scale factor so that all of
the clips set up at 
> +	   // the appropriate size.
> +	   maskImageContext->scale(FloatSize(deviceScaleFactor,
deviceScaleFactor));

It seems to me that without adding a feature to ImageBuffer where it knows its
scale factor, we could create a single helper function that creates an image
buffer given a size and a scale factor. It would do all the steps here:
Increase the size by the scale factor, create the buffer, scale the context.