Bug 91877 – [Chromium] Broken image icon does not appear in high dpi

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Chromium] Broken image icon does not appear in high dpi

Created attachment 153530 [details]
Patch

Preliminary patch

[Dana Jansens]

Comment on attachment 153530 [details]
Patch

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

> Source/WebCore/rendering/RenderImage.cpp:317
> +            if (m_imageResource->errorOccurred() && deviceScaleFactor*usableWidth >= image->width() && deviceScaleFactor*usableHeight >= image->height() && !image->isNull()) {

why not just change the value of usable* when they are being set above? Should the "- 2" be scaled by deviceScaleFactor also?

(In reply to comment #2)
> (From update of attachment 153530 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=153530&action=review
> 
> > Source/WebCore/rendering/RenderImage.cpp:317
> > +            if (m_imageResource->errorOccurred() && deviceScaleFactor*usableWidth >= image->width() && deviceScaleFactor*usableHeight >= image->height() && !image->isNull()) {
> 
> why not just change the value of usable* when they are being set above? Should the "- 2" be scaled by deviceScaleFactor also?

The value is used below and need to be in original form, otherwise the image will be in a wrong position.

If usableWidth represents what it's supposed to (usable width), then I'd guess all its components should be scaled.

Also see comments on crbug.com/136118.

[Dana Jansens]

Comment on attachment 153530 [details]
Patch

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

>>> Source/WebCore/rendering/RenderImage.cpp:317
>>> +            if (m_imageResource->errorOccurred() && deviceScaleFactor*usableWidth >= image->width() && deviceScaleFactor*usableHeight >= image->height() && !image->isNull()) {
>> 
>> why not just change the value of usable* when they are being set above? Should the "- 2" be scaled by deviceScaleFactor also?
> 
> The value is used below and need to be in original form, otherwise the image will be in a wrong position.
> 
> If usableWidth represents what it's supposed to (usable width), then I'd guess all its components should be scaled.
> 
> Also see comments on crbug.com/136118.

Ok, the image is scaled by only deviceScaleFactor, so I agree this seems right to do if the context scale is always >= deviceScaleFactor. This does seem right in the context scale = deviceScale case, and won't be any different in other cases.

style nit: put spaces around * and maybe consider using a temporary variable for them eg deviceScaledUsuableWidth.

[Terry Anderson]

If https://bugs.webkit.org/show_bug.cgi?id=90190 lands before this patch, please upload baselines for the following two tests:

fast/hidpi/broken-image-icon-hidpi.html
fast/hidpi/broken-image-with-size-hidpi.html

and also unskip these in TestExpectations

[Terry Anderson]

(In reply to comment #5)
> If https://bugs.webkit.org/show_bug.cgi?id=90190 lands before this patch, please upload baselines for the following two tests:
> 
> fast/hidpi/broken-image-icon-hidpi.html
> fast/hidpi/broken-image-with-size-hidpi.html
> 
> and also unskip these in TestExpectations

Sorry, please disregard my previous comment for the time being. I am still working on WK90190.

This is a chromium resource issue, no change is required in WebKit. See http://crbug.com/136118 for details.