Diff
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (93579 => 93580)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-08-23 04:09:07 UTC (rev 93579)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-08-23 05:45:49 UTC (rev 93580)
@@ -3620,6 +3620,11 @@
BUGWK65462 VISTA : http/tests/cache/history-only-cached-subresource-loads-max-age-https.html = PASS TIMEOUT
+// Needs new baselines for change to image resize caching code.
+BUGWK65587 : svg/dynamic-updates/SVGFEImageElement-dom-preserveAspectRatio-attr.html = IMAGE
+BUGWK65587 : svg/dynamic-updates/SVGFEImageElement-svgdom-preserveAspectRatio-prop.html = IMAGE
+BUGWK65587 : svg/custom/image-small-width-height.svg = IMAGE
+
// Introduced in r92298, which might cause another test crashing.
BUGZMO SKIP : fast/loader/reload-zero-byte-plugin.html = FAIL
Modified: trunk/Source/WebCore/ChangeLog (93579 => 93580)
--- trunk/Source/WebCore/ChangeLog 2011-08-23 04:09:07 UTC (rev 93579)
+++ trunk/Source/WebCore/ChangeLog 2011-08-23 05:45:49 UTC (rev 93580)
@@ -1,3 +1,26 @@
+2011-08-22 John Bates <[email protected]>
+
+ Implemented skia support for caching resizes of cropped images.
+ https://bugs.webkit.org/show_bug.cgi?id=65587
+
+ Reviewed by Darin Fisher.
+
+ Previously, resizes of cropped images would not be cached. This causes various websites to have janky CSS animations in software compositing mode.
+
+ * platform/graphics/skia/ImageSkia.cpp:
+ (WebCore::drawResampledBitmap): Changed to use new APIs for subset caching.
+ (WebCore::Image::drawPattern): Added allowCaching parameter.
+ * platform/graphics/skia/NativeImageSkia.cpp:
+ (WebCore::NativeImageSkia::NativeImageSkia):
+ (WebCore::NativeImageSkia::CachedImageInfo::CachedImageInfo):
+ (WebCore::NativeImageSkia::CachedImageInfo::isEqual):
+ (WebCore::NativeImageSkia::CachedImageInfo::set):
+ (WebCore::NativeImageSkia::hasResizedBitmap): Changed this method so that it does not modify caching data. Added a second version used for cropped image resizes.
+ (WebCore::NativeImageSkia::resizedBitmap): Added parameter to let caller specify whether caching is allowed.
+ (WebCore::NativeImageSkia::shouldCacheResampling): Added a second version used for cropped image resizes.
+ (WebCore::NativeImageSkia::shouldCacheResamplingInternal): Both shouldCacheResampling methods call down to this for the shared logic.
+ * platform/graphics/skia/NativeImageSkia.h: Added CachedImageInfo to uniquely identify the cached or requested image resize operation.
+
2011-08-22 Tony Gentilcore <[email protected]>
[chromium] Fonts returned by FontCache::getFontDataForCharacters() are never released
Modified: trunk/Source/WebCore/platform/graphics/skia/ImageSkia.cpp (93579 => 93580)
--- trunk/Source/WebCore/platform/graphics/skia/ImageSkia.cpp 2011-08-23 04:09:07 UTC (rev 93579)
+++ trunk/Source/WebCore/platform/graphics/skia/ImageSkia.cpp 2011-08-23 05:45:49 UTC (rev 93580)
@@ -161,25 +161,11 @@
// scrolling, for example, we are only drawing a small strip of the image.
// Resampling the whole image every time is very slow, so this speeds up things
// dramatically.
+//
+// Note: this code is only used when the canvas transformation is limited to
+// scaling or translation.
static void drawResampledBitmap(SkCanvas& canvas, SkPaint& paint, const NativeImageSkia& bitmap, const SkIRect& srcIRect, const SkRect& destRect)
{
- // First get the subset we need. This is efficient and does not copy pixels.
- SkBitmap subset;
- bitmap.extractSubset(&subset, srcIRect);
- SkRect srcRect;
- srcRect.set(srcIRect);
-
- // Whether we're doing a subset or using the full source image.
- bool srcIsFull = srcIRect.fLeft == 0 && srcIRect.fTop == 0
- && srcIRect.width() == bitmap.width()
- && srcIRect.height() == bitmap.height();
-
- // We will always draw in integer sizes, so round the destination rect.
- SkIRect destRectRounded;
- destRect.round(&destRectRounded);
- SkIRect resizedImageRect = // Represents the size of the resized image.
- { 0, 0, destRectRounded.width(), destRectRounded.height() };
-
// Apply forward transform to destRect to estimate required size of
// re-sampled bitmap, and use only in calls required to resize, or that
// check for the required size.
@@ -188,70 +174,35 @@
SkIRect destRectTransformedRounded;
destRectTransformed.round(&destRectTransformedRounded);
- if (srcIsFull && bitmap.hasResizedBitmap(destRectTransformedRounded.width(), destRectTransformedRounded.height())) {
- // Yay, this bitmap frame already has a resized version.
- SkBitmap resampled = bitmap.resizedBitmap(destRectTransformedRounded.width(), destRectTransformedRounded.height());
- canvas.drawBitmapRect(resampled, 0, destRect, &paint);
- return;
- }
-
// Compute the visible portion of our rect.
- // We also need to compute the transformed portion of the
- // visible portion for use below.
- SkRect destBitmapSubsetSk;
- ClipRectToCanvas(canvas, destRect, &destBitmapSubsetSk);
- SkRect destBitmapSubsetTransformed;
- canvas.getTotalMatrix().mapRect(&destBitmapSubsetTransformed, destBitmapSubsetSk);
- destBitmapSubsetSk.offset(-destRect.fLeft, -destRect.fTop);
+ SkRect destRectVisibleSubset;
+ ClipRectToCanvas(canvas, destRect, &destRectVisibleSubset);
+ // ClipRectToCanvas often overshoots, resulting in a larger region than our
+ // original destRect. Intersecting gets us back inside.
+ if (!destRectVisibleSubset.intersect(destRect))
+ return; // Nothing visible in destRect.
+
+ // Compute the transformed (screen space) portion of the visible portion for
+ // use below.
+ SkRect destRectVisibleSubsetTransformed;
+ canvas.getTotalMatrix().mapRect(&destRectVisibleSubsetTransformed, destRectVisibleSubset);
+ SkRect destBitmapSubsetTransformed = destRectVisibleSubsetTransformed;
+ destBitmapSubsetTransformed.offset(-destRectTransformed.fLeft,
+ -destRectTransformed.fTop);
SkIRect destBitmapSubsetTransformedRounded;
destBitmapSubsetTransformed.round(&destBitmapSubsetTransformedRounded);
- destBitmapSubsetTransformedRounded.offset(-destRectTransformedRounded.fLeft, -destRectTransformedRounded.fTop);
- // The matrix inverting, etc. could have introduced rounding error which
- // causes the bounds to be outside of the resized bitmap. We round outward
- // so we always lean toward it being larger rather than smaller than we
- // need, and then clamp to the bitmap bounds so we don't get any invalid
- // data.
- SkIRect destBitmapSubsetSkI;
- destBitmapSubsetSk.roundOut(&destBitmapSubsetSkI);
- if (!destBitmapSubsetSkI.intersect(resizedImageRect))
- return; // Resized image does not intersect.
+ // Transforms above plus rounding may cause destBitmapSubsetTransformedRounded
+ // to go outside the image, so need to clip to avoid problems.
+ if (!destBitmapSubsetTransformedRounded.intersect(
+ 0, 0, destRectTransformedRounded.width(), destRectTransformedRounded.height()))
+ return; // Image is not visible.
- 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.
- SkBitmap resampled = bitmap.resizedBitmap(destRectTransformedRounded.width(),
- destRectTransformedRounded.height());
- canvas.drawBitmapRect(resampled, 0, destRect, &paint);
- } else {
- // We should only resize the exposed part of the bitmap to do the
- // minimal possible work.
-
- // Resample the needed part of the image.
- // Transforms above plus rounding may cause destBitmapSubsetTransformedRounded
- // to go outside the image, so need to clip to avoid problems.
- if (destBitmapSubsetTransformedRounded.intersect(0, 0,
- destRectTransformedRounded.width(), destRectTransformedRounded.height())) {
-
- SkBitmap resampled = skia::ImageOperations::Resize(subset,
- skia::ImageOperations::RESIZE_LANCZOS3,
- destRectTransformedRounded.width(), destRectTransformedRounded.height(),
- destBitmapSubsetTransformedRounded);
-
- // Compute where the new bitmap should be drawn. Since our new bitmap
- // may be smaller than the original, we have to shift it over by the
- // same amount that we cut off the top and left.
- destBitmapSubsetSkI.offset(destRect.fLeft, destRect.fTop);
- SkRect offsetDestRect;
- offsetDestRect.set(destBitmapSubsetSkI);
-
- canvas.drawBitmapRect(resampled, 0, offsetDestRect, &paint);
- }
- }
+ SkBitmap resampled = bitmap.resizedBitmap(srcIRect,
+ destRectTransformedRounded.width(),
+ destRectTransformedRounded.height(),
+ destBitmapSubsetTransformedRounded);
+ canvas.drawBitmapRect(resampled, 0, destRectVisibleSubset, &paint);
}
static void paintSkBitmap(PlatformContextSkia* platformContext, const NativeImageSkia& bitmap, const SkIRect& srcRect, const SkRect& destRect, const SkXfermode::Mode& compOp)
@@ -340,58 +291,37 @@
{
FloatRect normSrcRect = normalizeRect(floatSrcRect);
if (destRect.isEmpty() || normSrcRect.isEmpty())
- return; // nothing to draw
+ return; // nothing to draw
NativeImageSkia* bitmap = nativeImageForCurrentFrame();
if (!bitmap)
return;
- // This is a very inexpensive operation. It will generate a new bitmap but
- // it will internally reference the old bitmap's pixels, adjusting the row
- // stride so the extra pixels appear as padding to the subsetted bitmap.
- SkBitmap srcSubset;
SkIRect srcRect = enclosingIntRect(normSrcRect);
- bitmap->extractSubset(&srcSubset, srcRect);
- SkBitmap resampled;
- SkShader* shader;
-
// Figure out what size the bitmap will be in the destination. The
// destination rect is the bounds of the pattern, we need to use the
- // matrix to see how bit it will be.
+ // matrix to see how big it will be.
float destBitmapWidth, destBitmapHeight;
TransformDimensions(patternTransform, srcRect.width(), srcRect.height(),
&destBitmapWidth, &destBitmapHeight);
// Compute the resampling mode.
ResamplingMode resampling;
- if (context->platformContext()->isAccelerated())
+ if (context->platformContext()->isAccelerated() || context->platformContext()->printing())
resampling = RESAMPLE_LINEAR;
- else {
- if (context->platformContext()->printing())
- resampling = RESAMPLE_LINEAR;
- else
- resampling = computeResamplingMode(context->platformContext(), *bitmap, srcRect.width(), srcRect.height(), destBitmapWidth, destBitmapHeight);
- }
+ else
+ resampling = computeResamplingMode(context->platformContext(), *bitmap, srcRect.width(), srcRect.height(), destBitmapWidth, destBitmapHeight);
// Load the transform WebKit requested.
SkMatrix matrix(patternTransform);
+ SkShader* shader;
if (resampling == RESAMPLE_AWESOME) {
// Do nice resampling.
- SkBitmap resampled;
int width = static_cast<int>(destBitmapWidth);
int height = static_cast<int>(destBitmapHeight);
- if (!srcRect.fLeft && !srcRect.fTop
- && srcRect.fRight == bitmap->width() && srcRect.fBottom == bitmap->height()
- && (bitmap->hasResizedBitmap(width, height)
- || bitmap->shouldCacheResampling(width, height, width, height))) {
- // resizedBitmap() caches resized image.
- resampled = bitmap->resizedBitmap(width, height);
- } else {
- resampled = skia::ImageOperations::Resize(srcSubset,
- skia::ImageOperations::RESIZE_LANCZOS3, width, height);
- }
+ SkBitmap resampled = bitmap->resizedBitmap(srcRect, width, height);
shader = SkShader::CreateBitmapShader(resampled, SkShader::kRepeat_TileMode, SkShader::kRepeat_TileMode);
// Since we just resized the bitmap, we need to undo the scale set in
@@ -400,6 +330,8 @@
matrix.setScaleY(SkIntToScalar(1));
} else {
// No need to do nice resampling.
+ SkBitmap srcSubset;
+ bitmap->extractSubset(&srcSubset, srcRect);
shader = SkShader::CreateBitmapShader(srcSubset, SkShader::kRepeat_TileMode, SkShader::kRepeat_TileMode);
}
Modified: trunk/Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp (93579 => 93580)
--- trunk/Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp 2011-08-23 04:09:07 UTC (rev 93579)
+++ trunk/Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp 2011-08-23 05:45:49 UTC (rev 93580)
@@ -40,7 +40,6 @@
NativeImageSkia::NativeImageSkia()
: m_isDataComplete(false),
- m_lastRequestSize(0, 0),
m_resizeRequests(0)
{
}
@@ -48,12 +47,10 @@
NativeImageSkia::NativeImageSkia(const SkBitmap& other)
: SkBitmap(other),
m_isDataComplete(false),
- m_lastRequestSize(0, 0),
m_resizeRequests(0)
{
}
-
NativeImageSkia::~NativeImageSkia()
{
}
@@ -63,33 +60,53 @@
return getSize() + m_resizedImage.getSize();
}
-bool NativeImageSkia::hasResizedBitmap(int w, int h) const
+bool NativeImageSkia::hasResizedBitmap(const SkIRect& srcSubset, int destWidth, int destHeight) const
{
- if (m_lastRequestSize.width() == w && m_lastRequestSize.height() == h)
- m_resizeRequests++;
- else {
- m_lastRequestSize = IntSize(w, h);
- m_resizeRequests = 0;
- }
-
- return m_resizedImage.width() == w && m_resizedImage.height() == h;
+ return m_cachedImageInfo.isEqual(srcSubset, destWidth, destHeight) && !m_resizedImage.empty();
}
-// FIXME: don't cache when image is in-progress.
-
-SkBitmap NativeImageSkia::resizedBitmap(int w, int h) const
+SkBitmap NativeImageSkia::resizedBitmap(const SkIRect& srcSubset,
+ int destWidth,
+ int destHeight,
+ const SkIRect& destVisibleSubset) const
{
- if (m_resizedImage.width() != w || m_resizedImage.height() != h)
- m_resizedImage = skia::ImageOperations::Resize(*this, skia::ImageOperations::RESIZE_LANCZOS3, w, h);
+ if (!hasResizedBitmap(srcSubset, destWidth, destHeight)) {
+ bool shouldCache = m_isDataComplete
+ && shouldCacheResampling(srcSubset, destWidth, destHeight, destVisibleSubset);
- return m_resizedImage;
+ SkBitmap subset;
+ extractSubset(&subset, srcSubset);
+ if (!shouldCache) {
+ // Just resize the visible subset and return it.
+ SkBitmap resizedImage = skia::ImageOperations::Resize(subset, skia::ImageOperations::RESIZE_LANCZOS3, destWidth, destHeight, destVisibleSubset);
+ return resizedImage;
+ }
+
+ m_resizedImage = skia::ImageOperations::Resize(subset, skia::ImageOperations::RESIZE_LANCZOS3, destWidth, destHeight);
+ }
+
+ SkBitmap visibleBitmap;
+ m_resizedImage.extractSubset(&visibleBitmap, destVisibleSubset);
+ return visibleBitmap;
}
-bool NativeImageSkia::shouldCacheResampling(int destWidth,
+bool NativeImageSkia::shouldCacheResampling(const SkIRect& srcSubset,
+ int destWidth,
int destHeight,
- int destSubsetWidth,
- int destSubsetHeight) const
+ const SkIRect& destVisibleSubset) const
{
+ // Check whether the requested dimensions match previous request.
+ bool matchesPreviousRequest = m_cachedImageInfo.isEqual(srcSubset, destWidth, destHeight);
+ if (matchesPreviousRequest)
+ ++m_resizeRequests;
+ else {
+ m_cachedImageInfo.set(srcSubset, destWidth, destHeight);
+ m_resizeRequests = 0;
+ // Reset m_resizedImage now, because we don't distinguish between the
+ // last requested resize info and m_resizedImage's resize info.
+ m_resizedImage.reset();
+ }
+
// We can not cache incomplete frames. This might be a good optimization in
// the future, were we know how much of the frame has been decoded, so when
// we incrementally draw more of the image, we only have to resample the
@@ -106,22 +123,31 @@
// If "too many" requests have been made for this bitmap, we assume that
// many more will be made as well, and we'll go ahead and cache it.
static const int kManyRequestThreshold = 4;
- if (m_lastRequestSize.width() == destWidth &&
- m_lastRequestSize.height() == destHeight) {
- if (m_resizeRequests >= kManyRequestThreshold)
- return true;
- } else {
- // When a different size is being requested, count this as a query
- // (hasResizedBitmap) and reset the counter.
- m_lastRequestSize = IntSize(destWidth, destHeight);
- m_resizeRequests = 0;
- }
+ if (m_resizeRequests >= kManyRequestThreshold)
+ return true;
- // Otherwise, use the heuristic that if more than 1/4 of the image is
- // requested, it's worth caching.
- int destSize = destWidth * destHeight;
- int destSubsetSize = destSubsetWidth * destSubsetHeight;
- return destSize / 4 < destSubsetSize;
+ // If more than 1/4 of the resized image is visible, it's worth caching.
+ int destVisibleSize = destVisibleSubset.width() * destVisibleSubset.height();
+ return (destVisibleSize > (destWidth * destHeight) / 4);
}
+NativeImageSkia::CachedImageInfo::CachedImageInfo()
+{
+ srcSubset.setEmpty();
+}
+
+bool NativeImageSkia::CachedImageInfo::isEqual(const SkIRect& otherSrcSubset, int width, int height) const
+{
+ return srcSubset == otherSrcSubset
+ && requestSize.width() == width
+ && requestSize.height() == height;
+}
+
+void NativeImageSkia::CachedImageInfo::set(const SkIRect& otherSrcSubset, int width, int height)
+{
+ srcSubset = otherSrcSubset;
+ requestSize.setWidth(width);
+ requestSize.setHeight(height);
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/graphics/skia/NativeImageSkia.h (93579 => 93580)
--- trunk/Source/WebCore/platform/graphics/skia/NativeImageSkia.h 2011-08-23 04:09:07 UTC (rev 93579)
+++ trunk/Source/WebCore/platform/graphics/skia/NativeImageSkia.h 2011-08-23 05:45:49 UTC (rev 93580)
@@ -32,6 +32,7 @@
#define NativeImageSkia_h
#include "SkBitmap.h"
+#include "SkRect.h"
#include "IntSize.h"
namespace WebCore {
@@ -63,30 +64,58 @@
// We can keep a resized version of the bitmap cached on this object.
// This function will return true if there is a cached version of the
- // given image subset with the given dimensions.
- bool hasResizedBitmap(int width, int height) const;
+ // given image subset with the given dimensions and subsets.
+ bool hasResizedBitmap(const SkIRect& srcSubset, int width, int height) const;
- // This will return an existing resized image, or generate a new one of
- // the specified size and store it in the cache. Subsetted images can not
- // be cached unless the subset is the entire bitmap.
- SkBitmap resizedBitmap(int width, int height) const;
+ // This will return an existing resized image subset, or generate a new one
+ // of the specified size and subsets and possibly cache it.
+ // srcSubset is the subset of the image to resize in image space.
+ SkBitmap resizedBitmap(const SkIRect& srcSubset, int destWidth, int destHeight) const
+ {
+ SkIRect destVisibleSubset = {0, 0, destWidth, destHeight};
+ return resizedBitmap(srcSubset, destWidth, destHeight, destVisibleSubset);
+ }
+ // Same as above, but returns a subset of the destination image (ie: the
+ // visible subset). destVisibleSubset is the subset of the resized
+ // (destWidth x destHeight) image.
+ // In other words:
+ // - crop image by srcSubset -> imageSubset.
+ // - resize imageSubset to destWidth x destHeight -> destImage.
+ // - return destImage cropped by destVisibleSubset.
+ SkBitmap resizedBitmap(const SkIRect& srcSubset, int destWidth, int destHeight, const SkIRect& destVisibleSubset) const;
+
+private:
+ // CachedImageInfo is used to uniquely identify cached or requested image
+ // resizes.
+ struct CachedImageInfo {
+ IntSize requestSize;
+ SkIRect srcSubset;
+
+ CachedImageInfo();
+
+ bool isEqual(const SkIRect& otherSrcSubset, int width, int height) const;
+ void set(const SkIRect& otherSrcSubset, int width, int height);
+ };
+
// Returns true if the given resize operation should either resize the whole
// image and cache it, or resize just the part it needs and throw the result
// away.
//
+ // Calling this function may increment a request count that can change the
+ // result of subsequent calls.
+ //
// On the one hand, if only a small subset is desired, then we will waste a
// lot of time resampling the entire thing, so we only want to do exactly
// what's required. On the other hand, resampling the entire bitmap is
// better if we're going to be using it more than once (like a bitmap
// scrolling on and off the screen. Since we only cache when doing the
// entire thing, it's best to just do it up front.
- bool shouldCacheResampling(int destWidth,
+ bool shouldCacheResampling(const SkIRect& srcSubset,
+ int destWidth,
int destHeight,
- int destSubsetWidth,
- int destSubsetHeight) const;
+ const SkIRect& destSubset) const;
-private:
// Set to true when the data is complete. Before the entire image has
// loaded, we do not want to cache a resize.
bool m_isDataComplete;
@@ -97,14 +126,17 @@
// References how many times that the image size has been requested for
// the last size.
//
- // Every time we get a request, if it matches the m_lastRequestSize, we'll
- // increment the counter, and if not, we'll reset the counter and save the
- // size.
+ // Every time we get a call to shouldCacheResampling, if it matches the
+ // m_cachedImageInfo, we'll increment the counter, and if not, we'll reset
+ // the counter and save the dimensions.
//
// This allows us to see if many requests have been made for the same
// resized image, we know that we should probably cache it, even if all of
// those requests individually are small and would not otherwise be cached.
- mutable IntSize m_lastRequestSize;
+ //
+ // We also track the source and destination subsets for caching partial
+ // image resizes.
+ mutable CachedImageInfo m_cachedImageInfo;
mutable int m_resizeRequests;
};
