255425 2023-04-13 16:54:13 -0700 Remove 'image.isPDFDocumentImage' from ImageQualityController.cpp 2023-04-26 10:21:15 -0700 1 1 1 Unclassified WebKit Images WebKit Nightly Build Unspecified Unspecified RESOLVED INVALID https://bugs.webkit.org/show_bug.cgi?id=121207 InRadar P2 Normal --- 1 ahmad.saleem792 webkit-unassigned sabouhallawa simon.fraser webkit-bug-importer oldest_to_newest 1948641 0 ahmad.saleem792 2023-04-13 16:54:13 -0700 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! 1950188 1 webkit-bug-importer 2023-04-20 16:55:21 -0700 <rdar://problem/108345917> 1951358 2 sabouhallawa 2023-04-26 10:11:38 -0700 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. 1951359 3 ahmad.saleem792 2023-04-26 10:21:15 -0700 (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.