AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "replace SkColorSpaceXformSteps::Required()"

Browse Source 
This reverts commit 122eda7877.

Reason for revert: breaking up into parts to help diagnose a possible performance regression.

Original change's description:
> replace SkColorSpaceXformSteps::Required()
> 
> It's error-prone to ask if a color space transform is required without
> explicitly mentioning alpha types.  I think the whole method can be
> replaced by a friendlier constructor and direct use of flags/mask:
> 
>     SkColorSpaceXformSteps::Required(src.colorSpace(), dst.colorSpace())
> 
>         ~~~>
> 
>     0 != SkColorSpaceXformSteps(src, dst).flags.mask()
> 
> This removes the unexpected gray square from the fiddle in skia:9662.
> 
> Bug: skia:9662
> Change-Id: Id54cae534023ab29c94dcc149e6b67fc9c166665
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/257399
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>

TBR=mtklein@google.com,brianosman@google.com

Change-Id: I4073f45c52e77f02fe3ecb75c20fc29873080c20
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:9662
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/257652
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
Mike Klein 2019-12-03 16:52:34 +00:00 committed by Skia Commit-Bot
parent 042fe0847b
commit b6b7e7b6c8
5 changed files with 30 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/core/SkBlitter_Sprite.cpp
View File

@ -56,7 +56,8 @@ void SkSpriteBlitter::blitMask(const SkMask& mask, const SkIRect& clip) {
class SkSpriteBlitter_Memcpy final : public SkSpriteBlitter {
public:
static bool Supports(const SkPixmap& dst, const SkPixmap& src, const SkPaint& paint) {
SkASSERT(0 == SkColorSpaceXformSteps(src, dst).flags.mask());
// the caller has already inspected the colorspace on src and dst
SkASSERT(!SkColorSpaceXformSteps::Required(src.colorSpace(), dst.colorSpace()));
if (dst.colorType() != src.colorType()) {
return false;
@ -177,14 +178,13 @@ SkBlitter* SkBlitter::ChooseSprite(const SkPixmap& dst, const SkPaint& paint,
*/
SkASSERT(allocator != nullptr);
// TODO: in principle SkRasterPipelineSpriteBlitter could be made to handle this.
if (source.alphaType() == kUnpremul_SkAlphaType) {
return nullptr;
}
SkSpriteBlitter* blitter = nullptr;
if (0 == SkColorSpaceXformSteps(source,dst).flags.mask()) {
if (!SkColorSpaceXformSteps::Required(source.colorSpace(), dst.colorSpace())) {
if (!blitter && SkSpriteBlitter_Memcpy::Supports(dst, source, paint)) {
blitter = allocator->make<SkSpriteBlitter_Memcpy>(source);
}

10
src/core/SkColorSpaceXformSteps.cpp
View File

@ -10,6 +10,16 @@
#include "src/core/SkColorSpaceXformSteps.h"
#include "src/core/SkRasterPipeline.h"
// TODO(mtklein): explain the logic of this file
bool SkColorSpaceXformSteps::Required(SkColorSpace* src, SkColorSpace* dst) {
// Any SkAlphaType will work fine here as long as we use the same one.
SkAlphaType at = kPremul_SkAlphaType;
return 0 != SkColorSpaceXformSteps(src, at,
dst, at).flags.mask();
// TODO(mtklein): quicker impl. that doesn't construct an SkColorSpaceXformSteps?
}
SkColorSpaceXformSteps::SkColorSpaceXformSteps(SkColorSpace* src, SkAlphaType srcAT,
SkColorSpace* dst, SkAlphaType dstAT) {
// Opaque outputs are treated as the same alpha type as the source input.

8
src/core/SkColorSpaceXformSteps.h
View File

@ -15,6 +15,9 @@
class SkRasterPipeline;
struct SkColorSpaceXformSteps {
// Returns true if SkColorSpaceXformSteps must be applied
// to draw content in `src` into a destination in `dst`.
static bool Required(SkColorSpace* src, SkColorSpace* dst);
struct Flags {
bool unpremul = false;
@ -35,11 +38,6 @@ struct SkColorSpaceXformSteps {
SkColorSpaceXformSteps(SkColorSpace* src, SkAlphaType srcAT,
SkColorSpace* dst, SkAlphaType dstAT);
template <typename S, typename D>
SkColorSpaceXformSteps(const S& src, const D& dst)
: SkColorSpaceXformSteps(src.colorSpace(), src.alphaType(),
dst.colorSpace(), dst.alphaType()) {}
void apply(float rgba[4]) const;
void apply(SkRasterPipeline*, bool src_is_normalized) const;

22
src/gpu/GrSurfaceContext.cpp
View File

@ -105,12 +105,13 @@ bool GrSurfaceContext::readPixels(const GrImageInfo& origDstInfo, void* dst, siz
// Our tight row bytes may have been changed by clipping.
tightRowBytes = dstInfo.minRowBytes();
bool premul = this->colorInfo().alphaType() == kUnpremul_SkAlphaType &&
dstInfo.alphaType() == kPremul_SkAlphaType;
bool unpremul = this->colorInfo().alphaType() == kPremul_SkAlphaType &&
dstInfo.alphaType() == kUnpremul_SkAlphaType;
SkColorSpaceXformSteps::Flags flags = SkColorSpaceXformSteps{this->colorInfo(), dstInfo}.flags;
bool unpremul = flags.unpremul,
needColorConversion = flags.linearize || flags.gamut_transform || flags.encode,
premul = flags.premul;
bool needColorConversion =
SkColorSpaceXformSteps::Required(this->colorInfo().colorSpace(), dstInfo.colorSpace());
const GrCaps* caps = direct->priv().caps();
// This is the getImageData equivalent to the canvas2D putImageData fast path. We probably don't
@ -263,10 +264,13 @@ bool GrSurfaceContext::writePixels(const GrImageInfo& origSrcInfo, const void* s
// Our tight row bytes may have been changed by clipping.
tightRowBytes = srcInfo.minRowBytes();
SkColorSpaceXformSteps::Flags flags = SkColorSpaceXformSteps{srcInfo, this->colorInfo()}.flags;
bool unpremul = flags.unpremul,
needColorConversion = flags.linearize || flags.gamut_transform || flags.encode,
premul = flags.premul;
bool premul = this->colorInfo().alphaType() == kPremul_SkAlphaType &&
srcInfo.alphaType() == kUnpremul_SkAlphaType;
bool unpremul = this->colorInfo().alphaType() == kUnpremul_SkAlphaType &&
srcInfo.alphaType() == kPremul_SkAlphaType;
bool needColorConversion =
SkColorSpaceXformSteps::Required(srcInfo.colorSpace(), this->colorInfo().colorSpace());
const GrCaps* caps = direct->priv().caps();

7
src/shaders/SkShader.cpp
View File

@ -111,12 +111,7 @@ SkShaderBase::Context::Context(const SkShaderBase& shader, const ContextRec& rec
SkShaderBase::Context::~Context() {}
bool SkShaderBase::ContextRec::isLegacyCompatible(SkColorSpace* shaderColorSpace) const {
// In legacy pipelines, shaders always produce premul (or opaque) and the destination is also
// always premul (or opaque). (And those "or opaque" caveats won't make any difference here.)
SkAlphaType shaderAT = kPremul_SkAlphaType,
dstAT = kPremul_SkAlphaType;
return 0 == SkColorSpaceXformSteps{shaderColorSpace, shaderAT,
fDstColorSpace, dstAT}.flags.mask();
return !SkColorSpaceXformSteps::Required(shaderColorSpace, fDstColorSpace);
}
SkImage* SkShader::isAImage(SkMatrix* localMatrix, SkTileMode xy[2]) const {