From e28a6b55dfbcd510d86cca39a167d179b81e4f4c Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Wed, 25 Jul 2018 13:05:17 -0400 Subject: add explicit accessor for sRGB singleton colorspaces SkColorSpace::MakeSRGB().get() is scary, and causes more ref/unref pairs than strictly necessary for these singletons. This time the implementation is still in SkColorSpace.cpp, so these should really work as singletons. Change-Id: I40f2942c8dcde3040663a04c4f5330aca90868ae Reviewed-on: https://skia-review.googlesource.com/143305 Auto-Submit: Mike Klein Commit-Queue: Brian Osman Reviewed-by: Brian Osman --- src/core/SkBlitter_Sprite.cpp | 3 ++- src/core/SkColorSpace.cpp | 14 +++++++------- src/core/SkColorSpacePriv.h | 6 ++++++ src/core/SkColorSpaceXformer.cpp | 3 ++- src/core/SkPM4fPriv.h | 5 +++-- src/core/SkRasterPipelineBlitter.cpp | 3 ++- src/effects/SkToSRGBColorFilter.cpp | 5 +++-- src/gpu/GrColorSpaceXform.cpp | 8 ++++---- src/shaders/SkImageShader.cpp | 3 ++- src/shaders/gradients/SkGradientShader.cpp | 9 +++++---- src/utils/SkPatchUtils.cpp | 13 +++++++------ 11 files changed, 43 insertions(+), 29 deletions(-) (limited to 'src') diff --git a/src/core/SkBlitter_Sprite.cpp b/src/core/SkBlitter_Sprite.cpp index b26b712d93..e0998d9608 100644 --- a/src/core/SkBlitter_Sprite.cpp +++ b/src/core/SkBlitter_Sprite.cpp @@ -7,6 +7,7 @@ #include "SkArenaAlloc.h" #include "SkColorSpace.h" +#include "SkColorSpacePriv.h" #include "SkColorSpaceXformSteps.h" #include "SkCoreBlitters.h" #include "SkOpts.h" @@ -143,7 +144,7 @@ public: if (!srcCS || fSource.colorType() == kAlpha_8_SkColorType) { // We treat untagged images as sRGB. // A8 images get their r,g,b from the paint color, so they're also sRGB. - srcCS = SkColorSpace::MakeSRGB().get(); + srcCS = sk_srgb_singleton(); } fAlloc->make(srcCS, kPremul_SkAlphaType, dstCS) ->apply(&p); diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp index 3b08e29a89..239f98aac8 100644 --- a/src/core/SkColorSpace.cpp +++ b/src/core/SkColorSpace.cpp @@ -192,21 +192,21 @@ static SkColorSpace* singleton_colorspace(SkGammaNamed gamma, const float to_xyz return new SkColorSpace_XYZ(gamma, m44); } -static SkColorSpace* srgb() { +SkColorSpace* sk_srgb_singleton() { static SkColorSpace* cs = singleton_colorspace(kSRGB_SkGammaNamed, gSRGB_toXYZD50); return cs; } -static SkColorSpace* srgb_linear() { +SkColorSpace* sk_srgb_linear_singleton() { static SkColorSpace* cs = singleton_colorspace(kLinear_SkGammaNamed, gSRGB_toXYZD50); return cs; } sk_sp SkColorSpace::MakeSRGB() { - return sk_ref_sp(srgb()); + return sk_ref_sp(sk_srgb_singleton()); } sk_sp SkColorSpace::MakeSRGBLinear() { - return sk_ref_sp(srgb_linear()); + return sk_ref_sp(sk_srgb_linear_singleton()); } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -258,7 +258,7 @@ uint32_t SkColorSpace::toXYZD50Hash() const { } bool SkColorSpace::isSRGB() const { - return srgb() == this; + return sk_srgb_singleton() == this; } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -331,13 +331,13 @@ size_t SkColorSpace::writeToMemory(void* memory) const { SkASSERT(this->toXYZD50()); // If we have a named profile, only write the enum. const SkGammaNamed gammaNamed = this->gammaNamed(); - if (this == srgb()) { + if (this == sk_srgb_singleton()) { if (memory) { *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack( k0_Version, kSRGB_NamedColorSpace, gammaNamed, 0); } return sizeof(ColorSpaceHeader); - } else if (this == srgb_linear()) { + } else if (this == sk_srgb_linear_singleton()) { if (memory) { *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack( k0_Version, kSRGBLinear_NamedColorSpace, gammaNamed, 0); diff --git a/src/core/SkColorSpacePriv.h b/src/core/SkColorSpacePriv.h index 7742a8770a..278a6a47d2 100644 --- a/src/core/SkColorSpacePriv.h +++ b/src/core/SkColorSpacePriv.h @@ -216,4 +216,10 @@ static inline bool named_to_parametric(SkColorSpaceTransferFn* coeffs, return false; } } + +// Return raw pointers to commonly used SkColorSpaces. +// No need to ref/unref these, but if you do, do it in pairs. +SkColorSpace* sk_srgb_singleton(); +SkColorSpace* sk_srgb_linear_singleton(); + #endif // SkColorSpacePriv_DEFINED diff --git a/src/core/SkColorSpaceXformer.cpp b/src/core/SkColorSpaceXformer.cpp index 5329294982..1e56d4da93 100644 --- a/src/core/SkColorSpaceXformer.cpp +++ b/src/core/SkColorSpaceXformer.cpp @@ -6,6 +6,7 @@ */ #include "SkColorFilter.h" +#include "SkColorSpacePriv.h" #include "SkColorSpaceXformer.h" #include "SkColorSpaceXformPriv.h" #include "SkDrawLooper.h" @@ -26,7 +27,7 @@ SkColorSpaceXformer::~SkColorSpaceXformer() {} std::unique_ptr SkColorSpaceXformer::Make(sk_sp dst) { std::unique_ptr fromSRGB = SkMakeColorSpaceXform( - SkColorSpace::MakeSRGB().get(), dst.get()); + sk_srgb_singleton(), dst.get()); return fromSRGB ? std::unique_ptr(new SkColorSpaceXformer(std::move(dst), diff --git a/src/core/SkPM4fPriv.h b/src/core/SkPM4fPriv.h index f5e9aa02e6..f7daabf7dc 100644 --- a/src/core/SkPM4fPriv.h +++ b/src/core/SkPM4fPriv.h @@ -10,6 +10,7 @@ #include "SkColorData.h" #include "SkColorSpace.h" +#include "SkColorSpacePriv.h" #include "SkColorSpaceXformSteps.h" #include "SkArenaAlloc.h" #include "SkPM4f.h" @@ -40,7 +41,7 @@ static inline uint32_t Sk4f_toL32(const Sk4f& px) { static inline SkPM4f premul_in_dst_colorspace(SkColor4f color4f, SkColorSpace* srcCS, SkColorSpace* dstCS) { // We treat untagged sources as sRGB. - if (!srcCS) { srcCS = SkColorSpace::MakeSRGB().get(); } + if (!srcCS) { srcCS = sk_srgb_singleton(); } // If dstCS is null, no color space transformation is needed (and apply() will just premul). if (!dstCS) { dstCS = srcCS; } @@ -58,7 +59,7 @@ static inline SkPM4f premul_in_dst_colorspace(SkColor c, SkColorSpace* dstCS) { swizzle_rb(Sk4f_fromL32(c)).store(color4f.vec()); // SkColors are always sRGB. - return premul_in_dst_colorspace(color4f, SkColorSpace::MakeSRGB().get(), dstCS); + return premul_in_dst_colorspace(color4f, sk_srgb_singleton(), dstCS); } // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! diff --git a/src/core/SkRasterPipelineBlitter.cpp b/src/core/SkRasterPipelineBlitter.cpp index ab6990ad1a..58497e07c6 100644 --- a/src/core/SkRasterPipelineBlitter.cpp +++ b/src/core/SkRasterPipelineBlitter.cpp @@ -11,6 +11,7 @@ #include "SkBlitter.h" #include "SkColor.h" #include "SkColorFilter.h" +#include "SkColorSpacePriv.h" #include "SkColorSpaceXformer.h" #include "SkColorSpaceXformSteps.h" #include "SkOpts.h" @@ -94,7 +95,7 @@ SkBlitter* SkCreateRasterPipelineBlitter(const SkPixmap& dst, // we need to sometimes still need to distinguish null dstCS from sRGB. #if 0 SkColorSpace* dstCS = dst.colorSpace() ? dst.colorSpace() - : SkColorSpace::MakeSRGB().get(); + : sk_srgb_singleton(); #else SkColorSpace* dstCS = dst.colorSpace(); #endif diff --git a/src/effects/SkToSRGBColorFilter.cpp b/src/effects/SkToSRGBColorFilter.cpp index 0bfa40b1d3..9942b02f37 100644 --- a/src/effects/SkToSRGBColorFilter.cpp +++ b/src/effects/SkToSRGBColorFilter.cpp @@ -5,6 +5,7 @@ * found in the LICENSE file. */ +#include "SkColorSpacePriv.h" #include "SkColorSpaceXformSteps.h" #include "SkPM4fPriv.h" #include "SkRasterPipeline.h" @@ -22,7 +23,7 @@ void SkToSRGBColorFilter::onAppendStages(SkRasterPipeline* p, SkArenaAlloc* alloc, bool shaderIsOpaque) const { alloc->make(fSrcColorSpace.get(), kPremul_SkAlphaType, - SkColorSpace::MakeSRGB().get()) + sk_srgb_singleton()) ->apply(p); } @@ -51,6 +52,6 @@ void SkToSRGBColorFilter::flatten(SkWriteBuffer& buffer) const { #if SK_SUPPORT_GPU std::unique_ptr SkToSRGBColorFilter::asFragmentProcessor( GrContext*, const GrColorSpaceInfo&) const { - return GrColorSpaceXformEffect::Make(fSrcColorSpace.get(), SkColorSpace::MakeSRGB().get()); + return GrColorSpaceXformEffect::Make(fSrcColorSpace.get(), sk_srgb_singleton()); } #endif diff --git a/src/gpu/GrColorSpaceXform.cpp b/src/gpu/GrColorSpaceXform.cpp index 4ccca52185..af822a882d 100644 --- a/src/gpu/GrColorSpaceXform.cpp +++ b/src/gpu/GrColorSpaceXform.cpp @@ -19,9 +19,9 @@ sk_sp GrColorSpaceXform::Make(SkColorSpace* src, SkColorSpace return nullptr; } - // Treat null sources as sRGB (safe because sRGB is a global singleton) + // Treat null sources as sRGB. if (!src) { - src = SkColorSpace::MakeSRGB().get(); + src = sk_srgb_singleton(); } // TODO: Plumb source alpha type @@ -38,9 +38,9 @@ sk_sp GrColorSpaceXform::MakeUnpremulToUnpremul(SkColorSpace* return nullptr; } - // Treat null sources as sRGB (safe because sRGB is a global singleton) + // Treat null sources as sRGB. if (!src) { - src = SkColorSpace::MakeSRGB().get(); + src = sk_srgb_singleton(); } SkColorSpaceXformSteps steps = SkColorSpaceXformSteps::UnpremulToUnpremul(src, dst); diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp index 938b7d6da8..9ccd5cd4cf 100644 --- a/src/shaders/SkImageShader.cpp +++ b/src/shaders/SkImageShader.cpp @@ -9,6 +9,7 @@ #include "SkBitmapController.h" #include "SkBitmapProcShader.h" #include "SkBitmapProvider.h" +#include "SkColorSpacePriv.h" #include "SkColorSpaceXformSteps.h" #include "SkEmptyShader.h" #include "SkImage_Base.h" @@ -402,7 +403,7 @@ bool SkImageShader::onAppendStages(const StageRec& rec) const { if (!srcCS || info.colorType() == kAlpha_8_SkColorType) { // We treat untagged images as sRGB. // A8 images get their r,g,b from the paint color, so they're also sRGB. - srcCS = SkColorSpace::MakeSRGB().get(); + srcCS = sk_srgb_singleton(); } alloc->make(srcCS, kPremul_SkAlphaType, rec.fDstCS) ->apply(p); diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp index ff22cc598a..8536dd82eb 100644 --- a/src/shaders/gradients/SkGradientShader.cpp +++ b/src/shaders/gradients/SkGradientShader.cpp @@ -8,6 +8,7 @@ #include #include "Sk4fLinearGradient.h" #include "SkColorSpace_XYZ.h" +#include "SkColorSpacePriv.h" #include "SkColorSpaceXformer.h" #include "SkFlattenablePriv.h" #include "SkFloatBits.h" @@ -462,14 +463,14 @@ SkColor4fXformer::SkColor4fXformer(const SkColor4f* colors, int colorCount, // Transform all of the colors to destination color space fColors = colors; - // Treat null destinations as sRGB (safe because sRGB is a global singleton) + // Treat null destinations as sRGB. if (!dst) { - dst = SkColorSpace::MakeSRGB().get(); + dst = sk_srgb_singleton(); } - // Treat null sources as sRGB (safe because sRGB is a global singleton) + // Treat null sources as sRGB. if (!src) { - src = SkColorSpace::MakeSRGB().get(); + src = sk_srgb_singleton(); } if (!SkColorSpace::Equals(src, dst)) { diff --git a/src/utils/SkPatchUtils.cpp b/src/utils/SkPatchUtils.cpp index af91626b51..badd63ff15 100644 --- a/src/utils/SkPatchUtils.cpp +++ b/src/utils/SkPatchUtils.cpp @@ -8,6 +8,7 @@ #include "SkPatchUtils.h" #include "SkColorData.h" +#include "SkColorSpacePriv.h" #include "SkGeometry.h" #include "SkPM4f.h" #include "SkTo.h" @@ -254,8 +255,8 @@ struct SkRGBAf { static void skcolor_to_float(SkRGBAf dst[], const SkColor src[], int count, SkColorSpace* dstCS, bool doPremul) { - // Source is always sRGB SkColor (safe because sRGB is a global singleton) - auto srcCS = SkColorSpace::MakeSRGB().get(); + // Source is always sRGB SkColor. + auto srcCS = sk_srgb_singleton(); auto op = doPremul ? SkColorSpaceXform::kPremul_AlphaOp : SkColorSpaceXform::kPreserve_AlphaOp; SkAssertResult(SkColorSpaceXform::Apply(dstCS, SkColorSpaceXform::kRGBA_F32_ColorFormat, dst, @@ -264,8 +265,8 @@ static void skcolor_to_float(SkRGBAf dst[], const SkColor src[], int count, SkCo } static void float_to_skcolor(SkColor dst[], const SkRGBAf src[], int count, SkColorSpace* srcCS) { - // Destination is always sRGB SkColor (safe because sRGB is a global singleton) - auto dstCS = SkColorSpace::MakeSRGB().get(); + // Destination is always sRGB SkColor. + auto dstCS = sk_srgb_singleton(); SkAssertResult(SkColorSpaceXform::Apply(dstCS, SkColorSpaceXform::kBGRA_8888_ColorFormat, dst, srcCS, SkColorSpaceXform::kRGBA_F32_ColorFormat, src, count, SkColorSpaceXform::kPreserve_AlphaOp)); @@ -292,9 +293,9 @@ sk_sp SkPatchUtils::MakeVertices(const SkPoint cubics[12], const SkC return nullptr; } - // Treat null interpolation space as sRGB (safe because sRGB is a global singleton) + // Treat null interpolation space as sRGB. if (!colorSpace) { - colorSpace = SkColorSpace::MakeSRGB().get(); + colorSpace = sk_srgb_singleton(); } int vertexCount = SkToS32(mult64); -- cgit v1.2.3