In GrSkSLFP, store inputs as SkData. Tighten size checks.
Simplifies interaction with users who already have inputs in this form,
and doesn't really add any overhead to other clients.
Change-Id: I6e137e02a65e007621adf481ffb994665117caf4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268836
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/include/effects/SkArithmeticImageFilter.h b/include/effects/SkArithmeticImageFilter.h
index 2945a13..84c2765 100644
--- a/include/effects/SkArithmeticImageFilter.h
+++ b/include/effects/SkArithmeticImageFilter.h
@@ -11,12 +11,8 @@
#include "include/core/SkImageFilter.h"
struct ArithmeticFPInputs {
- ArithmeticFPInputs() {
- memset(this, 0, sizeof(*this));
- }
-
- float k[4];
- bool enforcePMColor;
+ float fK[4];
+ bool fEnforcePMColor;
};
// DEPRECATED: Use include/effects/SkImageFilters::Arithmetic
diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp
index 0cb50ce..ee28e98 100644
--- a/src/core/SkColorFilter.cpp
+++ b/src/core/SkColorFilter.cpp
@@ -408,9 +408,7 @@
#if SK_SUPPORT_GPU
std::unique_ptr<GrFragmentProcessor> asFragmentProcessor(GrRecordingContext* context,
const GrColorInfo&) const override {
- return GrSkSLFP::Make(context, fEffect, "Runtime Color Filter",
- fInputs ? fInputs->data() : nullptr,
- fInputs ? fInputs->size() : 0);
+ return GrSkSLFP::Make(context, fEffect, "Runtime Color Filter", fInputs);
}
#endif
diff --git a/src/core/SkRuntimeEffect.cpp b/src/core/SkRuntimeEffect.cpp
index a3ab9a4..6ae366b 100644
--- a/src/core/SkRuntimeEffect.cpp
+++ b/src/core/SkRuntimeEffect.cpp
@@ -211,9 +211,9 @@
}
size_t SkRuntimeEffect::inputSize() const {
- return fInAndUniformVars.empty()
- ? 0
- : fInAndUniformVars.back().fOffset + fInAndUniformVars.back().sizeInBytes();
+ return fInAndUniformVars.empty() ? 0
+ : SkAlign4(fInAndUniformVars.back().fOffset +
+ fInAndUniformVars.back().sizeInBytes());
}
SkRuntimeEffect::SpecializeResult SkRuntimeEffect::specialize(SkSL::Program& baseProgram,
@@ -300,7 +300,7 @@
sk_sp<SkShader> SkRuntimeEffect::makeShader(sk_sp<SkData> inputs,
sk_sp<SkShader> children[], size_t childCount,
const SkMatrix* localMatrix, bool isOpaque) {
- return inputs && inputs->size() >= this->inputSize() && childCount >= fChildren.size()
+ return inputs && inputs->size() == this->inputSize() && childCount >= fChildren.size()
? sk_sp<SkShader>(new SkRTShader(sk_ref_sp(this), std::move(inputs), localMatrix,
children, childCount, isOpaque))
: nullptr;
@@ -309,7 +309,7 @@
sk_sp<SkColorFilter> SkRuntimeEffect::makeColorFilter(sk_sp<SkData> inputs) {
extern sk_sp<SkColorFilter> SkMakeRuntimeColorFilter(sk_sp<SkRuntimeEffect>, sk_sp<SkData>);
- return inputs && inputs->size() >= this->inputSize()
+ return inputs && inputs->size() == this->inputSize()
? SkMakeRuntimeColorFilter(sk_ref_sp(this), std::move(inputs))
: nullptr;
}
diff --git a/src/effects/SkOverdrawColorFilter.cpp b/src/effects/SkOverdrawColorFilter.cpp
index b7b724d..266c5b1 100644
--- a/src/effects/SkOverdrawColorFilter.cpp
+++ b/src/effects/SkOverdrawColorFilter.cpp
@@ -91,11 +91,13 @@
std::unique_ptr<GrFragmentProcessor> SkOverdrawColorFilter::asFragmentProcessor(
GrRecordingContext* context, const GrColorInfo&) const {
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_OVERDRAW_SRC)));
- SkColor4f floatColors[kNumColors];
+ SkASSERT(effect->inputSize() == (kNumColors * sizeof(SkColor4f)));
+ auto inputs = SkData::MakeUninitialized(kNumColors * sizeof(SkColor4f));
+ SkColor4f* floatColors = reinterpret_cast<SkColor4f*>(inputs->writable_data());
for (int i = 0; i < kNumColors; ++i) {
floatColors[i] = SkColor4f::FromBytes_RGBA(fColors[i]);
}
- return GrSkSLFP::Make(context, effect, "Overdraw", floatColors, sizeof(floatColors));
+ return GrSkSLFP::Make(context, effect, "Overdraw", std::move(inputs));
}
#endif
diff --git a/src/effects/imagefilters/SkArithmeticImageFilter.cpp b/src/effects/imagefilters/SkArithmeticImageFilter.cpp
index 180be8a..06045a4 100644
--- a/src/effects/imagefilters/SkArithmeticImageFilter.cpp
+++ b/src/effects/imagefilters/SkArithmeticImageFilter.cpp
@@ -52,7 +52,7 @@
public:
ArithmeticImageFilterImpl(float k1, float k2, float k3, float k4, bool enforcePMColor,
sk_sp<SkImageFilter> inputs[2], const CropRect* cropRect)
- : INHERITED(inputs, 2, cropRect), fK{k1, k2, k3, k4}, fEnforcePMColor(enforcePMColor) {}
+ : INHERITED(inputs, 2, cropRect), fInputs{{k1, k2, k3, k4}, enforcePMColor} {}
protected:
sk_sp<SkSpecialImage> onFilterImage(const Context&, SkIPoint* offset) const override;
@@ -77,10 +77,9 @@
friend void SkArithmeticImageFilter::RegisterFlattenables();
SK_FLATTENABLE_HOOKS(ArithmeticImageFilterImpl)
- bool affectsTransparentBlack() const override { return !SkScalarNearlyZero(fK[3]); }
+ bool affectsTransparentBlack() const override { return !SkScalarNearlyZero(fInputs.fK[3]); }
- const float fK[4];
- const bool fEnforcePMColor;
+ ArithmeticFPInputs fInputs;
typedef SkImageFilter_Base INHERITED;
};
@@ -142,9 +141,9 @@
void ArithmeticImageFilterImpl::flatten(SkWriteBuffer& buffer) const {
this->INHERITED::flatten(buffer);
for (int i = 0; i < 4; ++i) {
- buffer.writeScalar(fK[i]);
+ buffer.writeScalar(fInputs.fK[i]);
}
- buffer.writeBool(fEnforcePMColor);
+ buffer.writeBool(fInputs.fEnforcePMColor);
}
static Sk4f pin(float min, const Sk4f& val, float max) {
@@ -282,33 +281,33 @@
// Arithmetic with non-zero k4 may influence the complete filter primitive
// region. [k4 > 0 => result(0,0) = k4 => result(i1,i2) >= k4]
- if (!SkScalarNearlyZero(fK[3])) {
+ if (!SkScalarNearlyZero(fInputs.fK[3])) {
i1.join(i2);
return i1;
}
// If both K2 or K3 are non-zero, both i1 and i2 appear.
- if (!SkScalarNearlyZero(fK[1]) && !SkScalarNearlyZero(fK[2])) {
+ if (!SkScalarNearlyZero(fInputs.fK[1]) && !SkScalarNearlyZero(fInputs.fK[2])) {
i1.join(i2);
return i1;
}
// If k2 is non-zero, output can be produced whenever i1 is non-transparent.
// [k3 = k4 = 0 => result(i1,i2) = k1*i1*i2 + k2*i1 = (k1*i2 + k2)*i1]
- if (!SkScalarNearlyZero(fK[1])) {
+ if (!SkScalarNearlyZero(fInputs.fK[1])) {
return i1;
}
// If k3 is non-zero, output can be produced whenever i2 is non-transparent.
// [k2 = k4 = 0 => result(i1,i2) = k1*i1*i2 + k3*i2 = (k1*i1 + k3)*i2]
- if (!SkScalarNearlyZero(fK[2])) {
+ if (!SkScalarNearlyZero(fInputs.fK[2])) {
return i2;
}
// If just k1 is non-zero, output will only be produce where both inputs
// are non-transparent. Use intersection.
// [k1 > 0 and k2 = k3 = k4 = 0 => result(i1,i2) = k1*i1*i2]
- if (!SkScalarNearlyZero(fK[0])) {
+ if (!SkScalarNearlyZero(fInputs.fK[0])) {
if (!i1.intersect(i2)) {
return SkIRect::MakeEmpty();
}
@@ -384,12 +383,9 @@
paint.addColorFragmentProcessor(std::move(fgFP));
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_ARITHMETIC_SRC)));
- ArithmeticFPInputs inputs;
- static_assert(sizeof(inputs.k) == sizeof(fK), "struct size mismatch");
- memcpy(inputs.k, fK, sizeof(inputs.k));
- inputs.enforcePMColor = fEnforcePMColor;
- std::unique_ptr<GrFragmentProcessor> xferFP = GrSkSLFP::Make(context, effect, "Arithmetic",
- &inputs, sizeof(inputs));
+ SkASSERT(effect->inputSize() == sizeof(fInputs));
+ std::unique_ptr<GrFragmentProcessor> xferFP = GrSkSLFP::Make(
+ context, effect, "Arithmetic", SkData::MakeWithCopy(&fInputs, sizeof(fInputs)));
if (xferFP) {
((GrSkSLFP&) *xferFP).addChild(std::move(bgFP));
paint.addColorFragmentProcessor(std::move(xferFP));
@@ -445,11 +441,11 @@
return;
}
- auto proc = fEnforcePMColor ? arith_span<true> : arith_span<false>;
+ auto proc = fInputs.fEnforcePMColor ? arith_span<true> : arith_span<false>;
SkPixmap tmpDst = dst;
if (intersect(&tmpDst, &src, fgoffset.fLeft, fgoffset.fTop)) {
for (int y = 0; y < tmpDst.height(); ++y) {
- proc(fK, tmpDst.writable_addr32(0, y), src.addr32(0, y), tmpDst.width());
+ proc(fInputs.fK, tmpDst.writable_addr32(0, y), src.addr32(0, y), tmpDst.width());
}
}
}
@@ -457,11 +453,11 @@
// Now apply the mode with transparent-color to the outside of the fg image
SkRegion outside(SkIRect::MakeWH(dst.width(), dst.height()));
outside.op(fgoffset, SkRegion::kDifference_Op);
- auto proc = fEnforcePMColor ? arith_transparent<true> : arith_transparent<false>;
+ auto proc = fInputs.fEnforcePMColor ? arith_transparent<true> : arith_transparent<false>;
for (SkRegion::Iterator iter(outside); !iter.done(); iter.next()) {
const SkIRect r = iter.rect();
for (int y = r.fTop; y < r.fBottom; ++y) {
- proc(fK, dst.writable_addr32(r.fLeft, y), r.width());
+ proc(fInputs.fK, dst.writable_addr32(r.fLeft, y), r.width());
}
}
}
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index 6224285..e051326 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -355,7 +355,7 @@
if (ditherRange >= 0) {
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_DITHER_SRC)));
auto ditherFP = GrSkSLFP::Make(context, effect, "Dither",
- &ditherRange, sizeof(ditherRange));
+ SkData::MakeWithCopy(&ditherRange, sizeof(ditherRange)));
if (ditherFP) {
grPaint->addColorFragmentProcessor(std::move(ditherFP));
}
diff --git a/src/gpu/effects/GrSkSLFP.cpp b/src/gpu/effects/GrSkSLFP.cpp
index 2bafaa2..7dfb413 100644
--- a/src/gpu/effects/GrSkSLFP.cpp
+++ b/src/gpu/effects/GrSkSLFP.cpp
@@ -117,7 +117,7 @@
const GrFragmentProcessor& _proc) override {
size_t uniIndex = 0;
const GrSkSLFP& outer = _proc.cast<GrSkSLFP>();
- char* inputs = (char*) outer.fInputs.get();
+ const uint8_t* inputs = outer.fInputs->bytes();
for (const auto& v : outer.fEffect->inputs()) {
if (v.fQualifier != SkRuntimeEffect::Variable::Qualifier::kUniform) {
continue;
@@ -160,24 +160,23 @@
};
std::unique_ptr<GrSkSLFP> GrSkSLFP::Make(GrContext_Base* context, sk_sp<SkRuntimeEffect> effect,
- const char* name, const void* inputs, size_t inputSize,
+ const char* name, sk_sp<SkData> inputs,
const SkMatrix* matrix) {
+ if (inputs->size() != effect->inputSize()) {
+ return nullptr;
+ }
return std::unique_ptr<GrSkSLFP>(new GrSkSLFP(context->priv().caps()->refShaderCaps(),
- std::move(effect), name, inputs, inputSize,
+ std::move(effect), name, std::move(inputs),
matrix));
}
GrSkSLFP::GrSkSLFP(sk_sp<const GrShaderCaps> shaderCaps, sk_sp<SkRuntimeEffect> effect,
- const char* name, const void* inputs, size_t inputSize, const SkMatrix* matrix)
+ const char* name, sk_sp<SkData> inputs, const SkMatrix* matrix)
: INHERITED(kGrSkSLFP_ClassID, kNone_OptimizationFlags)
, fShaderCaps(std::move(shaderCaps))
, fEffect(std::move(effect))
, fName(name)
- , fInputs(new int8_t[inputSize])
- , fInputSize(inputSize) {
- if (fInputSize) {
- memcpy(fInputs.get(), inputs, inputSize);
- }
+ , fInputs(std::move(inputs)) {
if (matrix) {
fCoordTransform = GrCoordTransform(*matrix);
this->addCoordTransform(&fCoordTransform);
@@ -189,11 +188,7 @@
, fShaderCaps(other.fShaderCaps)
, fEffect(other.fEffect)
, fName(other.fName)
- , fInputs(new int8_t[other.fInputSize])
- , fInputSize(other.fInputSize) {
- if (fInputSize) {
- memcpy(fInputs.get(), other.fInputs.get(), fInputSize);
- }
+ , fInputs(other.fInputs) {
if (other.numCoordTransforms()) {
fCoordTransform = other.fCoordTransform;
this->addCoordTransform(&fCoordTransform);
@@ -211,13 +206,13 @@
GrGLSLFragmentProcessor* GrSkSLFP::onCreateGLSLInstance() const {
// Note: This is actually SkSL (again) but with inline format specifiers.
SkSL::PipelineStageArgs args;
- SkAssertResult(fEffect->toPipelineStage(fInputs.get(), fShaderCaps.get(), &args));
+ SkAssertResult(fEffect->toPipelineStage(fInputs->data(), fShaderCaps.get(), &args));
return new GrGLSLSkSLFP(std::move(args));
}
void GrSkSLFP::onGetGLSLProcessorKey(const GrShaderCaps& caps, GrProcessorKeyBuilder* b) const {
b->add32(fEffect->index());
- char* inputs = (char*) fInputs.get();
+ const uint8_t* inputs = fInputs->bytes();
for (const auto& v : fEffect->inputs()) {
if (v.fQualifier != SkRuntimeEffect::Variable::Qualifier::kIn) {
continue;
@@ -241,9 +236,7 @@
bool GrSkSLFP::onIsEqual(const GrFragmentProcessor& other) const {
const GrSkSLFP& sk = other.cast<GrSkSLFP>();
- SkASSERT(fEffect->index() != sk.fEffect->index() || fInputSize == sk.fInputSize);
- return fEffect->index() == sk.fEffect->index() &&
- !memcmp(fInputs.get(), sk.fInputs.get(), fInputSize);
+ return fEffect->index() == sk.fEffect->index() && fInputs->equals(sk.fInputs.get());
}
std::unique_ptr<GrFragmentProcessor> GrSkSLFP::clone() const {
@@ -277,22 +270,21 @@
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_DITHER_SRC)));
int rangeType = d->fRandom->nextULessThan(3);
auto result = GrSkSLFP::Make(d->context(), effect, "Dither",
- &rangeType, sizeof(rangeType));
+ SkData::MakeWithCopy(&rangeType, sizeof(rangeType)));
return std::unique_ptr<GrFragmentProcessor>(result.release());
}
case 1: {
static auto effect = std::get<0>(SkRuntimeEffect::Make(SkString(SKSL_ARITHMETIC_SRC)));
ArithmeticFPInputs inputs;
- inputs.k[0] = d->fRandom->nextF();
- inputs.k[1] = d->fRandom->nextF();
- inputs.k[2] = d->fRandom->nextF();
- inputs.k[3] = d->fRandom->nextF();
- inputs.enforcePMColor = d->fRandom->nextBool();
+ inputs.fK[0] = d->fRandom->nextF();
+ inputs.fK[1] = d->fRandom->nextF();
+ inputs.fK[2] = d->fRandom->nextF();
+ inputs.fK[3] = d->fRandom->nextF();
+ inputs.fEnforcePMColor = d->fRandom->nextBool();
auto result = GrSkSLFP::Make(d->context(), effect, "Arithmetic",
- &inputs, sizeof(inputs));
+ SkData::MakeWithCopy(&inputs, sizeof(inputs)));
result->addChild(GrConstColorProcessor::Make(
- SK_PMColor4fWHITE,
- GrConstColorProcessor::InputMode::kIgnore));
+ SK_PMColor4fWHITE, GrConstColorProcessor::InputMode::kIgnore));
return std::unique_ptr<GrFragmentProcessor>(result.release());
}
case 2: {
@@ -302,7 +294,7 @@
inputs[i] = SkColor4f::FromBytes_RGBA(d->fRandom->nextU());
}
auto result = GrSkSLFP::Make(d->context(), effect, "Overdraw",
- &inputs, sizeof(inputs));
+ SkData::MakeWithCopy(&inputs, sizeof(inputs)));
return std::unique_ptr<GrFragmentProcessor>(result.release());
}
}
diff --git a/src/gpu/effects/GrSkSLFP.h b/src/gpu/effects/GrSkSLFP.h
index c9fa9e8..283f733 100644
--- a/src/gpu/effects/GrSkSLFP.h
+++ b/src/gpu/effects/GrSkSLFP.h
@@ -24,6 +24,7 @@
class GrContext_Base;
class GrShaderCaps;
+class SkData;
class SkRuntimeEffect;
class GrSkSLFP : public GrFragmentProcessor {
@@ -67,8 +68,7 @@
static std::unique_ptr<GrSkSLFP> Make(GrContext_Base* context,
sk_sp<SkRuntimeEffect> effect,
const char* name,
- const void* inputs,
- size_t inputSize,
+ sk_sp<SkData> inputs,
const SkMatrix* matrix = nullptr);
const char* name() const override;
@@ -79,7 +79,7 @@
private:
GrSkSLFP(sk_sp<const GrShaderCaps> shaderCaps, sk_sp<SkRuntimeEffect> effect,
- const char* name, const void* inputs, size_t inputSize, const SkMatrix* matrix);
+ const char* name, sk_sp<SkData> inputs, const SkMatrix* matrix);
GrSkSLFP(const GrSkSLFP& other);
@@ -93,9 +93,7 @@
sk_sp<SkRuntimeEffect> fEffect;
const char* fName;
-
- const std::unique_ptr<int8_t[]> fInputs;
- size_t fInputSize;
+ sk_sp<SkData> fInputs;
GrCoordTransform fCoordTransform;
diff --git a/src/shaders/SkRTShader.cpp b/src/shaders/SkRTShader.cpp
index 90aeb1f..d368d20 100644
--- a/src/shaders/SkRTShader.cpp
+++ b/src/shaders/SkRTShader.cpp
@@ -130,8 +130,7 @@
if (!this->totalLocalMatrix(args.fPreLocalMatrix, args.fPostLocalMatrix)->invert(&matrix)) {
return nullptr;
}
- auto fp = GrSkSLFP::Make(args.fContext, fEffect, "runtime-shader",
- fInputs->data(), fInputs->size(), &matrix);
+ auto fp = GrSkSLFP::Make(args.fContext, fEffect, "runtime-shader", fInputs, &matrix);
for (const auto& child : fChildren) {
auto childFP = child ? as_SB(child)->asFragmentProcessor(args) : nullptr;
if (!childFP) {