From 47a67a7f4f326d064e9fa1c44f81b3d51f0fafba Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Thu, 16 Jun 2022 12:31:08 -0400 Subject: [PATCH] [graphite] Use CombinationBuilder to create SkPaintParamsKeys directly Bug: skia:12701 Change-Id: I53bbabb2aba44e3bfc215df81dae1ae59d149263 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544319 Reviewed-by: Michael Ludwig Reviewed-by: Greg Daniel Commit-Queue: Robert Phillips --- dm/DMSrcSink.cpp | 2 +- include/core/SkCombinationBuilder.h | 4 + src/core/SkCombinationBuilder.cpp | 105 +++++++++++++++--- src/core/SkKeyHelpers.cpp | 103 ----------------- src/core/SkKeyHelpers.h | 6 - tests/graphite/CombinationBuilderTestAccess.h | 13 +++ tests/graphite/PaintParamsKeyTest.cpp | 60 +++++++++- 7 files changed, 166 insertions(+), 127 deletions(-) diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index 4d04ab60e9..c4f69d1751 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -2167,7 +2167,7 @@ Result GraphiteSink::draw(const Src& src, return result; } - // For now we cast and call directly into Surface. Once we have a been idea of + // For now we cast and call directly into Surface. Once we have a better idea of // what the public API for synchronous graphite readPixels we can update this call to use // that instead. SkPixmap pm; diff --git a/include/core/SkCombinationBuilder.h b/include/core/SkCombinationBuilder.h index 81ac7909bf..5f8ca9e890 100644 --- a/include/core/SkCombinationBuilder.h +++ b/include/core/SkCombinationBuilder.h @@ -19,6 +19,7 @@ class SkArenaAllocWithReset; class SkCombinationBuilder; +class SkKeyContext; class SkOption; class SkPaintParamsKeyBuilder; class SkShaderCodeDictionary; @@ -155,6 +156,9 @@ private: return this->numShaderCombinations() * this->numBlendModeCombinations(); } + // 'desiredCombination' must be less than numCombinations + void createKey(const SkKeyContext&, int desiredCombination, SkPaintParamsKeyBuilder*); + #ifdef SK_DEBUG void dump() const; int epoch() const { return fEpoch; } diff --git a/src/core/SkCombinationBuilder.cpp b/src/core/SkCombinationBuilder.cpp index 01957bf0ad..17591d0d44 100644 --- a/src/core/SkCombinationBuilder.cpp +++ b/src/core/SkCombinationBuilder.cpp @@ -97,6 +97,8 @@ public: int numCombinations() const; + void addToKey(const SkKeyContext&, int desiredCombination, SkPaintParamsKeyBuilder*); + #ifdef SK_DEBUG void dump(int indent) const; #endif @@ -155,6 +157,39 @@ public: return this->numIntrinsicCombinations() * this->numChildCombinations(); } + void addToKey(const SkKeyContext& keyContext, + int desiredCombination, + SkPaintParamsKeyBuilder* keyBuilder) { + SkASSERT(desiredCombination < this->numCombinations()); + + int intrinsicCombination = desiredCombination / this->numChildCombinations(); + int childCombination = desiredCombination % this->numChildCombinations(); + + this->beginBlock(keyContext, intrinsicCombination, keyBuilder); + + if (fNumSlots) { + int numCombinationsSeen = 0; + for (int slotIndex = 0; slotIndex < fNumSlots; ++slotIndex) { + SkSlot* slot = this->getSlot(slotIndex); + + numCombinationsSeen += slot->numCombinations(); + int numCombosLeft = this->numChildCombinations() / numCombinationsSeen; + + int slotCombination; + if (slotIndex+1 < fNumSlots) { + slotCombination = childCombination / (numCombosLeft ? numCombosLeft : 1); + childCombination %= numCombosLeft; + } else { + slotCombination = childCombination; + } + + slot->addToKey(keyContext, slotCombination, keyBuilder); + } + } + + keyBuilder->endBlock(); + } + #ifdef SK_DEBUG void dump(int indent = 0) const { SkDebugf("%s", type_to_str(fType)); @@ -211,6 +246,21 @@ int SkOption::SkSlot::numCombinations() const { return numCombinations; } +void SkOption::SkSlot::addToKey(const SkKeyContext& keyContext, + int desiredCombination, + SkPaintParamsKeyBuilder* keyBuilder) { + SkASSERT(desiredCombination < this->numCombinations()); + + for (SkOption* option = fHead; option; option = option->fNext) { + if (desiredCombination < option->numCombinations()) { + option->addToKey(keyContext, desiredCombination, keyBuilder); + return; + } + + desiredCombination -= option->numCombinations(); + } +} + #ifdef SK_DEBUG void SkOption::SkSlot::dump(int indent) const { SkDebugf("{ %d } ", this->numCombinations()); @@ -657,6 +707,40 @@ void SkCombinationBuilder::dump() const { } #endif +void SkCombinationBuilder::createKey(const SkKeyContext& keyContext, + int desiredCombination, + SkPaintParamsKeyBuilder* keyBuilder) { + SkDEBUGCODE(keyBuilder->checkReset();) + SkASSERT(desiredCombination < this->numCombinations()); + + int numBlendModeCombos = this->numBlendModeCombinations(); + + int desiredShaderCombination = desiredCombination / numBlendModeCombos; + int desiredBlendCombination = desiredCombination % numBlendModeCombos; + + for (SkOption* shaderOption : fShaderOptions) { + if (desiredShaderCombination < shaderOption->numCombinations()) { + shaderOption->addToKey(keyContext, desiredShaderCombination, keyBuilder); + break; + } + + desiredShaderCombination -= shaderOption->numCombinations(); + } + + if (desiredBlendCombination < SkPopCount(fBlendModes)) { + int ith_set_bit = SkNthSet(fBlendModes, desiredBlendCombination); + + SkASSERT(ith_set_bit < kSkBlendModeCount); + SkBlendMode bm = (SkBlendMode) ith_set_bit; + + BlendModeBlock::BeginBlock(keyContext, keyBuilder, /*gatherer=*/nullptr, bm); // bm is used! + keyBuilder->endBlock(); + } else { + // TODO: need to handle fBlenders here + } + +} + void SkCombinationBuilder::buildCombinations( SkShaderCodeDictionary* dict, const std::function& func) { @@ -673,23 +757,12 @@ void SkCombinationBuilder::buildCombinations( this->addOption(SkShaderType::kSolidColor); } - for (int i = 0; i < kSkBlendModeCount; ++i) { - if (!(fBlendModes & (0x1 << i))) { - continue; - } + int numCombos = this->numCombinations(); + for (int i = 0; i < numCombos; ++i) { + this->createKey(keyContext, i, &builder); - SkBlendMode bm = (SkBlendMode) i; + auto entry = dict->findOrCreate(&builder); - // TODO: actually iterate over the SkOption's combinations and have each option add - // itself to the key. - for (SkOption* shaderOption : fShaderOptions) { - // TODO: expand CreateKey to take either an SkBlendMode or an SkBlendID - SkUniquePaintParamsID uniqueID = CreateKey(keyContext, &builder, - shaderOption->type(), bm); - - func(uniqueID); - } + func(entry->uniqueID()); } - - // TODO: need to loop over fBlenders here } diff --git a/src/core/SkKeyHelpers.cpp b/src/core/SkKeyHelpers.cpp index 11f68dfadd..041213c15f 100644 --- a/src/core/SkKeyHelpers.cpp +++ b/src/core/SkKeyHelpers.cpp @@ -641,106 +641,3 @@ const SkRuntimeEffect* TestingOnly_GetCommonRuntimeEffect() { )"); return effect; } - -//-------------------------------------------------------------------------------------------------- -// TODO: we need to feed the number of stops in the gradients into this method from the -// combination code -SkUniquePaintParamsID CreateKey(const SkKeyContext& keyContext, - SkPaintParamsKeyBuilder* builder, - SkShaderType s, - SkBlendMode bm) { - SkDEBUGCODE(builder->checkReset()); - - // TODO: split out the portion of the block data that is always required from the portion - // that is only required to gather uniforms. Right now we're passing in a lot of unused - // data and it is unclear what is actually used. - switch (s) { - case SkShaderType::kSolidColor: - SolidColorShaderBlock::BeginBlock(keyContext, builder, nullptr, - /* unused */ kErrorColor); - builder->endBlock(); - break; - case SkShaderType::kLinearGradient: - GradientShaderBlocks::BeginBlock(keyContext, builder, nullptr, - // only the type and numStops are used - { SkShader::kLinear_GradientType, 0 }); - builder->endBlock(); - break; - case SkShaderType::kRadialGradient: - GradientShaderBlocks::BeginBlock(keyContext, builder, nullptr, - // only the type and numStops are used - { SkShader::kRadial_GradientType, 0 }); - builder->endBlock(); - break; - case SkShaderType::kSweepGradient: - GradientShaderBlocks::BeginBlock(keyContext, builder, nullptr, - // only the type and numStops are used - { SkShader::kSweep_GradientType, 0 }); - builder->endBlock(); - break; - case SkShaderType::kConicalGradient: - GradientShaderBlocks::BeginBlock(keyContext, builder, nullptr, - // only the type and numStops are used - { SkShader::kConical_GradientType, 0 }); - builder->endBlock(); - break; - case SkShaderType::kLocalMatrix: - LocalMatrixShaderBlock::BeginBlock(keyContext, builder, nullptr, - // matrix is unused - { SkMatrix::I() }); - - { - // proxy shader - SolidColorShaderBlock::BeginBlock(keyContext, builder, nullptr, - /* unused */ kErrorColor); - builder->endBlock(); - } - - builder->endBlock(); - break; - case SkShaderType::kImage: - ImageShaderBlock::BeginBlock(keyContext, builder, nullptr, - // none of the ImageData is used - { SkSamplingOptions(), - SkTileMode::kClamp, SkTileMode::kClamp, - SkRect::MakeEmpty(), SkMatrix::I() }); - builder->endBlock(); - break; - case SkShaderType::kBlendShader: - BlendShaderBlock::BeginBlock(keyContext, builder, nullptr, - { SkBlendMode::kSrc }); - - { - // dst - SolidColorShaderBlock::BeginBlock(keyContext, builder, nullptr, - /* unused */ kErrorColor); - builder->endBlock(); - - // src - SolidColorShaderBlock::BeginBlock(keyContext, builder, nullptr, - /* unused */ kErrorColor); - builder->endBlock(); - } - - builder->endBlock(); - break; - case SkShaderType::kRuntimeShader: - { - const SkRuntimeEffect* effect = TestingOnly_GetCommonRuntimeEffect(); - RuntimeShaderBlock::BeginBlock( - keyContext, builder, nullptr, - {sk_ref_sp(effect), SkMatrix::I(), /*uniforms=*/nullptr}); - builder->endBlock(); - } - break; - } - - BlendModeBlock::BeginBlock(keyContext, builder, /* pipelineData*/ nullptr, bm); // 'bm' is used - builder->endBlock(); - - auto dict = keyContext.dict(); - - auto entry = dict->findOrCreate(builder); - - return entry->uniqueID(); -} diff --git a/src/core/SkKeyHelpers.h b/src/core/SkKeyHelpers.h index b3d58e4734..b525a8b71d 100644 --- a/src/core/SkKeyHelpers.h +++ b/src/core/SkKeyHelpers.h @@ -195,12 +195,6 @@ struct RuntimeShaderBlock { const ShaderData&); }; -// Bridge between the combinations system and the SkPaintParamsKey const SkRuntimeEffect* TestingOnly_GetCommonRuntimeEffect(); -SkUniquePaintParamsID CreateKey(const SkKeyContext&, - SkPaintParamsKeyBuilder*, - SkShaderType, - SkBlendMode); - #endif // SkKeyHelpers_DEFINED diff --git a/tests/graphite/CombinationBuilderTestAccess.h b/tests/graphite/CombinationBuilderTestAccess.h index 5d9ff5c2cf..824cfb0828 100644 --- a/tests/graphite/CombinationBuilderTestAccess.h +++ b/tests/graphite/CombinationBuilderTestAccess.h @@ -9,12 +9,25 @@ #define CombinationBuilderTestAccess_DEFINED #include "include/core/SkCombinationBuilder.h" +#include "include/private/SkUniquePaintParamsID.h" + class CombinationBuilderTestAccess { public: static int NumCombinations(SkCombinationBuilder* builder) { return builder->numCombinations(); } + static std::vector BuildCombinations(SkShaderCodeDictionary* dict, + SkCombinationBuilder* builder) { + std::vector uniqueIDs; + + builder->buildCombinations(dict, + [&](SkUniquePaintParamsID uniqueID) { + uniqueIDs.push_back(uniqueID); + }); + + return uniqueIDs; + } #ifdef SK_DEBUG static int Epoch(const SkCombinationBuilder& builder) { return builder.epoch(); diff --git a/tests/graphite/PaintParamsKeyTest.cpp b/tests/graphite/PaintParamsKeyTest.cpp index 9f7bfef8a6..f23c408308 100644 --- a/tests/graphite/PaintParamsKeyTest.cpp +++ b/tests/graphite/PaintParamsKeyTest.cpp @@ -28,6 +28,7 @@ #include "src/gpu/graphite/RecorderPriv.h" #include "src/gpu/graphite/ResourceProvider.h" #include "src/shaders/SkImageShader.h" +#include "tests/graphite/CombinationBuilderTestAccess.h" using namespace skgpu::graphite; @@ -106,6 +107,57 @@ std::tuple create_paint(Recorder* recorder, return { p, numTextures }; } +SkUniquePaintParamsID create_key(Context* context, + SkPaintParamsKeyBuilder* keyBuilder, + SkShaderType shaderType, + SkTileMode tileMode, + SkBlendMode blendMode) { + SkShaderCodeDictionary* dict = context->priv().shaderCodeDictionary(); + + SkCombinationBuilder combinationBuilder(context); + + switch (shaderType) { + case SkShaderType::kSolidColor: + combinationBuilder.addOption(shaderType); + break; + case SkShaderType::kLinearGradient: [[fallthrough]]; + case SkShaderType::kRadialGradient: [[fallthrough]]; + case SkShaderType::kSweepGradient: [[fallthrough]]; + case SkShaderType::kConicalGradient: + combinationBuilder.addOption(shaderType, 2, 2); + break; + case SkShaderType::kLocalMatrix: { + SkCombinationOption option = combinationBuilder.addOption(shaderType); + option.addChildOption(0, SkShaderType::kSolidColor); + } break; + case SkShaderType::kImage: { + SkTileModePair tilingOptions[] = { { tileMode, tileMode } }; + + combinationBuilder.addOption(shaderType, tilingOptions); + } break; + case SkShaderType::kBlendShader: { + SkCombinationOption option = combinationBuilder.addOption(shaderType); + option.addChildOption(0, SkShaderType::kSolidColor); + option.addChildOption(1, SkShaderType::kSolidColor); + } break; + case SkShaderType::kRuntimeShader: { + combinationBuilder.addOption(shaderType); + // TODO: this needs to be connected to the runtime effect from + // TestingOnly_GetCommonRuntimeEffect. Unfortunately, right now, we only have a + // way of adding runtime blenders to the combination system. For now, we skip this + // case below + } break; + } + + combinationBuilder.addOption(blendMode); + + std::vector uniqueIDs = CombinationBuilderTestAccess::BuildCombinations( + dict, &combinationBuilder); + + SkASSERT(uniqueIDs.size() == 1); + return uniqueIDs[0]; +} + } // anonymous namespace // This is intended to be a smoke test for the agreement between the two ways of creating a @@ -148,6 +200,12 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(PaintParamsKeyTest, reporter, context) { } } + // TODO: re-enable this combination when we can add runtime shaders to the combination + // system. + if (s == SkShaderType::kRuntimeShader) { + continue; + } + // TODO: test out a runtime SkBlender here for (auto bm : { SkBlendMode::kSrc, SkBlendMode::kSrcOver }) { auto [ p, expectedNumTextures ] = create_paint(recorder.get(), s, tm, bm); @@ -155,7 +213,7 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(PaintParamsKeyTest, reporter, context) { auto [ uniqueID1, uIndex, tIndex] = ExtractPaintData(recorder.get(), &gatherer, &builder, {}, PaintParams(p)); - SkUniquePaintParamsID uniqueID2 = CreateKey(keyContext, &builder, s, bm); + SkUniquePaintParamsID uniqueID2 = create_key(context, &builder, s, tm, bm); // ExtractPaintData and CreateKey agree REPORTER_ASSERT(reporter, uniqueID1 == uniqueID2);