Bug 255425

Summary: Remove 'image.isPDFDocumentImage' from ImageQualityController.cpp
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: ImagesAssignee: 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
Reported 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!
Attachments
Radar WebKit Bug Importer
Comment 1 2023-04-20 16:55:21 PDT
Said Abou-Hallawa
Comment 2 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.
Ahmad Saleem
Comment 3 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.
Note You need to log in before you can comment on or make changes to this bug.