[webkit-reviews] review granted: [Bug 67885] Outline for the high-resolution broken image icon draws at 2x : [Attachment 107223] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Beth Dakin <bdakin at apple.com>'s request for review:
Bug 67885: Outline for the high-resolution broken image icon draws at 2x
https://bugs.webkit.org/show_bug.cgi?id=67885

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

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=107223&action=review


>>> Source/WebCore/rendering/RenderImage.cpp:92
>>> +		 imageSize.scale(0.5f);        
>> 
>> It seems strange to always multiply by 1/2 rather than by
1/deviceScaleFactor. Is this relying on some knowledge of which sizes the
broken image is available at?
> 
> This matter came up in https://bugs.webkit.org/show_bug.cgi?id=67819 
> 
> In that bug, Darin felt strongly that code similar to this in RenderLayer
should use 0.5 instead of 1/deviceScaleFactor which was how I had coded it at
one point. His argument was that currently we only have 2x images so (to quote
him directly): "If the scale factor was 4 we would load the 2x image and then
scale its size down by 4, so so display a half-sized resized. Not good! It
would just get worse at higher resolutions."

I see. Dividing by the scale factor would be incorrect, and doing this is
correct, but the reason why it’s correct is nowhere near this code. A comment
here could explain why this is the right thing to do, but still won’t help us
remember to change this when we add more versions of the image.