Fix picture shader handling of outer local matrix
Instead of applying local matrices to the cached shader, pass them
explicitly/composed to createContext/appendStages/asFragmentProcessor.
Change-Id: I39aaf07ac883094c447c4e03e2ef9dcf8de13555
Reviewed-on: https://skia-review.googlesource.com/104580
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
diff --git a/gm/pictureshader.cpp b/gm/pictureshader.cpp
index bc98d80..df91a33 100644
--- a/gm/pictureshader.cpp
+++ b/gm/pictureshader.cpp
@@ -25,10 +25,10 @@
class PictureShaderGM : public skiagm::GM {
public:
- PictureShaderGM(SkScalar tileSize, SkScalar sceneSize)
+ PictureShaderGM(SkScalar tileSize, SkScalar sceneSize, bool useLocalMatrixWrapper = false)
: fTileSize(tileSize)
- , fSceneSize(sceneSize) {
- }
+ , fSceneSize(sceneSize)
+ , fUseLocalMatrixWrapper(useLocalMatrixWrapper) {}
protected:
void onOnceBeforeDraw() override {
@@ -47,7 +47,7 @@
SkString onShortName() override {
- return SkString("pictureshader");
+ return SkStringPrintf("pictureshader%s", fUseLocalMatrixWrapper ? "_localwrapper" : "");
}
SkISize onISize() override {
@@ -151,32 +151,43 @@
canvas->drawRect(SkRect::MakeWH(fSceneSize, fSceneSize), paint);
canvas->drawRect(SkRect::MakeXYWH(fSceneSize * 1.1f, 0, fSceneSize, fSceneSize), paint);
- paint.setShader(SkShader::MakePictureShader(fPicture, kTileConfigs[tileMode].tmx,
- kTileConfigs[tileMode].tmy, &localMatrix,
- nullptr));
+ auto pictureShader = SkShader::MakePictureShader(fPicture, kTileConfigs[tileMode].tmx,
+ kTileConfigs[tileMode].tmy,
+ fUseLocalMatrixWrapper
+ ? nullptr : &localMatrix,
+ nullptr);
+ paint.setShader(fUseLocalMatrixWrapper
+ ? pictureShader->makeWithLocalMatrix(localMatrix)
+ : pictureShader);
canvas->drawRect(SkRect::MakeWH(fSceneSize, fSceneSize), paint);
canvas->translate(fSceneSize * 1.1f, 0);
- paint.setShader(SkShader::MakeBitmapShader(fBitmap,
- kTileConfigs[tileMode].tmx,
- kTileConfigs[tileMode].tmy,
- &localMatrix));
+ auto bitmapShader = SkShader::MakeBitmapShader(fBitmap,
+ kTileConfigs[tileMode].tmx,
+ kTileConfigs[tileMode].tmy,
+ fUseLocalMatrixWrapper
+ ? nullptr : &localMatrix);
+ paint.setShader(fUseLocalMatrixWrapper
+ ? bitmapShader->makeWithLocalMatrix(localMatrix)
+ : bitmapShader);
canvas->drawRect(SkRect::MakeWH(fSceneSize, fSceneSize), paint);
canvas->restore();
}
- SkScalar fTileSize;
- SkScalar fSceneSize;
-
sk_sp<SkPicture> fPicture;
SkBitmap fBitmap;
+ SkScalar fTileSize;
+ SkScalar fSceneSize;
+ bool fUseLocalMatrixWrapper;
+
typedef GM INHERITED;
};
DEF_GM(return new PictureShaderGM(50, 100);)
+DEF_GM(return new PictureShaderGM(50, 100, true);)
DEF_SIMPLE_GM(tiled_picture_shader, canvas, 400, 400) {
// https://code.google.com/p/skia/issues/detail?id=3398
diff --git a/src/shaders/SkPictureShader.cpp b/src/shaders/SkPictureShader.cpp
index 21bd5a5..097a605 100644
--- a/src/shaders/SkPictureShader.cpp
+++ b/src/shaders/SkPictureShader.cpp
@@ -38,7 +38,6 @@
SkShader::TileMode tmx,
SkShader::TileMode tmy,
const SkSize& scale,
- const SkMatrix& localMatrix,
SkTransferFunctionBehavior blendBehavior)
: fColorSpace(std::move(colorSpace))
, fTile(tile)
@@ -47,15 +46,10 @@
, fScale(scale)
, fBlendBehavior(blendBehavior) {
- for (int i = 0; i < 9; ++i) {
- fLocalMatrixStorage[i] = localMatrix[i];
- }
-
static const size_t keySize = sizeof(fColorSpace) +
sizeof(fTile) +
sizeof(fTmx) + sizeof(fTmy) +
sizeof(fScale) +
- sizeof(fLocalMatrixStorage) +
sizeof(fBlendBehavior);
// This better be packed.
SkASSERT(sizeof(uint32_t) * (&fEndOfStruct - (uint32_t*)&fColorSpace) == keySize);
@@ -79,7 +73,6 @@
SkRect fTile;
SkShader::TileMode fTmx, fTmy;
SkSize fScale;
- SkScalar fLocalMatrixStorage[9];
SkTransferFunctionBehavior fBlendBehavior;
SkDEBUGCODE(uint32_t fEndOfStruct;)
@@ -179,16 +172,28 @@
fPicture->flatten(buffer);
}
-sk_sp<SkShader> SkPictureShader::refBitmapShader(const SkMatrix& viewMatrix, const SkMatrix* localM,
+// This helper returns two artifacts:
+//
+// 1) a cached image shader, which wraps a single picture tile at the given CTM/local matrix
+//
+// 2) a "composite" local matrix, to be passed down when dispatching createContext(),
+// appendStages() and asFragmentProcessor() in callers
+//
+// The composite local matrix includes the actual local matrix, any inherited/outer local matrix
+// and a scale component (to mape the actual tile bitmap size -> fTile size).
+//
+sk_sp<SkShader> SkPictureShader::refBitmapShader(const SkMatrix& viewMatrix,
+ const SkMatrix* outerLocalMatrix,
SkColorSpace* dstColorSpace,
+ SkMatrix* compositeLocalMatrix,
const int maxTextureSize) const {
SkASSERT(fPicture && !fPicture->cullRect().isEmpty());
- SkMatrix m;
- m.setConcat(viewMatrix, this->getLocalMatrix());
- if (localM) {
- m.preConcat(*localM);
+ *compositeLocalMatrix = this->getLocalMatrix();
+ if (outerLocalMatrix) {
+ compositeLocalMatrix->preConcat(*outerLocalMatrix);
}
+ const SkMatrix m = SkMatrix::Concat(viewMatrix, *compositeLocalMatrix);
// Use a rotation-invariant scale
SkPoint scale;
@@ -246,7 +251,6 @@
fTmx,
fTmy,
tileScale,
- this->getLocalMatrix(),
blendBehavior);
if (!SkResourceCache::Find(key, BitmapShaderRec::Visitor, &tileShader)) {
@@ -265,14 +269,14 @@
tileImage = tileImage->makeColorSpace(fColorSpace, SkTransferFunctionBehavior::kIgnore);
}
- SkMatrix shaderMatrix = this->getLocalMatrix();
- shaderMatrix.preScale(1 / tileScale.width(), 1 / tileScale.height());
- tileShader = tileImage->makeShader(fTmx, fTmy, &shaderMatrix);
+ tileShader = tileImage->makeShader(fTmx, fTmy);
SkResourceCache::Add(new BitmapShaderRec(key, tileShader.get()));
fAddedToCache.store(true);
}
+ compositeLocalMatrix->preScale(1 / tileScale.width(), 1 / tileScale.height());
+
return tileShader;
}
@@ -284,21 +288,32 @@
bool SkPictureShader::onAppendStages(const StageRec& rec) const {
// Keep bitmapShader alive by using alloc instead of stack memory
auto& bitmapShader = *rec.fAlloc->make<sk_sp<SkShader>>();
- bitmapShader = this->refBitmapShader(rec.fCTM, rec.fLocalM, rec.fDstCS);
- return bitmapShader && as_SB(bitmapShader)->appendStages(rec);
+ SkMatrix compositeLocalMatrix;
+ bitmapShader = this->refBitmapShader(rec.fCTM, rec.fLocalM, rec.fDstCS, &compositeLocalMatrix);
+
+ StageRec localRec = rec;
+ localRec.fLocalM = compositeLocalMatrix.isIdentity() ? nullptr : &compositeLocalMatrix;
+
+ return bitmapShader && as_SB(bitmapShader)->appendStages(localRec);
}
/////////////////////////////////////////////////////////////////////////////////////////
SkShaderBase::Context* SkPictureShader::onMakeContext(const ContextRec& rec, SkArenaAlloc* alloc)
const {
- sk_sp<SkShader> bitmapShader(this->refBitmapShader(*rec.fMatrix, rec.fLocalMatrix,
- rec.fDstColorSpace));
+ SkMatrix compositeLocalMatrix;
+ sk_sp<SkShader> bitmapShader = this->refBitmapShader(*rec.fMatrix,
+ rec.fLocalMatrix,
+ rec.fDstColorSpace,
+ &compositeLocalMatrix);
if (!bitmapShader) {
return nullptr;
}
+ ContextRec localRec = rec;
+ localRec.fLocalMatrix = compositeLocalMatrix.isIdentity() ? nullptr : &compositeLocalMatrix;
+
PictureShaderContext* ctx =
- alloc->make<PictureShaderContext>(*this, rec, std::move(bitmapShader), alloc);
+ alloc->make<PictureShaderContext>(*this, localRec, std::move(bitmapShader), alloc);
if (nullptr == ctx->fBitmapShaderContext) {
ctx = nullptr;
}
@@ -362,14 +377,20 @@
if (args.fContext) {
maxTextureSize = args.fContext->caps()->maxTextureSize();
}
+ SkMatrix compositeLocalMatrix;
sk_sp<SkShader> bitmapShader(this->refBitmapShader(*args.fViewMatrix, args.fLocalMatrix,
args.fDstColorSpaceInfo->colorSpace(),
+ &compositeLocalMatrix,
maxTextureSize));
if (!bitmapShader) {
return nullptr;
}
- return as_SB(bitmapShader)
- ->asFragmentProcessor(GrFPArgs(args.fContext, args.fViewMatrix,
- args.fFilterQuality, args.fDstColorSpaceInfo));
+
+ return as_SB(bitmapShader)->asFragmentProcessor(
+ GrFPArgs(args.fContext,
+ args.fViewMatrix,
+ compositeLocalMatrix.isIdentity() ? nullptr : &compositeLocalMatrix,
+ args.fFilterQuality,
+ args.fDstColorSpaceInfo));
}
#endif
diff --git a/src/shaders/SkPictureShader.h b/src/shaders/SkPictureShader.h
index fe388ee..3b7cab0 100644
--- a/src/shaders/SkPictureShader.h
+++ b/src/shaders/SkPictureShader.h
@@ -49,6 +49,7 @@
sk_sp<SkShader> refBitmapShader(const SkMatrix&, const SkMatrix* localMatrix,
SkColorSpace* dstColorSpace,
+ SkMatrix* compositeLocalMatrix,
const int maxTextureSize = 0) const;
class PictureShaderContext : public Context {