Text drawn via -webkit-background-clip:text should be non-blurry with all scaling techniques

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Beth Dakin]

This is a followup bug from https://bugs.webkit.org/show_bug.cgi?id=67415 . RenderBoxModelObject has a static function named createDeviceScaledImageBuffer(). That function should take page scale, page zoom, and transforms from ancestors into account as well.

[Darin Adler]

Created attachment 108428 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 108428 [details]
Patch

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

r=me, perhaps with a little more thought about the naming.

> Source/WebCore/platform/graphics/GraphicsContext.h:418
> +        PassOwnPtr<ImageBuffer> createCompatibleBuffer(const IntSize&) const;

I'm not sure that "compatible" says enough about the intent. It needs to communicate "appropriately scaled", or use a term like "resolution" or "scale". Maybe createScaledBuffer()?

[Beth Dakin]

Looks good! It would be nice to add a test for this at some point. DumpRenderTree and WebKitTestRunner both have a scalePageBy function that tests the Page::setPageScaleFactor().

[Darin Adler]

Comment on attachment 108428 [details]
Patch

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

>> Source/WebCore/platform/graphics/GraphicsContext.h:418
>> +        PassOwnPtr<ImageBuffer> createCompatibleBuffer(const IntSize&) const;
> 
> I'm not sure that "compatible" says enough about the intent. It needs to communicate "appropriately scaled", or use a term like "resolution" or "scale". Maybe createScaledBuffer()?

Maybe createCompatiblyScaledBuffer? Part of why I named it this way is that eventually it might need to do something more than just scale--maybe set up an appropriate color space or something--but that’s a vague thought at the moment.

[Darin Adler]

Before landing I probably should add a test.

I also wonder whether it’s a bad policy for this to make a smaller buffer when things are scaled down.

I think I should check for overflow.

I wonder whether I need to add any rules about maximum buffer sizes. Probably already handled at the ImageBuffer level.

[Beth Dakin]

(In reply to comment #5)
> Before landing I probably should add a test.
> 
> I also wonder whether it’s a bad policy for this to make a smaller buffer when things are scaled down.
> 
> I think I should check for overflow.
> 
> I wonder whether I need to add any rules about maximum buffer sizes. Probably already handled at the ImageBuffer level.

I made a test for this, and I would like to check it in along with your patch. I'm just wondering if you have any further thoughts regarding the other possible changes you mentioned. I will think them over myself too.

[Beth Dakin]

It seems like ImageBuffer takes care of overflow…of course we would end up with something of a strange size if we overflowed so much that we had a different positive number when performing the size * scale calculations in GraphicsContext. But ImageBuffer will at least protect us from crashing and and negative sizes.

[Beth Dakin]

I also haven't been able to see any problems with creating a smaller image buffer when things are scaled down.

[Beth Dakin]

Committed change with revision 97481.

[Beth Dakin]

And a Leopard build fix with revision 97487.