Bug 255425
| Summary: | Remove 'image.isPDFDocumentImage' from ImageQualityController.cpp | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
| Component: | Images | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED INVALID | ||
| Severity: | Normal | CC: | sabouhallawa, simon.fraser, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=121207 | ||
Ahmad Saleem
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!
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/108345917>
Said Abou-Hallawa
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.
Ahmad Saleem
(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.