aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Florin Malita <fmalita@chromium.org>2018-04-19 21:07:19 -0400
committerGravatar Skia Commit-Bot <skia-commit-bot@chromium.org>2018-04-20 14:14:45 +0000
commit8f88d89796d0ab7fefb302b21e03cc186dfc8bc3 (patch)
tree8cff6d250fdb65877142c1a1d9890b3ce7b9ea77
parentadeb75d67caff878ce6c5f8654c34c3fdd201776 (diff)
Fix use-of-uninitialized-value in SkPictureShader::onMakeContext
SkPictureShader::refBitmapShader is expected to always initialize the scale adjust vector when returning a non-null shader. But the code path returning EmptyShader does not do that. Instead of hauling around a separate scale adjustment, we can refactor to avoid this problem by adjusting the local matrix directly, if needed, in refBitmapShader. The local matrix is conveniently already stored in a SkTCopyOnFirstWrite. Bug: chromium:835048, oss-fuzz:7738 Change-Id: I2df3bde7d6237f01bc71857c2fe254e86b186dc0 Reviewed-on: https://skia-review.googlesource.com/122544 Auto-Submit: Florin Malita <fmalita@chromium.org> Reviewed-by: Kevin Lubick <kjlubick@google.com> Commit-Queue: Florin Malita <fmalita@chromium.org>
-rw-r--r--src/shaders/SkPictureShader.cpp47
-rw-r--r--src/shaders/SkPictureShader.h3
2 files changed, 12 insertions, 38 deletions
diff --git a/src/shaders/SkPictureShader.cpp b/src/shaders/SkPictureShader.cpp
index bbed6d695a..42f5c3e497 100644
--- a/src/shaders/SkPictureShader.cpp
+++ b/src/shaders/SkPictureShader.cpp
@@ -171,24 +171,15 @@ void SkPictureShader::flatten(SkWriteBuffer& buffer) const {
fPicture->flatten(buffer);
}
-// This helper returns two artifacts:
-//
-// 1) a cached image shader, which wraps a single picture tile at the given CTM/local matrix
-//
-// 2) a tile scale adjustment, to be applied downstream 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).
-//
+// Returns a cached image shader, which wraps a single picture tile at the given
+// CTM/local matrix. Also adjusts the local matrix for tile scaling.
sk_sp<SkShader> SkPictureShader::refBitmapShader(const SkMatrix& viewMatrix,
- const SkMatrix& localMatrix,
+ SkTCopyOnFirstWrite<SkMatrix>* localMatrix,
SkColorSpace* dstColorSpace,
- SkVector* scaleAdjust,
const int maxTextureSize) const {
SkASSERT(fPicture && !fPicture->cullRect().isEmpty());
- const SkMatrix m = SkMatrix::Concat(viewMatrix, localMatrix);
+ const SkMatrix m = SkMatrix::Concat(viewMatrix, **localMatrix);
// Use a rotation-invariant scale
SkPoint scale;
@@ -270,27 +261,24 @@ sk_sp<SkShader> SkPictureShader::refBitmapShader(const SkMatrix& viewMatrix,
fAddedToCache.store(true);
}
- scaleAdjust->set(1 / tileScale.width(), 1 / tileScale.height());
+ if (tileScale.width() != 1 || tileScale.height() != 1) {
+ localMatrix->writable()->preScale(1 / tileScale.width(), 1 / tileScale.height());
+ }
return tileShader;
}
bool SkPictureShader::onAppendStages(const StageRec& rec) const {
auto lm = this->totalLocalMatrix(rec.fLocalM);
- SkVector scaleAdjust;
// Keep bitmapShader alive by using alloc instead of stack memory
auto& bitmapShader = *rec.fAlloc->make<sk_sp<SkShader>>();
- bitmapShader = this->refBitmapShader(rec.fCTM, *lm, rec.fDstCS, &scaleAdjust);
+ bitmapShader = this->refBitmapShader(rec.fCTM, &lm, rec.fDstCS);
if (!bitmapShader) {
return false;
}
- if (scaleAdjust != SkVector::Make(1, 1)) {
- lm.writable()->preScale(scaleAdjust.fX, scaleAdjust.fY);
- }
-
StageRec localRec = rec;
localRec.fLocalM = lm->isIdentity() ? nullptr : lm.get();
@@ -301,19 +289,11 @@ bool SkPictureShader::onAppendStages(const StageRec& rec) const {
SkShaderBase::Context* SkPictureShader::onMakeContext(const ContextRec& rec, SkArenaAlloc* alloc)
const {
auto lm = this->totalLocalMatrix(rec.fLocalMatrix);
- SkVector scaleAdjust;
- sk_sp<SkShader> bitmapShader = this->refBitmapShader(*rec.fMatrix,
- *lm,
- rec.fDstColorSpace,
- &scaleAdjust);
+ sk_sp<SkShader> bitmapShader = this->refBitmapShader(*rec.fMatrix, &lm, rec.fDstColorSpace);
if (!bitmapShader) {
return nullptr;
}
- if (scaleAdjust != SkVector::Make(1, 1)) {
- lm.writable()->preScale(scaleAdjust.fX, scaleAdjust.fY);
- }
-
ContextRec localRec = rec;
localRec.fLocalMatrix = lm->isIdentity() ? nullptr : lm.get();
@@ -382,18 +362,13 @@ std::unique_ptr<GrFragmentProcessor> SkPictureShader::asFragmentProcessor(
}
auto lm = this->totalLocalMatrix(args.fPreLocalMatrix, args.fPostLocalMatrix);
- SkVector scaleAdjust;
- sk_sp<SkShader> bitmapShader(this->refBitmapShader(*args.fViewMatrix,*lm,
+ sk_sp<SkShader> bitmapShader(this->refBitmapShader(*args.fViewMatrix, &lm,
args.fDstColorSpaceInfo->colorSpace(),
- &scaleAdjust, maxTextureSize));
+ maxTextureSize));
if (!bitmapShader) {
return nullptr;
}
- if (scaleAdjust != SkVector::Make(1, 1)) {
- lm.writable()->preScale(scaleAdjust.fX, scaleAdjust.fY);
- }
-
// We want to *reset* args.fPreLocalMatrix, not compose it.
GrFPArgs newArgs(args.fContext, args.fViewMatrix, args.fFilterQuality, args.fDstColorSpaceInfo);
newArgs.fPreLocalMatrix = lm.get();
diff --git a/src/shaders/SkPictureShader.h b/src/shaders/SkPictureShader.h
index 5f28b6074b..6e5201e8fb 100644
--- a/src/shaders/SkPictureShader.h
+++ b/src/shaders/SkPictureShader.h
@@ -46,9 +46,8 @@ private:
SkPictureShader(sk_sp<SkPicture>, TileMode, TileMode, const SkMatrix*, const SkRect*,
sk_sp<SkColorSpace>);
- sk_sp<SkShader> refBitmapShader(const SkMatrix&, const SkMatrix& localMatrix,
+ sk_sp<SkShader> refBitmapShader(const SkMatrix&, SkTCopyOnFirstWrite<SkMatrix>* localMatrix,
SkColorSpace* dstColorSpace,
- SkVector* scaleAdjust,
const int maxTextureSize = 0) const;
class PictureShaderContext : public Context {