From 6644d9353f3f0c09914385fd762e073f98d54205 Mon Sep 17 00:00:00 2001 From: reed Date: Fri, 10 Jun 2016 11:41:47 -0700 Subject: [PATCH] respect srgb gamma when building mips Proposed policy: - If the target is *legacy* (e.g. L32/PMColor) ignore gamma - If the target is S32/F16 respect gamma BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2029373004 Review-Url: https://codereview.chromium.org/2029373004 --- bench/MipMapBench.cpp | 21 +++++++---- gm/gamma.cpp | 13 ++++--- gm/mipmap.cpp | 49 ++++++++++++++++++++++++ gm/showmiplevels.cpp | 6 ++- include/private/SkTemplates.h | 6 +++ src/core/SkBitmapCache.cpp | 21 +++++++---- src/core/SkBitmapCache.h | 8 ++-- src/core/SkBitmapController.cpp | 15 +++++--- src/core/SkBitmapController.h | 7 +++- src/core/SkBitmapProcShader.cpp | 6 ++- src/core/SkBitmapProcState.cpp | 10 +++-- src/core/SkBitmapProcState.h | 17 ++++++--- src/core/SkLightingShader.cpp | 6 ++- src/core/SkMipMap.cpp | 67 ++++++++++++++++++++++++++------- src/core/SkMipMap.h | 16 +++++--- src/core/SkPM4fPriv.h | 49 ++++++++++++++++++++++++ src/gpu/SkGr.cpp | 8 +--- tests/MipMapTest.cpp | 6 +-- tests/SkResourceCacheTest.cpp | 21 +++++++---- 19 files changed, 270 insertions(+), 82 deletions(-) diff --git a/bench/MipMapBench.cpp b/bench/MipMapBench.cpp index 652524364b..1f41e817db 100644 --- a/bench/MipMapBench.cpp +++ b/bench/MipMapBench.cpp @@ -13,10 +13,13 @@ class MipMapBench: public Benchmark { SkBitmap fBitmap; SkString fName; const int fW, fH; + SkSourceGammaTreatment fTreatment; public: - MipMapBench(int w, int h) : fW(w), fH(h) { - fName.printf("mipmap_build_%dx%d", w, h); + MipMapBench(int w, int h, SkSourceGammaTreatment treatment) + : fW(w), fH(h), fTreatment(treatment) + { + fName.printf("mipmap_build_%dx%d_%d_gamma", w, h, static_cast(treatment)); } protected: @@ -27,13 +30,14 @@ protected: const char* onGetName() override { return fName.c_str(); } void onDelayedSetup() override { - fBitmap.allocN32Pixels(fW, fH, true); + SkImageInfo info = SkImageInfo::MakeS32(fW, fH, kPremul_SkAlphaType); + fBitmap.allocPixels(info); fBitmap.eraseColor(SK_ColorWHITE); // so we don't read uninitialized memory } void onDraw(int loops, SkCanvas*) override { for (int i = 0; i < loops * 4; i++) { - SkMipMap::Build(fBitmap, nullptr)->unref(); + SkMipMap::Build(fBitmap, fTreatment, nullptr)->unref(); } } @@ -44,7 +48,8 @@ private: // Build variants that exercise the width and heights being even or odd at each level, as the // impl specializes on each of these. // -DEF_BENCH( return new MipMapBench(511, 511); ) -DEF_BENCH( return new MipMapBench(512, 511); ) -DEF_BENCH( return new MipMapBench(511, 512); ) -DEF_BENCH( return new MipMapBench(512, 512); ) +DEF_BENCH( return new MipMapBench(511, 511, SkSourceGammaTreatment::kIgnore); ) +DEF_BENCH( return new MipMapBench(512, 511, SkSourceGammaTreatment::kIgnore); ) +DEF_BENCH( return new MipMapBench(511, 512, SkSourceGammaTreatment::kIgnore); ) +DEF_BENCH( return new MipMapBench(512, 512, SkSourceGammaTreatment::kIgnore); ) +DEF_BENCH( return new MipMapBench(512, 512, SkSourceGammaTreatment::kRespect); ) diff --git a/gm/gamma.cpp b/gm/gamma.cpp index 25608d26b3..c9fa28be10 100644 --- a/gm/gamma.cpp +++ b/gm/gamma.cpp @@ -9,6 +9,7 @@ #include "Resources.h" #include "SkGradientShader.h" +#include "SkPM4fPriv.h" DEF_SIMPLE_GM(gamma, canvas, 560, 200) { SkPaint p; @@ -38,14 +39,14 @@ DEF_SIMPLE_GM(gamma, canvas, 560, 200) { srgbGreyBmp.eraseARGB(0xFF, 0xBC, 0xBC, 0xBC); SkBitmap mipmapBmp; - SkImageInfo mipmapInfo = SkImageInfo::MakeN32(2, 2, kOpaque_SkAlphaType, - kSRGB_SkColorProfileType); + SkImageInfo mipmapInfo = SkImageInfo::Make(2, 2, kN32_SkColorType, kOpaque_SkAlphaType, + SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named)); mipmapBmp.allocPixels(mipmapInfo); SkPMColor* mipmapPixels = reinterpret_cast(mipmapBmp.getPixels()); - // 0x89 = 255 * linear_to_srgb(0.25f) - mipmapPixels[0] = mipmapPixels[3] = SkPackARGB32(0xFF, 0x89, 0x89, 0x89); - // 0xE1 = 255 * linear_to_srgb(0.75f) - mipmapPixels[1] = mipmapPixels[2] = SkPackARGB32(0xFF, 0xE1, 0xE1, 0xE1); + unsigned s25 = 0x89; // 255 * linear_to_srgb(0.25f) + unsigned s75 = 0xE1; // 255 * linear_to_srgb(0.75f) + mipmapPixels[0] = mipmapPixels[3] = SkPackARGB32(0xFF, s25, s25, s25); + mipmapPixels[1] = mipmapPixels[2] = SkPackARGB32(0xFF, s75, s75, s75); SkPaint textPaint; textPaint.setColor(SK_ColorWHITE); diff --git a/gm/mipmap.cpp b/gm/mipmap.cpp index ae73a8de9d..45b4d15126 100644 --- a/gm/mipmap.cpp +++ b/gm/mipmap.cpp @@ -47,3 +47,52 @@ DEF_SIMPLE_GM(mipmap, canvas, 400, 200) { } canvas->drawImage(img.get(), 20, 20, nullptr); } + +/////////////////////////////////////////////////////////////////////////////////////////////////// + +// create a circle image computed raw, so we can wrap it as a linear or srgb image +static sk_sp make(sk_sp cs) { + const int N = 100; + SkImageInfo info = SkImageInfo::Make(N, N, kN32_SkColorType, kPremul_SkAlphaType, cs); + SkBitmap bm; + bm.allocPixels(info); + + for (int y = 0; y < N; ++y) { + for (int x = 0; x < N; ++x) { + *bm.getAddr32(x, y) = (x ^ y) & 1 ? 0xFFFFFFFF : 0xFF000000; + } + } + bm.setImmutable(); + return SkImage::MakeFromBitmap(bm); +} + +static void show_mips(SkCanvas* canvas, SkImage* img) { + SkPaint paint; + paint.setFilterQuality(kMedium_SkFilterQuality); + + SkRect dst = SkRect::MakeIWH(img->width(), img->height()); + while (dst.width() > 5) { + canvas->drawImageRect(img, dst, &paint); + dst.offset(dst.width() + 10, 0); + dst.fRight = dst.fLeft + SkScalarHalf(dst.width()); + dst.fBottom = dst.fTop + SkScalarHalf(dst.height()); + } +} + +/* + * Ensure that in L32 drawing mode, both images/mips look the same as each other, and + * their mips are darker than the original (since the mips should ignore the gamma in L32). + * + * Ensure that in S32 drawing mode, all images/mips look the same, and look correct (i.e. + * the mip levels match the original in brightness). + */ +DEF_SIMPLE_GM(mipmap_srgb, canvas, 260, 230) { + sk_sp limg = make(nullptr); + sk_sp simg = make(SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named)); + + canvas->translate(10, 10); + show_mips(canvas, limg.get()); + canvas->translate(0, limg->height() + 10.0f); + show_mips(canvas, simg.get()); +} + diff --git a/gm/showmiplevels.cpp b/gm/showmiplevels.cpp index 717d629ade..9f074344cf 100644 --- a/gm/showmiplevels.cpp +++ b/gm/showmiplevels.cpp @@ -144,7 +144,8 @@ protected: baseBM.lockPixels(); baseBM.peekPixels(&prevPM); - SkAutoTUnref mm(SkMipMap::Build(baseBM, nullptr)); + SkSourceGammaTreatment treatment = SkSourceGammaTreatment::kIgnore; + SkAutoTUnref mm(SkMipMap::Build(baseBM, treatment, nullptr)); int index = 0; SkMipMap::Level level; @@ -252,7 +253,8 @@ protected: SkScalar x = 4; SkScalar y = 4; - SkAutoTUnref mm(SkMipMap::Build(baseBM, nullptr)); + SkSourceGammaTreatment treatment = SkSourceGammaTreatment::kIgnore; + SkAutoTUnref mm(SkMipMap::Build(baseBM, treatment, nullptr)); int index = 0; SkMipMap::Level level; diff --git a/include/private/SkTemplates.h b/include/private/SkTemplates.h index 272b5856ad..7e2c19f09d 100644 --- a/include/private/SkTemplates.h +++ b/include/private/SkTemplates.h @@ -434,6 +434,12 @@ T* SkInPlaceNewCheck(void* storage, size_t size, const A1& a1, const A2& a2, con return (sizeof(T) <= size) ? new (storage) T(a1, a2, a3) : new T(a1, a2, a3); } +template +T* SkInPlaceNewCheck(void* storage, size_t size, + const A1& a1, const A2& a2, const A3& a3, const A4& a4) { + return (sizeof(T) <= size) ? new (storage) T(a1, a2, a3, a4) : new T(a1, a2, a3, a4); +} + /** * Reserves memory that is aligned on double and pointer boundaries. * Hopefully this is sufficient for all practical purposes. diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp index 83eec1b666..153a24748b 100644 --- a/src/core/SkBitmapCache.cpp +++ b/src/core/SkBitmapCache.cpp @@ -230,18 +230,21 @@ static unsigned gMipMapKeyNamespaceLabel; struct MipMapKey : public SkResourceCache::Key { public: - MipMapKey(uint32_t genID, const SkIRect& bounds) : fGenID(genID), fBounds(bounds) { + MipMapKey(uint32_t genID, SkSourceGammaTreatment treatment, const SkIRect& bounds) + : fGenID(genID), fSrcGammaTreatment(static_cast(treatment)), fBounds(bounds) + { this->init(&gMipMapKeyNamespaceLabel, SkMakeResourceCacheSharedIDForBitmap(genID), - sizeof(fGenID) + sizeof(fBounds)); + sizeof(fGenID) + sizeof(fSrcGammaTreatment) + sizeof(fBounds)); } uint32_t fGenID; + uint32_t fSrcGammaTreatment; SkIRect fBounds; }; struct MipMapRec : public SkResourceCache::Rec { - MipMapRec(const SkBitmap& src, const SkMipMap* result) - : fKey(src.getGenerationID(), get_bounds_from_bitmap(src)) + MipMapRec(const SkBitmap& src, SkSourceGammaTreatment treatment, const SkMipMap* result) + : fKey(src.getGenerationID(), treatment, get_bounds_from_bitmap(src)) , fMipMap(result) { fMipMap->attachToCacheAndRef(); @@ -279,9 +282,10 @@ private: } const SkMipMap* SkMipMapCache::FindAndRef(const SkBitmapCacheDesc& desc, + SkSourceGammaTreatment treatment, SkResourceCache* localCache) { // Note: we ignore width/height from desc, just need id and bounds - MipMapKey key(desc.fImageID, desc.fBounds); + MipMapKey key(desc.fImageID, treatment, desc.fBounds); const SkMipMap* result; if (!CHECK_LOCAL(localCache, find, Find, key, MipMapRec::Finder, &result)) { @@ -295,10 +299,11 @@ static SkResourceCache::DiscardableFactory get_fact(SkResourceCache* localCache) : SkResourceCache::GetDiscardableFactory(); } -const SkMipMap* SkMipMapCache::AddAndRef(const SkBitmap& src, SkResourceCache* localCache) { - SkMipMap* mipmap = SkMipMap::Build(src, get_fact(localCache)); +const SkMipMap* SkMipMapCache::AddAndRef(const SkBitmap& src, SkSourceGammaTreatment treatment, + SkResourceCache* localCache) { + SkMipMap* mipmap = SkMipMap::Build(src, treatment, get_fact(localCache)); if (mipmap) { - MipMapRec* rec = new MipMapRec(src, mipmap); + MipMapRec* rec = new MipMapRec(src, treatment, mipmap); CHECK_LOCAL(localCache, add, Add, rec); src.pixelRef()->notifyAddedToCache(); } diff --git a/src/core/SkBitmapCache.h b/src/core/SkBitmapCache.h index 9371e77ca9..76bcef06cf 100644 --- a/src/core/SkBitmapCache.h +++ b/src/core/SkBitmapCache.h @@ -8,12 +8,11 @@ #ifndef SkBitmapCache_DEFINED #define SkBitmapCache_DEFINED -#include "SkScalar.h" #include "SkBitmap.h" +#include "SkMipMap.h" class SkImage; class SkResourceCache; -class SkMipMap; uint64_t SkMakeResourceCacheSharedIDForBitmap(uint32_t bitmapGenID); @@ -73,9 +72,10 @@ public: class SkMipMapCache { public: - static const SkMipMap* FindAndRef(const SkBitmapCacheDesc&, + static const SkMipMap* FindAndRef(const SkBitmapCacheDesc&, SkSourceGammaTreatment, SkResourceCache* localCache = nullptr); - static const SkMipMap* AddAndRef(const SkBitmap& src, SkResourceCache* localCache = nullptr); + static const SkMipMap* AddAndRef(const SkBitmap& src, SkSourceGammaTreatment, + SkResourceCache* localCache = nullptr); }; #endif diff --git a/src/core/SkBitmapController.cpp b/src/core/SkBitmapController.cpp index ac1029db81..f4ee0fb6c1 100644 --- a/src/core/SkBitmapController.cpp +++ b/src/core/SkBitmapController.cpp @@ -44,10 +44,12 @@ SkBitmapController::State* SkBitmapController::requestBitmap(const SkBitmapProvi class SkDefaultBitmapControllerState : public SkBitmapController::State { public: - SkDefaultBitmapControllerState(const SkBitmapProvider&, const SkMatrix& inv, SkFilterQuality); + SkDefaultBitmapControllerState(const SkBitmapProvider&, const SkMatrix& inv, SkFilterQuality, + SkSourceGammaTreatment); private: SkBitmap fResultBitmap; + SkSourceGammaTreatment fSrcGammaTreatment; SkAutoTUnref fCurrMip; bool processHQRequest(const SkBitmapProvider&); @@ -164,13 +166,13 @@ bool SkDefaultBitmapControllerState::processMediumRequest(const SkBitmapProvider } if (invScaleSize.width() > SK_Scalar1 || invScaleSize.height() > SK_Scalar1) { - fCurrMip.reset(SkMipMapCache::FindAndRef(provider.makeCacheDesc())); + fCurrMip.reset(SkMipMapCache::FindAndRef(provider.makeCacheDesc(), fSrcGammaTreatment)); if (nullptr == fCurrMip.get()) { SkBitmap orig; if (!provider.asBitmap(&orig)) { return false; } - fCurrMip.reset(SkMipMapCache::AddAndRef(orig)); + fCurrMip.reset(SkMipMapCache::AddAndRef(orig, fSrcGammaTreatment)); if (nullptr == fCurrMip.get()) { return false; } @@ -200,9 +202,11 @@ bool SkDefaultBitmapControllerState::processMediumRequest(const SkBitmapProvider SkDefaultBitmapControllerState::SkDefaultBitmapControllerState(const SkBitmapProvider& provider, const SkMatrix& inv, - SkFilterQuality qual) { + SkFilterQuality qual, + SkSourceGammaTreatment treatment) { fInvMatrix = inv; fQuality = qual; + fSrcGammaTreatment = treatment; if (this->processHQRequest(provider) || this->processMediumRequest(provider)) { SkASSERT(fResultBitmap.getPixels()); @@ -223,5 +227,6 @@ SkBitmapController::State* SkDefaultBitmapController::onRequestBitmap(const SkBi const SkMatrix& inverse, SkFilterQuality quality, void* storage, size_t size) { - return SkInPlaceNewCheck(storage, size, bm, inverse, quality); + return SkInPlaceNewCheck(storage, size, bm, inverse, quality, + fSrcGammaTreatment); } diff --git a/src/core/SkBitmapController.h b/src/core/SkBitmapController.h index 86b8755ec1..f31c8eef55 100644 --- a/src/core/SkBitmapController.h +++ b/src/core/SkBitmapController.h @@ -53,13 +53,18 @@ protected: /////////////////////////////////////////////////////////////////////////////////////////////////// +#include "SkMipMap.h" + class SkDefaultBitmapController : public SkBitmapController { public: - SkDefaultBitmapController() {} + SkDefaultBitmapController(SkSourceGammaTreatment treatment) : fSrcGammaTreatment(treatment) {} protected: State* onRequestBitmap(const SkBitmapProvider&, const SkMatrix& inverse, SkFilterQuality, void* storage, size_t storageSize) override; + +private: + const SkSourceGammaTreatment fSrcGammaTreatment; }; #endif diff --git a/src/core/SkBitmapProcShader.cpp b/src/core/SkBitmapProcShader.cpp index 972d73173f..c86dfdd989 100644 --- a/src/core/SkBitmapProcShader.cpp +++ b/src/core/SkBitmapProcShader.cpp @@ -229,10 +229,11 @@ SkShader::Context* SkBitmapProcShader::MakeContext(const SkShader& shader, // Decide if we can/want to use the new linear pipeline bool useLinearPipeline = choose_linear_pipeline(rec, provider.info()); + SkSourceGammaTreatment treatment = SkMipMap::DeduceTreatment(rec); if (useLinearPipeline) { void* infoStorage = (char*)storage + sizeof(LinearPipelineContext); - SkBitmapProcInfo* info = new (infoStorage) SkBitmapProcInfo(provider, tmx, tmy); + SkBitmapProcInfo* info = new (infoStorage) SkBitmapProcInfo(provider, tmx, tmy, treatment); if (!info->init(totalInverse, *rec.fPaint)) { info->~SkBitmapProcInfo(); return nullptr; @@ -241,7 +242,8 @@ SkShader::Context* SkBitmapProcShader::MakeContext(const SkShader& shader, return new (storage) LinearPipelineContext(shader, rec, info); } else { void* stateStorage = (char*)storage + sizeof(BitmapProcShaderContext); - SkBitmapProcState* state = new (stateStorage) SkBitmapProcState(provider, tmx, tmy); + SkBitmapProcState* state = new (stateStorage) SkBitmapProcState(provider, tmx, tmy, + treatment); if (!state->setup(totalInverse, *rec.fPaint)) { state->~SkBitmapProcState(); return nullptr; diff --git a/src/core/SkBitmapProcState.cpp b/src/core/SkBitmapProcState.cpp index 33dd8f5194..900b367b5d 100644 --- a/src/core/SkBitmapProcState.cpp +++ b/src/core/SkBitmapProcState.cpp @@ -37,18 +37,22 @@ extern void Clamp_S32_opaque_D32_nofilter_DX_shaderproc(const void*, int, int, u #include "SkBitmapProcState_procs.h" SkBitmapProcInfo::SkBitmapProcInfo(const SkBitmapProvider& provider, - SkShader::TileMode tmx, SkShader::TileMode tmy) + SkShader::TileMode tmx, SkShader::TileMode tmy, + SkSourceGammaTreatment treatment) : fProvider(provider) , fTileModeX(tmx) , fTileModeY(tmy) + , fSrcGammaTreatment(treatment) , fBMState(nullptr) {} SkBitmapProcInfo::SkBitmapProcInfo(const SkBitmap& bm, - SkShader::TileMode tmx, SkShader::TileMode tmy) + SkShader::TileMode tmx, SkShader::TileMode tmy, + SkSourceGammaTreatment treatment) : fProvider(SkBitmapProvider(bm)) , fTileModeX(tmx) , fTileModeY(tmy) + , fSrcGammaTreatment(treatment) , fBMState(nullptr) {} @@ -129,7 +133,7 @@ bool SkBitmapProcInfo::init(const SkMatrix& inv, const SkPaint& paint) { allow_ignore_fractional_translate = false; } - SkDefaultBitmapController controller; + SkDefaultBitmapController controller(fSrcGammaTreatment); fBMState = controller.requestBitmap(fProvider, inv, paint.getFilterQuality(), fBMStateStorage.get(), fBMStateStorage.size()); // Note : we allow the controller to return an empty (zero-dimension) result. Should we? diff --git a/src/core/SkBitmapProcState.h b/src/core/SkBitmapProcState.h index 40dc31a5e0..e2e4f96951 100644 --- a/src/core/SkBitmapProcState.h +++ b/src/core/SkBitmapProcState.h @@ -28,8 +28,10 @@ typedef SkFixed3232 SkFractionalInt; class SkPaint; struct SkBitmapProcInfo { - SkBitmapProcInfo(const SkBitmapProvider&, SkShader::TileMode tmx, SkShader::TileMode tmy); - SkBitmapProcInfo(const SkBitmap&, SkShader::TileMode tmx, SkShader::TileMode tmy); + SkBitmapProcInfo(const SkBitmapProvider&, SkShader::TileMode tmx, SkShader::TileMode tmy, + SkSourceGammaTreatment); + SkBitmapProcInfo(const SkBitmap&, SkShader::TileMode tmx, SkShader::TileMode tmy, + SkSourceGammaTreatment); ~SkBitmapProcInfo(); const SkBitmapProvider fProvider; @@ -43,6 +45,7 @@ struct SkBitmapProcInfo { SkShader::TileMode fTileModeY; SkFilterQuality fFilterQuality; SkMatrix::TypeMask fInvType; + SkSourceGammaTreatment fSrcGammaTreatment; bool init(const SkMatrix& inverse, const SkPaint&); @@ -55,10 +58,12 @@ private: }; struct SkBitmapProcState : public SkBitmapProcInfo { - SkBitmapProcState(const SkBitmapProvider& prov, SkShader::TileMode tmx, SkShader::TileMode tmy) - : SkBitmapProcInfo(prov, tmx, tmy) {} - SkBitmapProcState(const SkBitmap& bitmap, SkShader::TileMode tmx, SkShader::TileMode tmy) - : SkBitmapProcInfo(bitmap, tmx, tmy) {} + SkBitmapProcState(const SkBitmapProvider& prov, SkShader::TileMode tmx, SkShader::TileMode tmy, + SkSourceGammaTreatment treatment) + : SkBitmapProcInfo(prov, tmx, tmy, treatment) {} + SkBitmapProcState(const SkBitmap& bitmap, SkShader::TileMode tmx, SkShader::TileMode tmy, + SkSourceGammaTreatment treatment) + : SkBitmapProcInfo(bitmap, tmx, tmy, treatment) {} bool setup(const SkMatrix& inv, const SkPaint& paint) { return this->init(inv, paint) && this->chooseProcs(); diff --git a/src/core/SkLightingShader.cpp b/src/core/SkLightingShader.cpp index a2ce52fe28..aaf29fcf3a 100644 --- a/src/core/SkLightingShader.cpp +++ b/src/core/SkLightingShader.cpp @@ -677,7 +677,8 @@ SkShader::Context* SkLightingShaderImpl::onCreateContext(const ContextRec& rec, void* diffuseStateStorage = (char*)storage + sizeof(LightingShaderContext); SkBitmapProcState* diffuseState = new (diffuseStateStorage) SkBitmapProcState(fDiffuseMap, - SkShader::kClamp_TileMode, SkShader::kClamp_TileMode); + SkShader::kClamp_TileMode, SkShader::kClamp_TileMode, + SkMipMap::DeduceTreatment(rec)); SkASSERT(diffuseState); if (!diffuseState->setup(diffTotalInv, *rec.fPaint)) { diffuseState->~SkBitmapProcState(); @@ -686,7 +687,8 @@ SkShader::Context* SkLightingShaderImpl::onCreateContext(const ContextRec& rec, void* normalStateStorage = (char*)storage + sizeof(LightingShaderContext) + sizeof(SkBitmapProcState); SkBitmapProcState* normalState = new (normalStateStorage) SkBitmapProcState(fNormalMap, - SkShader::kClamp_TileMode, SkShader::kClamp_TileMode); + SkShader::kClamp_TileMode, SkShader::kClamp_TileMode, + SkMipMap::DeduceTreatment(rec)); SkASSERT(normalState); if (!normalState->setup(normTotalInv, *rec.fPaint)) { diffuseState->~SkBitmapProcState(); diff --git a/src/core/SkMipMap.cpp b/src/core/SkMipMap.cpp index 355c1edebb..69c8466f69 100644 --- a/src/core/SkMipMap.cpp +++ b/src/core/SkMipMap.cpp @@ -11,6 +11,7 @@ #include "SkHalf.h" #include "SkMathPriv.h" #include "SkNx.h" +#include "SkPM4fPriv.h" #include "SkTypes.h" // @@ -41,6 +42,16 @@ struct ColorTypeFilter_8888 { #endif }; +struct ColorTypeFilter_S32 { + typedef uint32_t Type; + static Sk4f Expand(uint32_t x) { + return Sk4f_fromS32(x); + } + static uint32_t Compact(const Sk4f& x) { + return Sk4f_toS32(x); + } +}; + struct ColorTypeFilter_565 { typedef uint16_t Type; static uint32_t Expand(uint16_t x) { @@ -293,7 +304,16 @@ size_t SkMipMap::AllocLevelsSize(int levelCount, size_t pixelSize) { return sk_64_asS32(size); } -SkMipMap* SkMipMap::Build(const SkPixmap& src, SkDiscardableFactoryProc fact) { +static bool treat_like_srgb(const SkImageInfo& info) { + if (info.colorSpace()) { + return SkColorSpace::k2Dot2Curve_GammaNamed == info.colorSpace()->gammaNamed(); + } else { + return kSRGB_SkColorProfileType == info.profileType(); + } +} + +SkMipMap* SkMipMap::Build(const SkPixmap& src, SkSourceGammaTreatment treatment, + SkDiscardableFactoryProc fact) { typedef void FilterProc(void*, const void* srcPtr, size_t srcRB, int count); FilterProc* proc_1_2 = nullptr; @@ -307,17 +327,31 @@ SkMipMap* SkMipMap::Build(const SkPixmap& src, SkDiscardableFactoryProc fact) { const SkColorType ct = src.colorType(); const SkAlphaType at = src.alphaType(); + const bool srgbGamma = (SkSourceGammaTreatment::kRespect == treatment) + && treat_like_srgb(src.info()); + switch (ct) { case kRGBA_8888_SkColorType: case kBGRA_8888_SkColorType: - proc_1_2 = downsample_1_2; - proc_1_3 = downsample_1_3; - proc_2_1 = downsample_2_1; - proc_2_2 = downsample_2_2; - proc_2_3 = downsample_2_3; - proc_3_1 = downsample_3_1; - proc_3_2 = downsample_3_2; - proc_3_3 = downsample_3_3; + if (srgbGamma) { + proc_1_2 = downsample_1_2; + proc_1_3 = downsample_1_3; + proc_2_1 = downsample_2_1; + proc_2_2 = downsample_2_2; + proc_2_3 = downsample_2_3; + proc_3_1 = downsample_3_1; + proc_3_2 = downsample_3_2; + proc_3_3 = downsample_3_3; + } else { + proc_1_2 = downsample_1_2; + proc_1_3 = downsample_1_3; + proc_2_1 = downsample_2_1; + proc_2_2 = downsample_2_2; + proc_2_3 = downsample_2_3; + proc_3_1 = downsample_3_1; + proc_3_2 = downsample_3_2; + proc_3_3 = downsample_3_3; + } break; case kRGB_565_SkColorType: proc_1_2 = downsample_1_2; @@ -394,8 +428,10 @@ SkMipMap* SkMipMap::Build(const SkPixmap& src, SkDiscardableFactoryProc fact) { } // init + mipmap->fCS = sk_ref_sp(src.info().colorSpace()); mipmap->fCount = countLevels; mipmap->fLevels = (Level*)mipmap->writable_data(); + SkASSERT(mipmap->fLevels); Level* levels = mipmap->fLevels; uint8_t* baseAddr = (uint8_t*)&levels[countLevels]; @@ -440,6 +476,9 @@ SkMipMap* SkMipMap::Build(const SkPixmap& src, SkDiscardableFactoryProc fact) { height = SkTMax(1, height >> 1); rowBytes = SkToU32(SkColorTypeMinRowBytes(ct, width)); + // We make the Info w/o any colorspace, since that storage is not under our control, and + // will not be deleted in a controlled fashion. When the caller is given the pixmap for + // a given level, we augment this pixmap with fCS (which we do manage). new (&levels[i].fPixmap) SkPixmap(SkImageInfo::Make(width, height, ct, at), addr, rowBytes); levels[i].fScale = SkSize::Make(SkIntToScalar(width) / src.width(), SkIntToScalar(height) / src.height()); @@ -459,6 +498,7 @@ SkMipMap* SkMipMap::Build(const SkPixmap& src, SkDiscardableFactoryProc fact) { } SkASSERT(addr == baseAddr + size); + SkASSERT(mipmap->fLevels); return mipmap; } @@ -547,9 +587,7 @@ bool SkMipMap::extractLevel(const SkSize& scaleSize, Level* levelPtr) const { return false; } SkASSERT(L >= 0); -// int rndLevel = SkScalarRoundToInt(L); int level = SkScalarFloorToInt(L); -// SkDebugf("mipmap scale=%g L=%g level=%d rndLevel=%d\n", scale, L, level, rndLevel); SkASSERT(level >= 0); if (level <= 0) { @@ -561,13 +599,16 @@ bool SkMipMap::extractLevel(const SkSize& scaleSize, Level* levelPtr) const { } if (levelPtr) { *levelPtr = fLevels[level - 1]; + // need to augment with our colorspace + levelPtr->fPixmap.setColorSpace(fCS); } return true; } // Helper which extracts a pixmap from the src bitmap // -SkMipMap* SkMipMap::Build(const SkBitmap& src, SkDiscardableFactoryProc fact) { +SkMipMap* SkMipMap::Build(const SkBitmap& src, SkSourceGammaTreatment treatment, + SkDiscardableFactoryProc fact) { SkAutoPixmapUnlock srcUnlocker; if (!src.requestLock(&srcUnlocker)) { return nullptr; @@ -577,7 +618,7 @@ SkMipMap* SkMipMap::Build(const SkBitmap& src, SkDiscardableFactoryProc fact) { if (nullptr == srcPixmap.addr()) { sk_throw(); } - return Build(srcPixmap, fact); + return Build(srcPixmap, treatment, fact); } int SkMipMap::countLevels() const { diff --git a/src/core/SkMipMap.h b/src/core/SkMipMap.h index 7c798ef833..0f31a9f703 100644 --- a/src/core/SkMipMap.h +++ b/src/core/SkMipMap.h @@ -12,6 +12,7 @@ #include "SkPixmap.h" #include "SkScalar.h" #include "SkSize.h" +#include "SkShader.h" class SkBitmap; class SkDiscardableMemory; @@ -27,8 +28,13 @@ typedef SkDiscardableMemory* (*SkDiscardableFactoryProc)(size_t bytes); */ class SkMipMap : public SkCachedData { public: - static SkMipMap* Build(const SkPixmap& src, SkDiscardableFactoryProc); - static SkMipMap* Build(const SkBitmap& src, SkDiscardableFactoryProc); + static SkMipMap* Build(const SkPixmap& src, SkSourceGammaTreatment, SkDiscardableFactoryProc); + static SkMipMap* Build(const SkBitmap& src, SkSourceGammaTreatment, SkDiscardableFactoryProc); + + static SkSourceGammaTreatment DeduceTreatment(const SkShader::ContextRec& rec) { + return (SkShader::ContextRec::kPMColor_DstType == rec.fPreferredDstType) ? + SkSourceGammaTreatment::kIgnore : SkSourceGammaTreatment::kRespect; + } // Determines how many levels a SkMipMap will have without creating that mipmap. // This does not include the base mipmap level that the user provided when @@ -61,10 +67,10 @@ protected: } private: - Level* fLevels; - int fCount; + sk_sp fCS; + Level* fLevels; // managed by the baseclass, may be null due to onDataChanged. + int fCount; - // we take ownership of levels, and will free it with sk_free() SkMipMap(void* malloc, size_t size) : INHERITED(malloc, size) {} SkMipMap(size_t size, SkDiscardableMemory* dm) : INHERITED(size, dm) {} diff --git a/src/core/SkPM4fPriv.h b/src/core/SkPM4fPriv.h index bf5ff5a1ad..12bf7d0f36 100644 --- a/src/core/SkPM4fPriv.h +++ b/src/core/SkPM4fPriv.h @@ -52,6 +52,48 @@ static inline float linear_to_srgb(float x) { return sqrtf(x); } +static void assert_unit(float x) { + SkASSERT(x >= 0 && x <= 1); +} + +static inline float exact_srgb_to_linear(float x) { + assert_unit(x); + float linear; + if (x <= 0.04045) { + linear = x / 12.92f; + } else { + linear = powf((x + 0.055f) / 1.055f, 2.4f); + } + assert_unit(linear); + return linear; +} + +static inline float exact_linear_to_srgb(float x) { + assert_unit(x); + float srgb; + if (x <= 0.0031308f) { + srgb = x * 12.92f; + } else { + srgb = 1.055f * powf(x, 0.41666667f) - 0.055f; + } + assert_unit(srgb); + return srgb; +} + +static inline Sk4f exact_srgb_to_linear(const Sk4f& x) { + Sk4f linear(exact_srgb_to_linear(x[0]), + exact_srgb_to_linear(x[1]), + exact_srgb_to_linear(x[2]), 1); + return set_alpha(linear, get_alpha(x)); +} + +static inline Sk4f exact_linear_to_srgb(const Sk4f& x) { + Sk4f srgb(exact_linear_to_srgb(x[0]), + exact_linear_to_srgb(x[1]), + exact_linear_to_srgb(x[2]), 1); + return set_alpha(srgb, get_alpha(x)); +} + /////////////////////////////////////////////////////////////////////////////////////////////////// static inline Sk4f Sk4f_fromL32(uint32_t src) { @@ -77,4 +119,11 @@ static inline uint32_t Sk4f_toS32(const Sk4f& x4) { return to_4b(linear_to_srgb(x4) * Sk4f(255) + Sk4f(0.5f)); } +static inline Sk4f exact_Sk4f_fromS32(uint32_t src) { + return exact_srgb_to_linear(to_4f(src) * Sk4f(1.0f/255)); +} +static inline uint32_t exact_Sk4f_toS32(const Sk4f& x4) { + return to_4b(exact_linear_to_srgb(x4) * Sk4f(255) + Sk4f(0.5f)); +} + #endif diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp index ca7d6b8e84..cf50f6004e 100644 --- a/src/gpu/SkGr.cpp +++ b/src/gpu/SkGr.cpp @@ -345,12 +345,6 @@ GrTexture* GrGenerateMipMapsAndUploadToTexture(GrContext* ctx, const SkBitmap& b return texture.release(); } - // SkMipMap::Build doesn't handle sRGB data correctly (yet). - // Failover to the GL code-path for now. - if (kLinear_SkColorProfileType != bitmap.profileType()) { - return nullptr; - } - SkASSERT(sizeof(int) <= sizeof(uint32_t)); if (bitmap.width() < 0 || bitmap.height() < 0) { return nullptr; @@ -366,7 +360,7 @@ GrTexture* GrGenerateMipMapsAndUploadToTexture(GrContext* ctx, const SkBitmap& b sk_throw(); } - SkAutoTDelete mipmaps(SkMipMap::Build(pixmap, nullptr)); + SkAutoTDelete mipmaps(SkMipMap::Build(pixmap, gammaTreatment, nullptr)); if (!mipmaps) { return nullptr; } diff --git a/tests/MipMapTest.cpp b/tests/MipMapTest.cpp index 596e995868..2c0b61d596 100644 --- a/tests/MipMapTest.cpp +++ b/tests/MipMapTest.cpp @@ -23,7 +23,7 @@ DEF_TEST(MipMap, reporter) { int width = 1 + rand.nextU() % 1000; int height = 1 + rand.nextU() % 1000; make_bitmap(&bm, width, height); - SkAutoTUnref mm(SkMipMap::Build(bm, nullptr)); + SkAutoTUnref mm(SkMipMap::Build(bm, SkSourceGammaTreatment::kIgnore, nullptr)); REPORTER_ASSERT(reporter, mm->countLevels() == SkMipMap::ComputeLevelCount(width, height)); REPORTER_ASSERT(reporter, !mm->extractLevel(SkSize::Make(SK_Scalar1, SK_Scalar1), @@ -60,7 +60,7 @@ static void test_mipmap_generation(int width, int height, int expectedMipLevelCo SkBitmap bm; bm.allocN32Pixels(width, height); bm.eraseColor(SK_ColorWHITE); - SkAutoTUnref mm(SkMipMap::Build(bm, nullptr)); + SkAutoTUnref mm(SkMipMap::Build(bm, SkSourceGammaTreatment::kIgnore, nullptr)); const int mipLevelCount = mm->countLevels(); REPORTER_ASSERT(reporter, mipLevelCount == expectedMipLevelCount); @@ -90,7 +90,7 @@ DEF_TEST(MipMap_DirectLevelAccess, reporter) { SkBitmap bm; bm.allocN32Pixels(1, 1); bm.eraseColor(SK_ColorWHITE); - SkAutoTUnref mm(SkMipMap::Build(bm, nullptr)); + SkAutoTUnref mm(SkMipMap::Build(bm, SkSourceGammaTreatment::kIgnore, nullptr)); REPORTER_ASSERT(reporter, mm == nullptr); } diff --git a/tests/SkResourceCacheTest.cpp b/tests/SkResourceCacheTest.cpp index 3c0d1ffee3..f363fb7a44 100644 --- a/tests/SkResourceCacheTest.cpp +++ b/tests/SkResourceCacheTest.cpp @@ -96,14 +96,18 @@ static void test_mipmapcache(skiatest::Reporter* reporter, SkResourceCache* cach src.allocN32Pixels(5, 5); src.setImmutable(); - const SkMipMap* mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), cache); + const SkSourceGammaTreatment treatment = SkSourceGammaTreatment::kIgnore; + + const SkMipMap* mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), treatment, + cache); REPORTER_ASSERT(reporter, nullptr == mipmap); - mipmap = SkMipMapCache::AddAndRef(src, cache); + mipmap = SkMipMapCache::AddAndRef(src, treatment, cache); REPORTER_ASSERT(reporter, mipmap); { - const SkMipMap* mm = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), cache); + const SkMipMap* mm = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), treatment, + cache); REPORTER_ASSERT(reporter, mm); REPORTER_ASSERT(reporter, mm == mipmap); mm->unref(); @@ -117,7 +121,7 @@ static void test_mipmapcache(skiatest::Reporter* reporter, SkResourceCache* cach check_data(reporter, mipmap, 1, kInCache, kNotLocked); // find us again - mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), cache); + mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src), treatment, cache); check_data(reporter, mipmap, 2, kInCache, kLocked); cache->purgeAll(); @@ -127,16 +131,19 @@ static void test_mipmapcache(skiatest::Reporter* reporter, SkResourceCache* cach } static void test_mipmap_notify(skiatest::Reporter* reporter, SkResourceCache* cache) { + const SkSourceGammaTreatment treatment = SkSourceGammaTreatment::kIgnore; const int N = 3; + SkBitmap src[N]; for (int i = 0; i < N; ++i) { src[i].allocN32Pixels(5, 5); src[i].setImmutable(); - SkMipMapCache::AddAndRef(src[i], cache)->unref(); + SkMipMapCache::AddAndRef(src[i], treatment, cache)->unref(); } for (int i = 0; i < N; ++i) { - const SkMipMap* mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src[i]), cache); + const SkMipMap* mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src[i]), + treatment, cache); if (cache) { // if cache is null, we're working on the global cache, and other threads might purge // it, making this check fragile. @@ -146,7 +153,7 @@ static void test_mipmap_notify(skiatest::Reporter* reporter, SkResourceCache* ca src[i].reset(); // delete the underlying pixelref, which *should* remove us from the cache - mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src[i]), cache); + mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src[i]), treatment, cache); REPORTER_ASSERT(reporter, !mipmap); } }