Bug 255425 - Remove 'image.isPDFDocumentImage' from ImageQualityController.cpp
Summary: Remove 'image.isPDFDocumentImage' from ImageQualityController.cpp
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-04-13 16:54 PDT by Ahmad Saleem
Modified: 2023-04-26 10:21 PDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-04-13 16:54:13 PDT
Hi Team,

While trying to do bug 115158, in the PR 5111, Said mentioned following:

_______________________

I think the image interpolation quality has nothing to do with the PDFDocumentImage. So I think we either need to remove the condition image.isPDFDocumentImage() or we fix the comment.

and

Yes. Please remove image.isPDFDocumentImage() from this if-statement since PDFDocumentImage::draw() does not call the ImagePaintingOptions::interpolationQuality().

________________________

I didn't get rid of it that time but I think I can do this in this separate bug.

Still present - https://github.com/Ahmad-S792/WebKit/blob/main/Source/WebCore/rendering/ImageQualityController.cpp#L119

Just wanted to raise and also get input again whether it is still good to go.

Happy to do PR.

Thanks!
Comment 1 Radar WebKit Bug Importer 2023-04-20 16:55:21 PDT
<rdar://problem/108345917>
Comment 2 Said Abou-Hallawa 2023-04-26 10:11:38 PDT
I think I was wrong.

Looking at the history of ImageQualityController.cpp, I found the bug 121207 added the condition image.isPDFDocumentImage() in ImageQualityController::chooseInterpolationQuality(). I found this statement in the ChangeLog regarding this change:

    * rendering/ImageQualityController.cpp:
    (WebCore::ImageQualityController::shouldPaintAtLowQuality):
    PDFDocumentImage is also interested in/capable of low-quality painting now.

I think this is correct since we usually cache the PDFDocumentImage in a CachedSubImage. And later we draw the ImageBuffer of the CachedSubImage to the GraphicsContext. To draw the ImageBuffer, we have to get a NativeImage from it and then call GraphicsContext::drawNativeImageInternal() which uses ImagePaintingOptions::interpolationQuality() to draw the image and decide whether to use the subImageCache or not.
Comment 3 Ahmad Saleem 2023-04-26 10:21:15 PDT
(In reply to Said Abou-Hallawa from comment #2)
> I think I was wrong.
> 
> Looking at the history of ImageQualityController.cpp, I found the bug 121207
> added the condition image.isPDFDocumentImage() in
> ImageQualityController::chooseInterpolationQuality(). I found this statement
> in the ChangeLog regarding this change:
> 
>     * rendering/ImageQualityController.cpp:
>     (WebCore::ImageQualityController::shouldPaintAtLowQuality):
>     PDFDocumentImage is also interested in/capable of low-quality painting
> now.
> 
> I think this is correct since we usually cache the PDFDocumentImage in a
> CachedSubImage. And later we draw the ImageBuffer of the CachedSubImage to
> the GraphicsContext. To draw the ImageBuffer, we have to get a NativeImage
> from it and then call GraphicsContext::drawNativeImageInternal() which uses
> ImagePaintingOptions::interpolationQuality() to draw the image and decide
> whether to use the subImageCache or not.

Closing my PR and this bug as well as RESOLVED INVALID.