From 20ee1baa69715365e53d12ffb0e9d05d8dc0eda6 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Tue, 15 Jul 2014 16:04:55 -0400 Subject: Change SkCanvasState to use inheritance. Cherry-pick from master. The base class, SkCanvasState, now holds the version, width, and height. These fields will always be a necessary part of the class. (Also add in some padding.) The other fields, which may change, have been moved into the subclass, SkCanvasState_v1. If/when the version changes, it will correspond to a new subclass. In SkCanvasStateUtils::CreateFromCanvasState, check the version on the base class, then do a static_cast to the version corresponding to SkCanvasState::version. Remove CANVAS_STATE_VERSION, which is redundant with the version specified by the subclass. Use unambiguous type for rowBytes. Build Android with SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG. This allows us to run the full suite of CanvasState tests. It is also representative of what will be used on Android by WebView. Fix CanvasStateTest where it was broken inside ifdef'ed out code. Use SkCanvas::getBaseLayerSize() instead of the deprecated SkCanvas::getDeviceSize(). Update the comments in the header to be more clear. In particular, an SkCanvasState can only be used to pass an SkCanvas' state to a future version of Skia (or the same); not an older version. NOTREECHECKS=true BUG=b/15693384 R=reed@google.com, djsollen@google.com, mtklein@google.com Author: scroggo@google.com Review URL: https://codereview.chromium.org/372003002 Review URL: https://codereview.chromium.org/396763002 --- include/utils/SkCanvasStateUtils.h | 15 ++++--- src/utils/SkCanvasStateUtils.cpp | 92 +++++++++++++++++++++----------------- 2 files changed, 60 insertions(+), 47 deletions(-) diff --git a/include/utils/SkCanvasStateUtils.h b/include/utils/SkCanvasStateUtils.h index f8266c9b3..6ea7b1030 100644 --- a/include/utils/SkCanvasStateUtils.h +++ b/include/utils/SkCanvasStateUtils.h @@ -13,9 +13,9 @@ class SkCanvasState; /** - * A set of functions that are useful for copying an SkCanvas across a library - * boundary where the Skia libraries on either side of the boundary may not be - * version identical. The expected usage is outline below... + * A set of functions that are useful for copying the state of an SkCanvas + * across a library boundary where the Skia library on the other side of the + * boundary may be newer. The expected usage is outline below... * * Lib Boundary * CaptureCanvasState(...) ||| @@ -30,8 +30,8 @@ class SkCanvasState; namespace SkCanvasStateUtils { /** * Captures the current state of the canvas into an opaque ptr that is safe - * to pass between different instances of Skia (which may or may not be the - * same version). The function will return NULL in the event that one of the + * to pass to a different instance of Skia (which may be the same version, + * or may be newer). The function will return NULL in the event that one of the * following conditions are true. * 1) the canvas device type is not supported (currently only raster is supported) * 2) the canvas clip type is not supported (currently only non-AA clips are supported) @@ -56,7 +56,7 @@ namespace SkCanvasStateUtils { * 1) the captured state is in an unrecognized format * 2) the captured canvas device type is not supported * - * @param canvas The canvas you wish to capture the current state of. + * @param state Opaque object created by CaptureCanvasState. * @return NULL or an SkCanvas* whose devices and matrix/clip state are * identical to the captured canvas. The caller is responsible for * calling unref on the SkCanvas. @@ -66,7 +66,8 @@ namespace SkCanvasStateUtils { /** * Free the memory associated with the captured canvas state. The state * should not be released until all SkCanvas objects created using that - * state have been dereferenced. + * state have been dereferenced. Must be called from the same library + * instance that created the state via CaptureCanvasState. * * @param state The captured state you wish to dispose of. */ diff --git a/src/utils/SkCanvasStateUtils.cpp b/src/utils/SkCanvasStateUtils.cpp index eaee61b5e..e286c9138 100644 --- a/src/utils/SkCanvasStateUtils.cpp +++ b/src/utils/SkCanvasStateUtils.cpp @@ -13,14 +13,13 @@ #include "SkErrorInternals.h" #include "SkWriter32.h" -#define CANVAS_STATE_VERSION 1 /* * WARNING: The structs below are part of a stable ABI and as such we explicitly * use unambigious primitives (e.g. int32_t instead of an enum). * - * ANY CHANGES TO THE STRUCTS BELOW THAT IMPACT THE ABI SHOULD RESULT IN AN - * UPDATE OF THE CANVAS_STATE_VERSION. SUCH CHANGES SHOULD ONLY BE MADE IF - * ABSOLUTELY NECESSARY! + * ANY CHANGES TO THE STRUCTS BELOW THAT IMPACT THE ABI SHOULD RESULT IN A NEW + * NEW SUBCLASS OF SkCanvasState. SUCH CHANGES SHOULD ONLY BE MADE IF ABSOLUTELY + * NECESSARY! */ enum RasterConfigs { kUnknown_RasterConfig = 0, @@ -48,7 +47,8 @@ struct SkMCState { ClipRect* clipRects; }; -// NOTE: If you add more members, bump CanvasState::version. +// NOTE: If you add more members, create a new subclass of SkCanvasState with a +// new CanvasState::version. struct SkCanvasLayerState { CanvasBackend type; int32_t x, y; @@ -60,7 +60,7 @@ struct SkCanvasLayerState { union { struct { RasterConfig config; // pixel format: a value from RasterConfigs. - size_t rowBytes; // Number of bytes from start of one line to next. + uint64_t rowBytes; // Number of bytes from start of one line to next. void* pixels; // The pixels, all (height * rowBytes) of them. } raster; struct { @@ -71,20 +71,41 @@ struct SkCanvasLayerState { class SkCanvasState { public: - SkCanvasState(SkCanvas* canvas) { + SkCanvasState(int32_t version, SkCanvas* canvas) { SkASSERT(canvas); - version = CANVAS_STATE_VERSION; - width = canvas->getDeviceSize().width(); - height = canvas->getDeviceSize().height(); + this->version = version; + width = canvas->getBaseLayerSize().width(); + height = canvas->getBaseLayerSize().height(); + + } + + /** + * The version this struct was built with. This field must always appear + * first in the struct so that when the versions don't match (and the + * remaining contents and size are potentially different) we can still + * compare the version numbers. + */ + int32_t version; + int32_t width; + int32_t height; + int32_t alignmentPadding; +}; + +class SkCanvasState_v1 : public SkCanvasState { +public: + static const int32_t kVersion = 1; + + SkCanvasState_v1(SkCanvas* canvas) + : INHERITED(kVersion, canvas) + { layerCount = 0; layers = NULL; - originalCanvas = SkRef(canvas); - mcState.clipRectCount = 0; mcState.clipRects = NULL; + originalCanvas = SkRef(canvas); } - ~SkCanvasState() { + ~SkCanvasState_v1() { // loop through the layers and free the data allocated to the clipRects for (int i = 0; i < layerCount; ++i) { sk_free(layers[i].mcState.clipRects); @@ -98,24 +119,13 @@ public: originalCanvas->unref(); } - /** - * The version this struct was built with. This field must always appear - * first in the struct so that when the versions don't match (and the - * remaining contents and size are potentially different) we can still - * compare the version numbers. - */ - int32_t version; - - int32_t width; - int32_t height; - SkMCState mcState; int32_t layerCount; SkCanvasLayerState* layers; - private: SkCanvas* originalCanvas; + typedef SkCanvasState INHERITED; }; //////////////////////////////////////////////////////////////////////////////// @@ -191,7 +201,7 @@ SkCanvasState* SkCanvasStateUtils::CaptureCanvasState(SkCanvas* canvas) { return NULL; } - SkAutoTDelete canvasState(SkNEW_ARGS(SkCanvasState, (canvas))); + SkAutoTDelete canvasState(SkNEW_ARGS(SkCanvasState_v1, (canvas))); // decompose the total matrix and clip setup_MC_state(&canvasState->mcState, canvas->getTotalMatrix(), @@ -247,7 +257,7 @@ SkCanvasState* SkCanvasStateUtils::CaptureCanvasState(SkCanvas* canvas) { // for now, just ignore any client supplied DrawFilter. if (canvas->getDrawFilter()) { -// SkDEBUGF(("CaptureCanvasState will ignore the canvases draw filter.\n")); +// SkDEBUGF(("CaptureCanvasState will ignore the canvas's draw filter.\n")); } return canvasState.detach(); @@ -291,7 +301,7 @@ static SkCanvas* create_canvas_from_canvas_layer(const SkCanvasLayerState& layer bitmap.installPixels(SkImageInfo::Make(layerState.width, layerState.height, colorType, kPremul_SkAlphaType), - layerState.raster.pixels, layerState.raster.rowBytes); + layerState.raster.pixels, (size_t) layerState.raster.rowBytes); SkASSERT(!bitmap.empty()); SkASSERT(!bitmap.isNull()); @@ -306,30 +316,28 @@ static SkCanvas* create_canvas_from_canvas_layer(const SkCanvasLayerState& layer SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state) { SkASSERT(state); + // Currently there is only one possible version. + SkASSERT(SkCanvasState_v1::kVersion == state->version); - // check that the versions match - if (CANVAS_STATE_VERSION != state->version) { - SkDebugf("CreateFromCanvasState version does not match the one use to create the input"); - return NULL; - } + const SkCanvasState_v1* state_v1 = static_cast(state); - if (state->layerCount < 1) { + if (state_v1->layerCount < 1) { return NULL; } SkAutoTUnref canvas(SkNEW_ARGS(SkCanvasStack, (state->width, state->height))); // setup the matrix and clip on the n-way canvas - setup_canvas_from_MC_state(state->mcState, canvas); + setup_canvas_from_MC_state(state_v1->mcState, canvas); // Iterate over the layers and add them to the n-way canvas - for (int i = state->layerCount - 1; i >= 0; --i) { - SkAutoTUnref canvasLayer(create_canvas_from_canvas_layer(state->layers[i])); + for (int i = state_v1->layerCount - 1; i >= 0; --i) { + SkAutoTUnref canvasLayer(create_canvas_from_canvas_layer(state_v1->layers[i])); if (!canvasLayer.get()) { return NULL; } - canvas->pushCanvas(canvasLayer.get(), SkIPoint::Make(state->layers[i].x, - state->layers[i].y)); + canvas->pushCanvas(canvasLayer.get(), SkIPoint::Make(state_v1->layers[i].x, + state_v1->layers[i].y)); } return canvas.detach(); @@ -338,5 +346,9 @@ SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state) //////////////////////////////////////////////////////////////////////////////// void SkCanvasStateUtils::ReleaseCanvasState(SkCanvasState* state) { - SkDELETE(state); + SkASSERT(!state || SkCanvasState_v1::kVersion == state->version); + // Upcast to the correct version of SkCanvasState. This avoids having a virtual destructor on + // SkCanvasState. That would be strange since SkCanvasState has no other virtual functions, and + // instead uses the field "version" to determine how to behave. + SkDELETE(static_cast(state)); } -- cgit v1.2.3 From f2d87ba43e54e1c62cfac22742210aa126113a10 Mon Sep 17 00:00:00 2001 From: bungeman Date: Wed, 23 Jul 2014 13:31:06 -0700 Subject: Get additional DW font metrics when available. BUG=chromium:395043 R=eae@chromium.org, caryclark@google.com Author: bungeman@google.com Review URL: https://codereview.chromium.org/412993002 --- src/ports/SkScalerContext_win_dw.cpp | 32 +++++++++++++++++++++++++++++--- src/ports/SkTypeface_win_dw.h | 10 +++++++++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/ports/SkScalerContext_win_dw.cpp b/src/ports/SkScalerContext_win_dw.cpp index 4cb983f5c..28d41c1b9 100644 --- a/src/ports/SkScalerContext_win_dw.cpp +++ b/src/ports/SkScalerContext_win_dw.cpp @@ -26,6 +26,7 @@ #include "SkTypeface_win_dw.h" #include +#include static bool isLCD(const SkScalerContext::Rec& rec) { return SkMask::kLCD16_Format == rec.fMaskFormat || @@ -493,10 +494,8 @@ void SkScalerContext_DW::generateFontMetrics(SkPaint::FontMetrics* mx, } if (my) { - my->fTop = -fTextSizeRender * SkIntToScalar(dwfm.ascent) / upem; - my->fAscent = my->fTop; + my->fAscent = -fTextSizeRender * SkIntToScalar(dwfm.ascent) / upem; my->fDescent = fTextSizeRender * SkIntToScalar(dwfm.descent) / upem; - my->fBottom = my->fDescent; my->fLeading = fTextSizeRender * SkIntToScalar(dwfm.lineGap) / upem; my->fXHeight = fTextSizeRender * SkIntToScalar(dwfm.xHeight) / upem; my->fUnderlineThickness = fTextSizeRender * SkIntToScalar(dwfm.underlineThickness) / upem; @@ -504,6 +503,33 @@ void SkScalerContext_DW::generateFontMetrics(SkPaint::FontMetrics* mx, my->fFlags |= SkPaint::FontMetrics::kUnderlineThinknessIsValid_Flag; my->fFlags |= SkPaint::FontMetrics::kUnderlinePositionIsValid_Flag; + + if (NULL != fTypeface->fDWriteFontFace1.get()) { + DWRITE_FONT_METRICS1 dwfm1; + fTypeface->fDWriteFontFace1->GetMetrics(&dwfm1); + my->fTop = -fTextSizeRender * SkIntToScalar(dwfm1.glyphBoxTop) / upem; + my->fBottom = -fTextSizeRender * SkIntToScalar(dwfm1.glyphBoxBottom) / upem; + my->fXMin = fTextSizeRender * SkIntToScalar(dwfm1.glyphBoxLeft) / upem; + my->fXMax = fTextSizeRender * SkIntToScalar(dwfm1.glyphBoxRight) / upem; + + my->fMaxCharWidth = my->fXMax - my->fXMin; + } else { + AutoTDWriteTable head(fTypeface->fDWriteFontFace.get()); + if (head.fExists && + head.fSize >= sizeof(SkOTTableHead) && + head->version == SkOTTableHead::version1) + { + my->fTop = -fTextSizeRender * (int16_t)SkEndian_SwapBE16(head->yMax) / upem; + my->fBottom = -fTextSizeRender * (int16_t)SkEndian_SwapBE16(head->yMin) / upem; + my->fXMin = fTextSizeRender * (int16_t)SkEndian_SwapBE16(head->xMin) / upem; + my->fXMax = fTextSizeRender * (int16_t)SkEndian_SwapBE16(head->xMax) / upem; + + my->fMaxCharWidth = my->fXMax - my->fXMin; + } else { + my->fTop = my->fAscent; + my->fBottom = my->fDescent; + } + } } } diff --git a/src/ports/SkTypeface_win_dw.h b/src/ports/SkTypeface_win_dw.h index b064ce577..465db3446 100644 --- a/src/ports/SkTypeface_win_dw.h +++ b/src/ports/SkTypeface_win_dw.h @@ -17,6 +17,7 @@ #include "SkTypes.h" #include +#include class SkFontDescriptor; struct SkScalerContextRec; @@ -50,7 +51,13 @@ private: , fDWriteFontFamily(SkRefComPtr(fontFamily)) , fDWriteFont(SkRefComPtr(font)) , fDWriteFontFace(SkRefComPtr(fontFace)) - { } + { + if (!SUCCEEDED(fDWriteFontFace->QueryInterface(&fDWriteFontFace1))) { + // IUnknown::QueryInterface states that if it fails, punk will be set to NULL. + // http://blogs.msdn.com/b/oldnewthing/archive/2004/03/26/96777.aspx + SK_ALWAYSBREAK(NULL == fDWriteFontFace1.get()); + } + } public: SkTScopedComPtr fFactory; @@ -59,6 +66,7 @@ public: SkTScopedComPtr fDWriteFontFamily; SkTScopedComPtr fDWriteFont; SkTScopedComPtr fDWriteFontFace; + SkTScopedComPtr fDWriteFontFace1; static DWriteFontTypeface* Create(IDWriteFactory* factory, IDWriteFontFace* fontFace, -- cgit v1.2.3 From 0c1c911ff19894031cd1b8acef3b597e26a85334 Mon Sep 17 00:00:00 2001 From: sugoi Date: Thu, 3 Jul 2014 10:44:26 -0700 Subject: Adding 64 bit checks Added a few more checks to avoid overflowing 32 bit sizes while computing convolutions. I also changed a dangerously misleading INHERITED typedef. BUG=389570 R=senorblanco@google.com, senorblanco@chromium.org Author: sugoi@chromium.org Review URL: https://codereview.chromium.org/361403006 --- src/core/SkConvolver.cpp | 2 +- src/core/SkScaledImageCache.cpp | 3 ++- src/effects/SkColorMatrixFilter.cpp | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/core/SkConvolver.cpp b/src/core/SkConvolver.cpp index 0e97fac07..0f5cf9021 100644 --- a/src/core/SkConvolver.cpp +++ b/src/core/SkConvolver.cpp @@ -434,7 +434,7 @@ void BGRAConvolve2D(const unsigned char* sourceData, } // Compute where in the output image this row of final data will go. - unsigned char* curOutputRow = &output[outY * outputByteRowStride]; + unsigned char* curOutputRow = &output[(uint64_t)outY * outputByteRowStride]; // Get the list of rows that the circular buffer has, in order. int firstRowInCircularBuffer; diff --git a/src/core/SkScaledImageCache.cpp b/src/core/SkScaledImageCache.cpp index f266f9717..a03024819 100644 --- a/src/core/SkScaledImageCache.cpp +++ b/src/core/SkScaledImageCache.cpp @@ -266,7 +266,8 @@ private: bool SkScaledImageCacheDiscardableAllocator::allocPixelRef(SkBitmap* bitmap, SkColorTable* ctable) { size_t size = bitmap->getSize(); - if (0 == size) { + uint64_t size64 = bitmap->computeSize64(); + if (0 == size || size64 > (uint64_t)size) { return false; } diff --git a/src/effects/SkColorMatrixFilter.cpp b/src/effects/SkColorMatrixFilter.cpp index b60fa84b6..bd1df79d1 100644 --- a/src/effects/SkColorMatrixFilter.cpp +++ b/src/effects/SkColorMatrixFilter.cpp @@ -450,6 +450,8 @@ public: private: GrGLUniformManager::UniformHandle fMatrixHandle; GrGLUniformManager::UniformHandle fVectorHandle; + + typedef GrGLEffect INHERITED; }; private: @@ -462,7 +464,7 @@ private: SkColorMatrix fMatrix; - typedef GrGLEffect INHERITED; + typedef GrEffect INHERITED; }; GR_DEFINE_EFFECT_TEST(ColorMatrixEffect); -- cgit v1.2.3 From 0d78ac294f814a45380eb44b0bb4c01aae2cab01 Mon Sep 17 00:00:00 2001 From: Hal Canary Date: Tue, 12 Aug 2014 15:28:22 -0400 Subject: Set maximum output size for scaled-image-cache images Accessable via: SkGraphics::{G|S}etImageCacheSingleAllocationByteLimit() Also, a unit test. BUG=389439 R=reed@google.com, humper@google.com, reveman@chromium.org, tomhudson@google.com, vangelis@chromium.org Author: halcanary@google.com Review URL: https://codereview.chromium.org/394003003 Review URL: https://codereview.chromium.org/462993003 --- bench/ImageCacheBench.cpp | 2 +- gyp/tests.gypi | 1 + include/core/SkGraphics.h | 44 ++++++++++++++++++++-- src/core/SkBitmapProcState.cpp | 19 +++++++++- src/core/SkScaledImageCache.cpp | 82 ++++++++++++++++++++++++++++------------- src/core/SkScaledImageCache.h | 35 ++++++++++++------ tests/ImageCacheTest.cpp | 2 +- tests/ScaledImageCache.cpp | 60 ++++++++++++++++++++++++++++++ 8 files changed, 200 insertions(+), 45 deletions(-) create mode 100644 tests/ScaledImageCache.cpp diff --git a/bench/ImageCacheBench.cpp b/bench/ImageCacheBench.cpp index 5f1715fc3..e65d1fc3e 100644 --- a/bench/ImageCacheBench.cpp +++ b/bench/ImageCacheBench.cpp @@ -37,7 +37,7 @@ protected: } virtual void onDraw(const int loops, SkCanvas*) SK_OVERRIDE { - if (fCache.getBytesUsed() == 0) { + if (fCache.getTotalBytesUsed() == 0) { this->populateCache(); } diff --git a/gyp/tests.gypi b/gyp/tests.gypi index 2a7dddd4f..9e418d687 100644 --- a/gyp/tests.gypi +++ b/gyp/tests.gypi @@ -155,6 +155,7 @@ '../tests/RuntimeConfigTest.cpp', '../tests/SHA1Test.cpp', '../tests/ScalarTest.cpp', + '../tests/ScaledImageCache.cpp', '../tests/SerializationTest.cpp', '../tests/ShaderImageFilterTest.cpp', '../tests/ShaderOpacityTest.cpp', diff --git a/include/core/SkGraphics.h b/include/core/SkGraphics.h index 2667a388d..e7865ca5a 100644 --- a/include/core/SkGraphics.h +++ b/include/core/SkGraphics.h @@ -79,9 +79,47 @@ public: */ static void PurgeFontCache(); - static size_t GetImageCacheBytesUsed(); - static size_t GetImageCacheByteLimit(); - static size_t SetImageCacheByteLimit(size_t newLimit); + /** + * Scaling bitmaps with the SkPaint::kHigh_FilterLevel setting is + * expensive, so the result is saved in the global Scaled Image + * Cache. + * + * This function returns the memory usage of the Scaled Image Cache. + */ + static size_t GetImageCacheTotalBytesUsed(); + /** + * These functions get/set the memory usage limit for the Scaled + * Image Cache. Bitmaps are purged from the cache when the + * memory useage exceeds this limit. + */ + static size_t GetImageCacheTotalByteLimit(); + static size_t SetImageCacheTotalByteLimit(size_t newLimit); + + // DEPRECATED + static size_t GetImageCacheBytesUsed() { + return GetImageCacheTotalBytesUsed(); + } + // DEPRECATED + static size_t GetImageCacheByteLimit() { + return GetImageCacheTotalByteLimit(); + } + // DEPRECATED + static size_t SetImageCacheByteLimit(size_t newLimit) { + return SetImageCacheTotalByteLimit(newLimit); + } + + /** + * Scaling bitmaps with the SkPaint::kHigh_FilterLevel setting is + * expensive, so the result is saved in the global Scaled Image + * Cache. When the resulting bitmap is too large, this can + * overload the cache. If the ImageCacheSingleAllocationByteLimit + * is set to a non-zero number, and the resulting bitmap would be + * larger than that value, the bitmap scaling algorithm falls + * back onto a cheaper algorithm and does not cache the result. + * Zero is the default value. + */ + static size_t GetImageCacheSingleAllocationByteLimit(); + static size_t SetImageCacheSingleAllocationByteLimit(size_t newLimit); /** * Applications with command line options may pass optional state, such diff --git a/src/core/SkBitmapProcState.cpp b/src/core/SkBitmapProcState.cpp index f6f9f3fa0..137bcda91 100644 --- a/src/core/SkBitmapProcState.cpp +++ b/src/core/SkBitmapProcState.cpp @@ -127,6 +127,21 @@ private: }; #define AutoScaledCacheUnlocker(...) SK_REQUIRE_LOCAL_VAR(AutoScaledCacheUnlocker) +// Check to see that the size of the bitmap that would be produced by +// scaling by the given inverted matrix is less than the maximum allowed. +static inline bool cache_size_okay(const SkBitmap& bm, const SkMatrix& invMat) { + size_t maximumAllocation + = SkScaledImageCache::GetSingleAllocationByteLimit(); + if (0 == maximumAllocation) { + return true; + } + // float matrixScaleFactor = 1.0 / (invMat.scaleX * invMat.scaleY); + // return ((origBitmapSize * matrixScaleFactor) < maximumAllocationSize); + // Skip the division step: + return bm.info().getSafeSize(bm.info().minRowBytes()) + < (maximumAllocation * invMat.getScaleX() * invMat.getScaleY()); +} + // TODO -- we may want to pass the clip into this function so we only scale // the portion of the image that we're going to need. This will complicate // the interface to the cache, but might be well worth it. @@ -140,14 +155,14 @@ bool SkBitmapProcState::possiblyScaleImage() { if (fFilterLevel <= SkPaint::kLow_FilterLevel) { return false; } - // Check to see if the transformation matrix is simple, and if we're // doing high quality scaling. If so, do the bitmap scale here and // remove the scaling component from the matrix. if (SkPaint::kHigh_FilterLevel == fFilterLevel && fInvMatrix.getType() <= (SkMatrix::kScale_Mask | SkMatrix::kTranslate_Mask) && - kN32_SkColorType == fOrigBitmap.colorType()) { + kN32_SkColorType == fOrigBitmap.colorType() && + cache_size_okay(fOrigBitmap, fInvMatrix)) { SkScalar invScaleX = fInvMatrix.getScaleX(); SkScalar invScaleY = fInvMatrix.getScaleY(); diff --git a/src/core/SkScaledImageCache.cpp b/src/core/SkScaledImageCache.cpp index a03024819..43ff7ef89 100644 --- a/src/core/SkScaledImageCache.cpp +++ b/src/core/SkScaledImageCache.cpp @@ -165,12 +165,13 @@ void SkScaledImageCache::init() { #else fHash = NULL; #endif - fBytesUsed = 0; + fTotalBytesUsed = 0; fCount = 0; + fSingleAllocationByteLimit = 0; fAllocator = NULL; // One of these should be explicit set by the caller after we return. - fByteLimit = 0; + fTotalByteLimit = 0; fDiscardableFactory = NULL; } @@ -297,7 +298,7 @@ SkScaledImageCache::SkScaledImageCache(DiscardableFactory factory) { SkScaledImageCache::SkScaledImageCache(size_t byteLimit) { this->init(); - fByteLimit = byteLimit; + fTotalByteLimit = byteLimit; } SkScaledImageCache::~SkScaledImageCache() { @@ -502,10 +503,10 @@ void SkScaledImageCache::purgeAsNeeded() { byteLimit = SK_MaxU32; // no limit based on bytes } else { countLimit = SK_MaxS32; // no limit based on count - byteLimit = fByteLimit; + byteLimit = fTotalByteLimit; } - size_t bytesUsed = fBytesUsed; + size_t bytesUsed = fTotalBytesUsed; int countUsed = fCount; Rec* rec = fTail; @@ -531,13 +532,13 @@ void SkScaledImageCache::purgeAsNeeded() { rec = prev; } - fBytesUsed = bytesUsed; + fTotalBytesUsed = bytesUsed; fCount = countUsed; } -size_t SkScaledImageCache::setByteLimit(size_t newLimit) { - size_t prevLimit = fByteLimit; - fByteLimit = newLimit; +size_t SkScaledImageCache::setTotalByteLimit(size_t newLimit) { + size_t prevLimit = fTotalByteLimit; + fTotalByteLimit = newLimit; if (newLimit < prevLimit) { this->purgeAsNeeded(); } @@ -597,7 +598,7 @@ void SkScaledImageCache::addToHead(Rec* rec) { if (!fTail) { fTail = rec; } - fBytesUsed += rec->bytesUsed(); + fTotalBytesUsed += rec->bytesUsed(); fCount += 1; this->validate(); @@ -609,14 +610,14 @@ void SkScaledImageCache::addToHead(Rec* rec) { void SkScaledImageCache::validate() const { if (NULL == fHead) { SkASSERT(NULL == fTail); - SkASSERT(0 == fBytesUsed); + SkASSERT(0 == fTotalBytesUsed); return; } if (fHead == fTail) { SkASSERT(NULL == fHead->fPrev); SkASSERT(NULL == fHead->fNext); - SkASSERT(fHead->bytesUsed() == fBytesUsed); + SkASSERT(fHead->bytesUsed() == fTotalBytesUsed); return; } @@ -631,7 +632,7 @@ void SkScaledImageCache::validate() const { while (rec) { count += 1; used += rec->bytesUsed(); - SkASSERT(used <= fBytesUsed); + SkASSERT(used <= fTotalBytesUsed); rec = rec->fNext; } SkASSERT(fCount == count); @@ -661,10 +662,20 @@ void SkScaledImageCache::dump() const { } SkDebugf("SkScaledImageCache: count=%d bytes=%d locked=%d %s\n", - fCount, fBytesUsed, locked, + fCount, fTotalBytesUsed, locked, fDiscardableFactory ? "discardable" : "malloc"); } +size_t SkScaledImageCache::setSingleAllocationByteLimit(size_t newLimit) { + size_t oldLimit = fSingleAllocationByteLimit; + fSingleAllocationByteLimit = newLimit; + return oldLimit; +} + +size_t SkScaledImageCache::getSingleAllocationByteLimit() const { + return fSingleAllocationByteLimit; +} + /////////////////////////////////////////////////////////////////////////////// #include "SkThread.h" @@ -751,19 +762,19 @@ void SkScaledImageCache::Unlock(SkScaledImageCache::ID* id) { // get_cache()->dump(); } -size_t SkScaledImageCache::GetBytesUsed() { +size_t SkScaledImageCache::GetTotalBytesUsed() { SkAutoMutexAcquire am(gMutex); - return get_cache()->getBytesUsed(); + return get_cache()->getTotalBytesUsed(); } -size_t SkScaledImageCache::GetByteLimit() { +size_t SkScaledImageCache::GetTotalByteLimit() { SkAutoMutexAcquire am(gMutex); - return get_cache()->getByteLimit(); + return get_cache()->getTotalByteLimit(); } -size_t SkScaledImageCache::SetByteLimit(size_t newLimit) { +size_t SkScaledImageCache::SetTotalByteLimit(size_t newLimit) { SkAutoMutexAcquire am(gMutex); - return get_cache()->setByteLimit(newLimit); + return get_cache()->setTotalByteLimit(newLimit); } SkBitmap::Allocator* SkScaledImageCache::GetAllocator() { @@ -776,18 +787,37 @@ void SkScaledImageCache::Dump() { get_cache()->dump(); } +size_t SkScaledImageCache::SetSingleAllocationByteLimit(size_t size) { + SkAutoMutexAcquire am(gMutex); + return get_cache()->setSingleAllocationByteLimit(size); +} + +size_t SkScaledImageCache::GetSingleAllocationByteLimit() { + SkAutoMutexAcquire am(gMutex); + return get_cache()->getSingleAllocationByteLimit(); +} + /////////////////////////////////////////////////////////////////////////////// #include "SkGraphics.h" -size_t SkGraphics::GetImageCacheBytesUsed() { - return SkScaledImageCache::GetBytesUsed(); +size_t SkGraphics::GetImageCacheTotalBytesUsed() { + return SkScaledImageCache::GetTotalBytesUsed(); +} + +size_t SkGraphics::GetImageCacheTotalByteLimit() { + return SkScaledImageCache::GetTotalByteLimit(); } -size_t SkGraphics::GetImageCacheByteLimit() { - return SkScaledImageCache::GetByteLimit(); +size_t SkGraphics::SetImageCacheTotalByteLimit(size_t newLimit) { + return SkScaledImageCache::SetTotalByteLimit(newLimit); } -size_t SkGraphics::SetImageCacheByteLimit(size_t newLimit) { - return SkScaledImageCache::SetByteLimit(newLimit); +size_t SkGraphics::GetImageCacheSingleAllocationByteLimit() { + return SkScaledImageCache::GetSingleAllocationByteLimit(); } + +size_t SkGraphics::SetImageCacheSingleAllocationByteLimit(size_t newLimit) { + return SkScaledImageCache::SetSingleAllocationByteLimit(newLimit); +} + diff --git a/src/core/SkScaledImageCache.h b/src/core/SkScaledImageCache.h index fe072306d..817147e3b 100644 --- a/src/core/SkScaledImageCache.h +++ b/src/core/SkScaledImageCache.h @@ -60,9 +60,12 @@ public: static void Unlock(ID*); - static size_t GetBytesUsed(); - static size_t GetByteLimit(); - static size_t SetByteLimit(size_t newLimit); + static size_t GetTotalBytesUsed(); + static size_t GetTotalByteLimit(); + static size_t SetTotalByteLimit(size_t newLimit); + + static size_t SetSingleAllocationByteLimit(size_t); + static size_t GetSingleAllocationByteLimit(); static SkBitmap::Allocator* GetAllocator(); @@ -76,9 +79,9 @@ public: /** * Construct the cache to call DiscardableFactory when it * allocates memory for the pixels. In this mode, the cache has - * not explicit budget, and so methods like getBytesUsed() and - * getByteLimit() will return 0, and setByteLimit will ignore its argument - * and return 0. + * not explicit budget, and so methods like getTotalBytesUsed() + * and getTotalByteLimit() will return 0, and setTotalByteLimit + * will ignore its argument and return 0. */ SkScaledImageCache(DiscardableFactory); @@ -86,7 +89,7 @@ public: * Construct the cache, allocating memory with malloc, and respect the * byteLimit, purging automatically when a new image is added to the cache * that pushes the total bytesUsed over the limit. Note: The limit can be - * changed at runtime with setByteLimit. + * changed at runtime with setTotalByteLimit. */ SkScaledImageCache(size_t byteLimit); @@ -144,15 +147,22 @@ public: */ void unlock(ID*); - size_t getBytesUsed() const { return fBytesUsed; } - size_t getByteLimit() const { return fByteLimit; } + size_t getTotalBytesUsed() const { return fTotalBytesUsed; } + size_t getTotalByteLimit() const { return fTotalByteLimit; } + /** + * This is respected by SkBitmapProcState::possiblyScaleImage. + * 0 is no maximum at all; this is the default. + * setSingleAllocationByteLimit() returns the previous value. + */ + size_t setSingleAllocationByteLimit(size_t maximumAllocationSize); + size_t getSingleAllocationByteLimit() const; /** * Set the maximum number of bytes available to this cache. If the current * cache exceeds this new value, it will be purged to try to fit within * this new limit. */ - size_t setByteLimit(size_t newLimit); + size_t setTotalByteLimit(size_t newLimit); SkBitmap::Allocator* allocator() const { return fAllocator; }; @@ -175,8 +185,9 @@ private: // the allocator is NULL or one that matches discardables SkBitmap::Allocator* fAllocator; - size_t fBytesUsed; - size_t fByteLimit; + size_t fTotalBytesUsed; + size_t fTotalByteLimit; + size_t fSingleAllocationByteLimit; int fCount; Rec* findAndLock(uint32_t generationID, SkScalar sx, SkScalar sy, diff --git a/tests/ImageCacheTest.cpp b/tests/ImageCacheTest.cpp index 92d0b519d..00f6c77aa 100644 --- a/tests/ImageCacheTest.cpp +++ b/tests/ImageCacheTest.cpp @@ -74,7 +74,7 @@ static void test_cache(skiatest::Reporter* reporter, SkScaledImageCache& cache, } } - cache.setByteLimit(0); + cache.setTotalByteLimit(0); } #include "SkDiscardableMemoryPool.h" diff --git a/tests/ScaledImageCache.cpp b/tests/ScaledImageCache.cpp new file mode 100644 index 000000000..2040afea2 --- /dev/null +++ b/tests/ScaledImageCache.cpp @@ -0,0 +1,60 @@ +/* + * Copyright 2014 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ +#include "Test.h" +#include "SkGraphics.h" +#include "SkCanvas.h" + +static const int kCanvasSize = 1; +static const int kBitmapSize = 16; +static const int kScale = 8; + +static size_t test_scaled_image_cache_useage() { + SkAutoTUnref canvas( + SkCanvas::NewRasterN32(kCanvasSize, kCanvasSize)); + SkBitmap bitmap; + SkAssertResult(bitmap.allocN32Pixels(kBitmapSize, kBitmapSize)); + SkScalar scaledSize = SkIntToScalar(kScale * kBitmapSize); + canvas->clipRect(SkRect::MakeLTRB(0, 0, scaledSize, scaledSize)); + SkPaint paint; + paint.setFilterLevel(SkPaint::kHigh_FilterLevel); + size_t bytesUsed = SkGraphics::GetImageCacheBytesUsed(); + canvas->drawBitmapRect(bitmap, + SkRect::MakeLTRB(0, 0, scaledSize, scaledSize), + &paint); + return SkGraphics::GetImageCacheBytesUsed() - bytesUsed; +} + +// http://crbug.com/389439 +DEF_TEST(ScaledImageCache_SingleAllocationByteLimit, reporter) { + size_t originalByteLimit = SkGraphics::GetImageCacheByteLimit(); + size_t originalAllocationLimit = + SkGraphics::GetImageCacheSingleAllocationByteLimit(); + + size_t size = kBitmapSize * kScale * kBitmapSize * kScale + * SkColorTypeBytesPerPixel(kN32_SkColorType); + + SkGraphics::SetImageCacheByteLimit(0); // clear cache + SkGraphics::SetImageCacheByteLimit(2 * size); + SkGraphics::SetImageCacheSingleAllocationByteLimit(0); + + REPORTER_ASSERT(reporter, size == test_scaled_image_cache_useage()); + + SkGraphics::SetImageCacheByteLimit(0); // clear cache + SkGraphics::SetImageCacheByteLimit(2 * size); + SkGraphics::SetImageCacheSingleAllocationByteLimit(size * 2); + + REPORTER_ASSERT(reporter, size == test_scaled_image_cache_useage()); + + SkGraphics::SetImageCacheByteLimit(0); // clear cache + SkGraphics::SetImageCacheByteLimit(2 * size); + SkGraphics::SetImageCacheSingleAllocationByteLimit(size / 2); + + REPORTER_ASSERT(reporter, 0 == test_scaled_image_cache_useage()); + + SkGraphics::SetImageCacheSingleAllocationByteLimit(originalAllocationLimit); + SkGraphics::SetImageCacheByteLimit(originalByteLimit); +} -- cgit v1.2.3 From 413546c54f7ab90c8b584a2f7056fabd4f918acf Mon Sep 17 00:00:00 2001 From: reed Date: Sun, 13 Jul 2014 04:32:32 -0700 Subject: Add SkBitmap::readPixels() and reimplement copyTo and SkCanvas::readPixels This reverts commit 651eaeadeb0b1407f5fe192aeda90db1680fa2b8. TBR= Author: reed@chromium.org Review URL: https://codereview.chromium.org/390693002 --- include/core/SkBitmap.h | 22 +++++++ include/core/SkCanvas.h | 21 +++---- src/core/SkBitmap.cpp | 127 ++++++++++++++++++++++----------------- src/core/SkBitmapDevice.cpp | 76 +----------------------- src/core/SkConfig8888.cpp | 142 ++++++++++++++++++++++++++++++++++++++++++++ src/core/SkConfig8888.h | 4 ++ tests/BitmapCopyTest.cpp | 95 ++++++++++++++++++++++++++++- 7 files changed, 346 insertions(+), 141 deletions(-) diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h index d3c33a8e0..13e9aa372 100644 --- a/include/core/SkBitmap.h +++ b/include/core/SkBitmap.h @@ -642,6 +642,28 @@ public: return this->copyTo(dst, this->colorType(), allocator); } + /** + * Copy the bitmap's pixels into the specified buffer (pixels + rowBytes), + * converting them into the requested format (SkImageInfo). The src pixels are read + * starting at the specified (srcX,srcY) offset, relative to the top-left corner. + * + * The specified ImageInfo and (srcX,srcY) offset specifies a source rectangle + * + * srcR.setXYWH(srcX, srcY, dstInfo.width(), dstInfo.height()); + * + * srcR is intersected with the bounds of the bitmap. If this intersection is not empty, + * then we have two sets of pixels (of equal size). Replace the dst pixels with the + * corresponding src pixels, performing any colortype/alphatype transformations needed + * (in the case where the src and dst have different colortypes or alphatypes). + * + * This call can fail, returning false, for several reasons: + * - If srcR does not intersect the bitmap bounds. + * - If the requested colortype/alphatype cannot be converted from the src's types. + * - If the src pixels are not available. + */ + bool readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes, + int srcX, int srcY) const; + /** * Returns true if this bitmap's pixels can be converted into the requested * colorType, such that copyTo() could succeed. diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index a08e82800..31429f941 100644 --- a/include/core/SkCanvas.h +++ b/include/core/SkCanvas.h @@ -230,30 +230,31 @@ public: /** * Copy the pixels from the base-layer into the specified buffer (pixels + rowBytes), * converting them into the requested format (SkImageInfo). The base-layer pixels are read - * starting at the specified (x,y) location in the coordinate system of the base-layer. + * starting at the specified (srcX,srcY) location in the coordinate system of the base-layer. * - * The specified ImageInfo and (x,y) offset specifies a source rectangle + * The specified ImageInfo and (srcX,srcY) offset specifies a source rectangle * - * srcR(x, y, info.width(), info.height()); + * srcR.setXYWH(srcX, srcY, dstInfo.width(), dstInfo.height()); * - * SrcR is intersected with the bounds of the base-layer. If this intersection is not empty, - * then we have two sets of pixels (of equal size), the "src" specified by base-layer at (x,y) - * and the "dst" by info+pixels+rowBytes. Replace the dst pixels with the corresponding src - * pixels, performing any colortype/alphatype transformations needed (in the case where the - * src and dst have different colortypes or alphatypes). + * srcR is intersected with the bounds of the base-layer. If this intersection is not empty, + * then we have two sets of pixels (of equal size). Replace the dst pixels with the + * corresponding src pixels, performing any colortype/alphatype transformations needed + * (in the case where the src and dst have different colortypes or alphatypes). * * This call can fail, returning false, for several reasons: + * - If srcR does not intersect the base-layer bounds. * - If the requested colortype/alphatype cannot be converted from the base-layer's types. * - If this canvas is not backed by pixels (e.g. picture or PDF) */ - bool readPixels(const SkImageInfo&, void* pixels, size_t rowBytes, int x, int y); + bool readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes, + int srcX, int srcY); /** * Helper for calling readPixels(info, ...). This call will check if bitmap has been allocated. * If not, it will attempt to call allocPixels(). If this fails, it will return false. If not, * it calls through to readPixels(info, ...) and returns its result. */ - bool readPixels(SkBitmap* bitmap, int x, int y); + bool readPixels(SkBitmap* bitmap, int srcX, int srcY); /** * Helper for allocating pixels and then calling readPixels(info, ...). The bitmap is resized diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index 789bf11b4..42254b518 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -873,11 +873,13 @@ bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const { #include "SkPaint.h" bool SkBitmap::canCopyTo(SkColorType dstColorType) const { - if (this->colorType() == kUnknown_SkColorType) { + const SkColorType srcCT = this->colorType(); + + if (srcCT == kUnknown_SkColorType) { return false; } - bool sameConfigs = (this->colorType() == dstColorType); + bool sameConfigs = (srcCT == dstColorType); switch (dstColorType) { case kAlpha_8_SkColorType: case kRGB_565_SkColorType: @@ -890,15 +892,66 @@ bool SkBitmap::canCopyTo(SkColorType dstColorType) const { } break; case kARGB_4444_SkColorType: - return sameConfigs || kN32_SkColorType == this->colorType(); + return sameConfigs || kN32_SkColorType == srcCT || kIndex_8_SkColorType == srcCT; default: return false; } return true; } -bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, - Allocator* alloc) const { +#include "SkConfig8888.h" + +bool SkBitmap::readPixels(const SkImageInfo& requestedDstInfo, void* dstPixels, size_t dstRB, + int x, int y) const { + if (kUnknown_SkColorType == requestedDstInfo.colorType()) { + return false; + } + if (NULL == dstPixels || dstRB < requestedDstInfo.minRowBytes()) { + return false; + } + if (0 == requestedDstInfo.width() || 0 == requestedDstInfo.height()) { + return false; + } + + SkIRect srcR = SkIRect::MakeXYWH(x, y, requestedDstInfo.width(), requestedDstInfo.height()); + if (!srcR.intersect(0, 0, this->width(), this->height())) { + return false; + } + + SkImageInfo dstInfo = requestedDstInfo; + // the intersect may have shrunk info's logical size + dstInfo.fWidth = srcR.width(); + dstInfo.fHeight = srcR.height(); + + // if x or y are negative, then we have to adjust pixels + if (x > 0) { + x = 0; + } + if (y > 0) { + y = 0; + } + // here x,y are either 0 or negative + dstPixels = ((char*)dstPixels - y * dstRB - x * dstInfo.bytesPerPixel()); + + ////////////// + + SkAutoLockPixels alp(*this); + + // since we don't stop creating un-pixeled devices yet, check for no pixels here + if (NULL == this->getPixels()) { + return false; + } + + SkImageInfo srcInfo = this->info(); + srcInfo.fWidth = dstInfo.width(); + srcInfo.fHeight = dstInfo.height(); + + const void* srcPixels = this->getAddr(srcR.x(), srcR.y()); + return SkPixelInfo::CopyPixels(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, this->rowBytes(), + this->getColorTable()); +} + +bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc) const { if (!this->canCopyTo(dstColorType)) { return false; } @@ -971,59 +1024,21 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, // returned false. SkASSERT(tmpDst.pixelRef() != NULL); - /* do memcpy for the same configs cases, else use drawing - */ - if (src->colorType() == dstColorType) { - if (tmpDst.getSize() == src->getSize()) { - memcpy(tmpDst.getPixels(), src->getPixels(), src->getSafeSize()); + if (!src->readPixels(tmpDst.info(), tmpDst.getPixels(), tmpDst.rowBytes(), 0, 0)) { + return false; + } - SkPixelRef* dstPixelRef = tmpDst.pixelRef(); - if (dstPixelRef->info() == fPixelRef->info()) { - dstPixelRef->cloneGenID(*fPixelRef); - } - } else { - const char* srcP = reinterpret_cast(src->getPixels()); - char* dstP = reinterpret_cast(tmpDst.getPixels()); - // to be sure we don't read too much, only copy our logical pixels - size_t bytesToCopy = tmpDst.width() * tmpDst.bytesPerPixel(); - for (int y = 0; y < tmpDst.height(); y++) { - memcpy(dstP, srcP, bytesToCopy); - srcP += src->rowBytes(); - dstP += tmpDst.rowBytes(); - } - } - } else if (kARGB_4444_SkColorType == dstColorType - && kN32_SkColorType == src->colorType()) { - if (src->alphaType() == kUnpremul_SkAlphaType) { - // Our method for converting to 4444 assumes premultiplied. - return false; + // (for BitmapHeap) Clone the pixelref genID even though we have a new pixelref. + // The old copyTo impl did this, so we continue it for now. + // + // TODO: should we ignore rowbytes (i.e. getSize)? Then it could just be + // if (src_pixelref->info == dst_pixelref->info) + // + if (src->colorType() == dstColorType && tmpDst.getSize() == src->getSize()) { + SkPixelRef* dstPixelRef = tmpDst.pixelRef(); + if (dstPixelRef->info() == fPixelRef->info()) { + dstPixelRef->cloneGenID(*fPixelRef); } - SkASSERT(src->height() == tmpDst.height()); - SkASSERT(src->width() == tmpDst.width()); - for (int y = 0; y < src->height(); ++y) { - SkPMColor16* SK_RESTRICT dstRow = (SkPMColor16*) tmpDst.getAddr16(0, y); - SkPMColor* SK_RESTRICT srcRow = (SkPMColor*) src->getAddr32(0, y); - DITHER_4444_SCAN(y); - for (int x = 0; x < src->width(); ++x) { - dstRow[x] = SkDitherARGB32To4444(srcRow[x], - DITHER_VALUE(x)); - } - } - } else { - if (tmpDst.alphaType() == kUnpremul_SkAlphaType) { - // We do not support drawing to unpremultiplied bitmaps. - return false; - } - - // Always clear the dest in case one of the blitters accesses it - // TODO: switch the allocation of tmpDst to call sk_calloc_throw - tmpDst.eraseColor(SK_ColorTRANSPARENT); - - SkCanvas canvas(tmpDst); - SkPaint paint; - - paint.setDither(true); - canvas.drawBitmap(*src, 0, 0, &paint); } dst->swap(tmpDst); diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp index 0f3cc2be5..1dfc3a6c0 100644 --- a/src/core/SkBitmapDevice.cpp +++ b/src/core/SkBitmapDevice.cpp @@ -155,59 +155,8 @@ void* SkBitmapDevice::onAccessPixels(SkImageInfo* info, size_t* rowBytes) { return NULL; } -static void rect_memcpy(void* dst, size_t dstRB, const void* src, size_t srcRB, size_t bytesPerRow, - int rowCount) { - SkASSERT(bytesPerRow <= srcRB); - SkASSERT(bytesPerRow <= dstRB); - for (int i = 0; i < rowCount; ++i) { - memcpy(dst, src, bytesPerRow); - dst = (char*)dst + dstRB; - src = (const char*)src + srcRB; - } -} - #include "SkConfig8888.h" -static bool copy_pixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes, - const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes) { - if (srcInfo.dimensions() != dstInfo.dimensions()) { - return false; - } - if (4 == srcInfo.bytesPerPixel() && 4 == dstInfo.bytesPerPixel()) { - SkDstPixelInfo dstPI; - dstPI.fColorType = dstInfo.colorType(); - dstPI.fAlphaType = dstInfo.alphaType(); - dstPI.fPixels = dstPixels; - dstPI.fRowBytes = dstRowBytes; - - SkSrcPixelInfo srcPI; - srcPI.fColorType = srcInfo.colorType(); - srcPI.fAlphaType = srcInfo.alphaType(); - srcPI.fPixels = srcPixels; - srcPI.fRowBytes = srcRowBytes; - - return srcPI.convertPixelsTo(&dstPI, srcInfo.width(), srcInfo.height()); - } - if (srcInfo.colorType() == dstInfo.colorType()) { - switch (srcInfo.colorType()) { - case kRGB_565_SkColorType: - case kAlpha_8_SkColorType: - break; - case kARGB_4444_SkColorType: - if (srcInfo.alphaType() != dstInfo.alphaType()) { - return false; - } - break; - default: - return false; - } - rect_memcpy(dstPixels, dstRowBytes, srcPixels, srcRowBytes, - srcInfo.width() * srcInfo.bytesPerPixel(), srcInfo.height()); - } - // TODO: add support for more conversions as needed - return false; -} - bool SkBitmapDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes, int x, int y) { // since we don't stop creating un-pixeled devices yet, check for no pixels here @@ -222,7 +171,7 @@ bool SkBitmapDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPi void* dstPixels = fBitmap.getAddr(x, y); size_t dstRowBytes = fBitmap.rowBytes(); - if (copy_pixels(dstInfo, dstPixels, dstRowBytes, srcInfo, srcPixels, srcRowBytes)) { + if (SkPixelInfo::CopyPixels(dstInfo, dstPixels, dstRowBytes, srcInfo, srcPixels, srcRowBytes)) { fBitmap.notifyPixelsChanged(); return true; } @@ -231,28 +180,7 @@ bool SkBitmapDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPi bool SkBitmapDevice::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes, int x, int y) { - // since we don't stop creating un-pixeled devices yet, check for no pixels here - if (NULL == fBitmap.getPixels()) { - return false; - } - - SkImageInfo srcInfo = fBitmap.info(); - - // perhaps can relax these in the future - if (4 != dstInfo.bytesPerPixel()) { - return false; - } - if (4 != srcInfo.bytesPerPixel()) { - return false; - } - - srcInfo.fWidth = dstInfo.width(); - srcInfo.fHeight = dstInfo.height(); - - const void* srcPixels = fBitmap.getAddr(x, y); - const size_t srcRowBytes = fBitmap.rowBytes(); - - return copy_pixels(dstInfo, dstPixels, dstRowBytes, srcInfo, srcPixels, srcRowBytes); + return fBitmap.readPixels(dstInfo, dstPixels, dstRowBytes, x, y); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp index 189309dae..b0572c054 100644 --- a/src/core/SkConfig8888.cpp +++ b/src/core/SkConfig8888.cpp @@ -1,5 +1,15 @@ +/* + * Copyright 2014 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkBitmap.h" +#include "SkCanvas.h" #include "SkConfig8888.h" #include "SkColorPriv.h" +#include "SkDither.h" #include "SkMathPriv.h" #include "SkUnPreMultiply.h" @@ -115,3 +125,135 @@ bool SkSrcPixelInfo::convertPixelsTo(SkDstPixelInfo* dst, int width, int height) } return true; } + +static void rect_memcpy(void* dst, size_t dstRB, const void* src, size_t srcRB, size_t bytesPerRow, + int rowCount) { + SkASSERT(bytesPerRow <= srcRB); + SkASSERT(bytesPerRow <= dstRB); + for (int i = 0; i < rowCount; ++i) { + memcpy(dst, src, bytesPerRow); + dst = (char*)dst + dstRB; + src = (const char*)src + srcRB; + } +} + +bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB, + const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRB, + SkColorTable* ctable) { + if (srcInfo.dimensions() != dstInfo.dimensions()) { + return false; + } + + const int width = srcInfo.width(); + const int height = srcInfo.height(); + + // Handle fancy alpha swizzling if both are ARGB32 + if (4 == srcInfo.bytesPerPixel() && 4 == dstInfo.bytesPerPixel()) { + SkDstPixelInfo dstPI; + dstPI.fColorType = dstInfo.colorType(); + dstPI.fAlphaType = dstInfo.alphaType(); + dstPI.fPixels = dstPixels; + dstPI.fRowBytes = dstRB; + + SkSrcPixelInfo srcPI; + srcPI.fColorType = srcInfo.colorType(); + srcPI.fAlphaType = srcInfo.alphaType(); + srcPI.fPixels = srcPixels; + srcPI.fRowBytes = srcRB; + + return srcPI.convertPixelsTo(&dstPI, width, height); + } + + // If they agree on colorType and the alphaTypes are compatible, then we just memcpy. + // Note: we've already taken care of 32bit colortypes above. + if (srcInfo.colorType() == dstInfo.colorType()) { + switch (srcInfo.colorType()) { + case kRGB_565_SkColorType: + case kAlpha_8_SkColorType: + break; + case kIndex_8_SkColorType: + case kARGB_4444_SkColorType: + if (srcInfo.alphaType() != dstInfo.alphaType()) { + return false; + } + break; + default: + return false; + } + rect_memcpy(dstPixels, dstRB, srcPixels, srcRB, width * srcInfo.bytesPerPixel(), height); + return true; + } + + /* + * Begin section where we try to change colorTypes along the way. Not all combinations + * are supported. + */ + + // Can no longer draw directly into 4444, but we can manually whack it for a few combinations + if (kARGB_4444_SkColorType == dstInfo.colorType() && + (kN32_SkColorType == srcInfo.colorType() || kIndex_8_SkColorType == srcInfo.colorType())) { + if (srcInfo.alphaType() == kUnpremul_SkAlphaType) { + // Our method for converting to 4444 assumes premultiplied. + return false; + } + + const SkPMColor* table = NULL; + if (kIndex_8_SkColorType == srcInfo.colorType()) { + if (NULL == ctable) { + return false; + } + table = ctable->lockColors(); + } + + for (int y = 0; y < height; ++y) { + DITHER_4444_SCAN(y); + SkPMColor16* SK_RESTRICT dstRow = (SkPMColor16*)dstPixels; + if (table) { + const uint8_t* SK_RESTRICT srcRow = (const uint8_t*)srcPixels; + for (int x = 0; x < width; ++x) { + dstRow[x] = SkDitherARGB32To4444(table[srcRow[x]], DITHER_VALUE(x)); + } + } else { + const SkPMColor* SK_RESTRICT srcRow = (const SkPMColor*)srcPixels; + for (int x = 0; x < width; ++x) { + dstRow[x] = SkDitherARGB32To4444(srcRow[x], DITHER_VALUE(x)); + } + } + dstPixels = (char*)dstPixels + dstRB; + srcPixels = (const char*)srcPixels + srcRB; + } + + if (table) { + ctable->unlockColors(); + } + return true; + } + + if (dstInfo.alphaType() == kUnpremul_SkAlphaType) { + // We do not support drawing to unpremultiplied bitmaps. + return false; + } + + // Final fall-back, draw with a canvas + // + // Always clear the dest in case one of the blitters accesses it + // TODO: switch the allocation of tmpDst to call sk_calloc_throw + { + SkBitmap bm; + if (!bm.installPixels(srcInfo, const_cast(srcPixels), srcRB, ctable, NULL, NULL)) { + return false; + } + SkAutoTUnref canvas(SkCanvas::NewRasterDirect(dstInfo, dstPixels, dstRB)); + if (NULL == canvas.get()) { + return false; + } + + SkPaint paint; + paint.setDither(true); + + canvas->clear(0); + canvas->drawBitmap(bm, 0, 0, &paint); + return true; + } +} + diff --git a/src/core/SkConfig8888.h b/src/core/SkConfig8888.h index 97a3433ad..da0cbc668 100644 --- a/src/core/SkConfig8888.h +++ b/src/core/SkConfig8888.h @@ -14,6 +14,10 @@ struct SkPixelInfo { SkColorType fColorType; SkAlphaType fAlphaType; size_t fRowBytes; + + static bool CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes, + const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes, + SkColorTable* srcCTable = NULL); }; struct SkDstPixelInfo : SkPixelInfo { diff --git a/tests/BitmapCopyTest.cpp b/tests/BitmapCopyTest.cpp index 40cfbe0d5..4a49a6b48 100644 --- a/tests/BitmapCopyTest.cpp +++ b/tests/BitmapCopyTest.cpp @@ -185,7 +185,7 @@ static void writeCoordPixels(SkBitmap& bm, const Coordinates& coords) { static const Pair gPairs[] = { { kUnknown_SkColorType, "000000" }, { kAlpha_8_SkColorType, "010101" }, - { kIndex_8_SkColorType, "011101" }, + { kIndex_8_SkColorType, "011111" }, { kRGB_565_SkColorType, "010101" }, { kARGB_4444_SkColorType, "010111" }, { kN32_SkColorType, "010111" }, @@ -542,3 +542,96 @@ DEF_TEST(BitmapCopy, reporter) { } // for (size_t copyCase ... } } + +#include "SkColorPriv.h" +#include "SkUtils.h" + +/** + * Construct 4x4 pixels where we can look at a color and determine where it should be in the grid. + * alpha = 0xFF, blue = 0x80, red = x, green = y + */ +static void fill_4x4_pixels(SkPMColor colors[16]) { + for (int y = 0; y < 4; ++y) { + for (int x = 0; x < 4; ++x) { + colors[y*4+x] = SkPackARGB32(0xFF, x, y, 0x80); + } + } +} + +static bool check_4x4_pixel(SkPMColor color, unsigned x, unsigned y) { + SkASSERT(x < 4 && y < 4); + return 0xFF == SkGetPackedA32(color) && + x == SkGetPackedR32(color) && + y == SkGetPackedG32(color) && + 0x80 == SkGetPackedB32(color); +} + +/** + * Fill with all zeros, which will never match any value from fill_4x4_pixels + */ +static void clear_4x4_pixels(SkPMColor colors[16]) { + sk_memset32(colors, 0, 16); +} + +// Much of readPixels is exercised by copyTo testing, since readPixels is the backend for that +// method. Here we explicitly test subset copies. +// +DEF_TEST(BitmapReadPixels, reporter) { + const int W = 4; + const int H = 4; + const size_t rowBytes = W * sizeof(SkPMColor); + const SkImageInfo srcInfo = SkImageInfo::MakeN32Premul(W, H); + SkPMColor srcPixels[16]; + fill_4x4_pixels(srcPixels); + SkBitmap srcBM; + srcBM.installPixels(srcInfo, srcPixels, rowBytes); + + SkImageInfo dstInfo = SkImageInfo::MakeN32Premul(W, H); + SkPMColor dstPixels[16]; + + const struct { + bool fExpectedSuccess; + SkIPoint fRequestedSrcLoc; + SkISize fRequestedDstSize; + // If fExpectedSuccess, check these, otherwise ignore + SkIPoint fExpectedDstLoc; + SkIRect fExpectedSrcR; + } gRec[] = { + { true, { 0, 0 }, { 4, 4 }, { 0, 0 }, { 0, 0, 4, 4 } }, + { true, { 1, 1 }, { 2, 2 }, { 0, 0 }, { 1, 1, 3, 3 } }, + { true, { 2, 2 }, { 4, 4 }, { 0, 0 }, { 2, 2, 4, 4 } }, + { true, {-1,-1 }, { 2, 2 }, { 1, 1 }, { 0, 0, 1, 1 } }, + { false, {-1,-1 }, { 1, 1 }, { 0, 0 }, { 0, 0, 0, 0 } }, + }; + + for (size_t i = 0; i < SK_ARRAY_COUNT(gRec); ++i) { + clear_4x4_pixels(dstPixels); + + dstInfo.fWidth = gRec[i].fRequestedDstSize.width(); + dstInfo.fHeight = gRec[i].fRequestedDstSize.height(); + bool success = srcBM.readPixels(dstInfo, dstPixels, rowBytes, + gRec[i].fRequestedSrcLoc.x(), gRec[i].fRequestedSrcLoc.y()); + + REPORTER_ASSERT(reporter, gRec[i].fExpectedSuccess == success); + if (success) { + const SkIRect srcR = gRec[i].fExpectedSrcR; + const int dstX = gRec[i].fExpectedDstLoc.x(); + const int dstY = gRec[i].fExpectedDstLoc.y(); + // Walk the dst pixels, and check if we got what we expected + for (int y = 0; y < H; ++y) { + for (int x = 0; x < W; ++x) { + SkPMColor dstC = dstPixels[y*4+x]; + // get into src coordinates + int sx = x - dstX + srcR.x(); + int sy = y - dstY + srcR.y(); + if (srcR.contains(sx, sy)) { + REPORTER_ASSERT(reporter, check_4x4_pixel(dstC, sx, sy)); + } else { + REPORTER_ASSERT(reporter, 0 == dstC); + } + } + } + } + } +} + -- cgit v1.2.3