Bug 37364 - ImageSkia resized image caching is not aggressive enough
Summary: ImageSkia resized image caching is not aggressive enough
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Brett Wilson (Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-09 15:21 PDT by Brett Wilson (Google)
Modified: 2013-04-09 12:51 PDT (History)
4 users (show)

See Also:


Attachments
[IMAGE] Screenshot with regression. (49.57 KB, image/png)
2010-06-03 00:20 PDT, Pavel Feldman
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brett Wilson (Google) 2010-04-09 15:21:14 PDT
I noticed a simple typo on line 187 of ImageSkia.cpp

    if (srcIsFull && bitmap.shouldCacheResampling(
            resizedImageRect.width(),
            resizedImageRect.height(),
            destBitmapSubsetSkI.width(),
            destBitmapSubsetSkI.height())) {
        // We're supposed to resize the entire image and cache it, even though
        // we don't need all of it.

The && in the if statement should be ||. The whole point of shouldCacheResampling is to catch cases where we're not doing the whole image.
Comment 1 Stephen White 2010-05-31 11:31:09 PDT
I landed this change as part of https://bugs.webkit.org/show_bug.cgi?id=38686.
Comment 2 Pavel Feldman 2010-06-03 00:19:56 PDT
Since recently, our background image spriting does not work when element is scaled. See image 
attached (toolbar button images next to Elements, Resources, etc).

James is saying WebKit r60391. Same code works on Mac fine.
Comment 3 Pavel Feldman 2010-06-03 00:20:37 PDT
Created attachment 57740 [details]
[IMAGE] Screenshot with regression.
Comment 4 Pavel Feldman 2010-06-03 00:23:05 PDT
Reverted:
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/platform/graphics/skia/ImageSkia.cpp
Committed r60611
Comment 5 Pavel Feldman 2010-06-03 01:22:36 PDT
Updated baselines. They were wrong (and different from Mac).

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/platform/chromium-linux/fast/backgrounds/size/backgroundSize15-expected.checksum
	M	LayoutTests/platform/chromium-linux/fast/backgrounds/size/backgroundSize15-expected.png
	M	LayoutTests/platform/chromium-linux/fast/borders/svg-as-border-image-expected.checksum
	M	LayoutTests/platform/chromium-linux/scrollbars/listbox-scrollbar-combinations-expected.checksum
	M	LayoutTests/platform/chromium-linux/scrollbars/overflow-scrollbar-combinations-expected.checksum
	M	LayoutTests/platform/chromium-linux/scrollbars/overflow-scrollbar-combinations-expected.png
	M	LayoutTests/platform/chromium-linux/svg/W3C-SVG-1.1/coords-viewattr-02-b-expected.checksum
	M	LayoutTests/platform/chromium-linux/svg/W3C-SVG-1.1/coords-viewattr-02-b-expected.png
	M	LayoutTests/platform/chromium-win/fast/backgrounds/size/backgroundSize15-expected.checksum
	M	LayoutTests/platform/chromium-win/fast/backgrounds/size/backgroundSize15-expected.png
	M	LayoutTests/platform/chromium-win/fast/borders/svg-as-border-image-expected.checksum
	M	LayoutTests/platform/chromium-win/fast/borders/svg-as-border-image-expected.png
	M	LayoutTests/platform/chromium-win/scrollbars/listbox-scrollbar-combinations-expected.checksum
	M	LayoutTests/platform/chromium-win/scrollbars/listbox-scrollbar-combinations-expected.png
	M	LayoutTests/platform/chromium-win/scrollbars/overflow-scrollbar-combinations-expected.checksum
	M	LayoutTests/platform/chromium-win/scrollbars/overflow-scrollbar-combinations-expected.png
	M	LayoutTests/platform/chromium-win/svg/W3C-SVG-1.1/coords-viewattr-02-b-expected.checksum
	M	LayoutTests/platform/chromium-win/svg/W3C-SVG-1.1/coords-viewattr-02-b-expected.png
Committed r60613
Comment 6 noel gordon 2012-02-09 13:44:57 PST
Is this bug fixed?
Comment 7 Stephen White 2012-02-13 07:39:54 PST
This code seems to have changed a lot since this bug was filed, so I'm not sure it's relevant anymore.  John Bates added support for resizing of cropped images in http://trac.webkit.org/changeset/93580, which I think is the same bug.  The relevant test now seems to be in NativeImageSkia::resizedBitmap().

Unless someone can come up with a test case which is still broken, I think we should close this bug.