From 2a04ac3ee1d1a821f0811e9ff1ad75740df8d860 Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Wed, 6 Apr 2022 15:08:57 -0400 Subject: [PATCH] [graphite] Move the UniformManager into the SkPipelineDataGatherer This means that the UniformManager w/in the SkPipelineDataGatherer will now collect all the uniforms into a single block of memory. Bug: skia:12701 Change-Id: I504a014d37662619191d9b519b4e1add69eac8bb Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527837 Reviewed-by: Jim Van Verth Commit-Queue: Robert Phillips --- experimental/graphite/src/BUILD.bazel | 2 +- experimental/graphite/src/ContextUtils.cpp | 4 +- experimental/graphite/src/DrawPass.cpp | 11 +- experimental/graphite/src/Renderer.h | 1 - experimental/graphite/src/UniformManager.cpp | 7 +- experimental/graphite/src/UniformManager.h | 4 +- gn/core.gni | 2 - src/core/BUILD.bazel | 22 +--- src/core/SkKeyHelpers.cpp | 132 +++++++------------ src/core/SkPipelineData.cpp | 47 +------ src/core/SkPipelineData.h | 79 ++++++----- src/core/SkUniformData.cpp | 25 ---- src/core/SkUniformData.h | 56 -------- tests/graphite/BUILD.bazel | 2 - tests/graphite/CommandBufferTest.cpp | 36 ++--- tests/graphite/PipelineDataCacheTest.cpp | 3 +- tests/graphite/UniformTest.cpp | 1 - 17 files changed, 124 insertions(+), 310 deletions(-) delete mode 100644 src/core/SkUniformData.cpp delete mode 100644 src/core/SkUniformData.h diff --git a/experimental/graphite/src/BUILD.bazel b/experimental/graphite/src/BUILD.bazel index fee10e4f9e..7c20483b7d 100644 --- a/experimental/graphite/src/BUILD.bazel +++ b/experimental/graphite/src/BUILD.bazel @@ -777,7 +777,7 @@ generated_cc_atom( "//include/core:SkMatrix_hdr", "//include/private:SkHalf_hdr", "//include/private:SkTemplates_hdr", - "//src/core:SkUniformData_hdr", + "//src/core:SkPipelineData_hdr", "//src/core:SkUniform_hdr", ], ) diff --git a/experimental/graphite/src/ContextUtils.cpp b/experimental/graphite/src/ContextUtils.cpp index 2bf8f29719..c5bb06a08a 100644 --- a/experimental/graphite/src/ContextUtils.cpp +++ b/experimental/graphite/src/ContextUtils.cpp @@ -41,7 +41,7 @@ ExtractPaintData(Recorder* recorder, TextureDataCache* textureDataCache = recorder->priv().textureDataCache(); auto entry = dict->findOrCreate(key, gatherer->blendInfo()); - UniformDataCache::Index uniformIndex = uniformDataCache->insert(gatherer->uniformDataBlock()); + UniformDataCache::Index uniformIndex = uniformDataCache->insert(gatherer->peekUniformData()); TextureDataCache::Index textureIndex = textureDataCache->insert(gatherer->textureDataBlock()); gatherer->reset(); @@ -57,7 +57,7 @@ UniformDataCache::Index ExtractRenderStepData(UniformDataCache* geometryUniformD step->writeUniforms(geometry, gatherer); - UniformDataCache::Index uIndex = geometryUniformDataCache->insert(gatherer->uniformDataBlock()); + UniformDataCache::Index uIndex = geometryUniformDataCache->insert(gatherer->peekUniformData()); gatherer->reset(); diff --git a/experimental/graphite/src/DrawPass.cpp b/experimental/graphite/src/DrawPass.cpp index 48a3f4e8f7..5c5d87027e 100644 --- a/experimental/graphite/src/DrawPass.cpp +++ b/experimental/graphite/src/DrawPass.cpp @@ -205,14 +205,9 @@ public: if (fBindings.find(uIndex.asUInt()) == fBindings.end()) { // First time encountering this data, so upload to the GPU - size_t totalDataSize = udb->totalUniformSize(); - SkASSERT(totalDataSize); - auto[writer, bufferInfo] = fBufferMgr->getUniformWriter(totalDataSize); - - // TODO: this const_cast will go away in a following CL - for (const auto &u : *const_cast(udb)) { - writer.write(u->data(), u->dataSize()); - } + SkASSERT(udb->size()); + auto[writer, bufferInfo] = fBufferMgr->getUniformWriter(udb->size()); + writer.write(udb->data(), udb->size()); fBindings.insert({uIndex.asUInt(), bufferInfo}); } diff --git a/experimental/graphite/src/Renderer.h b/experimental/graphite/src/Renderer.h index 17628f2cbf..a8865fd514 100644 --- a/experimental/graphite/src/Renderer.h +++ b/experimental/graphite/src/Renderer.h @@ -26,7 +26,6 @@ enum class SkPathFillType; class SkPipelineDataGatherer; -class SkUniformData; namespace skgpu { diff --git a/experimental/graphite/src/UniformManager.cpp b/experimental/graphite/src/UniformManager.cpp index e3bce30a37..72c40e9900 100644 --- a/experimental/graphite/src/UniformManager.cpp +++ b/experimental/graphite/src/UniformManager.cpp @@ -11,8 +11,8 @@ #include "include/core/SkMatrix.h" #include "include/private/SkHalf.h" #include "include/private/SkTemplates.h" +#include "src/core/SkPipelineData.h" #include "src/core/SkUniform.h" -#include "src/core/SkUniformData.h" // ensure that these types are the sizes the uniform data is expecting static_assert(sizeof(int32_t) == 4); @@ -508,11 +508,10 @@ UniformManager::UniformManager(Layout layout) : fLayout(layout) { this->reset(); } -sk_sp UniformManager::createUniformData() { - return SkUniformData::Make(fStorage.begin(), fStorage.count()); +SkUniformDataBlock UniformManager::peekData() const { + return SkUniformDataBlock(SkMakeSpan(fStorage.begin(), fStorage.count()), false); } - void UniformManager::reset() { #ifdef SK_DEBUG fCurUBOOffset = 0; diff --git a/experimental/graphite/src/UniformManager.h b/experimental/graphite/src/UniformManager.h index 6b67a82779..8563a63c0a 100644 --- a/experimental/graphite/src/UniformManager.h +++ b/experimental/graphite/src/UniformManager.h @@ -18,7 +18,7 @@ struct SkPoint; struct SkRect; class SkUniform; -class SkUniformData; +class SkUniformDataBlock; namespace skgpu { @@ -34,7 +34,7 @@ class UniformManager { public: UniformManager(Layout layout); - sk_sp createUniformData(); + SkUniformDataBlock peekData() const; int size() const { return fStorage.count(); } void reset(); diff --git a/gn/core.gni b/gn/core.gni index ed74966894..6d7d723371 100644 --- a/gn/core.gni +++ b/gn/core.gni @@ -417,8 +417,6 @@ skia_core_sources = [ "$_src/core/SkTypeface_remote.h", "$_src/core/SkUnPreMultiply.cpp", "$_src/core/SkUniform.h", - "$_src/core/SkUniformData.cpp", - "$_src/core/SkUniformData.h", "$_src/core/SkUtils.cpp", "$_src/core/SkUtils.h", "$_src/core/SkVM.cpp", diff --git a/src/core/BUILD.bazel b/src/core/BUILD.bazel index a230fde9cb..b2d838b2eb 100644 --- a/src/core/BUILD.bazel +++ b/src/core/BUILD.bazel @@ -5652,24 +5652,6 @@ generated_cc_atom( deps = ["//include/core:SkTypes_hdr"], ) -generated_cc_atom( - name = "SkUniformData_hdr", - hdrs = ["SkUniformData.h"], - visibility = ["//:__subpackages__"], - deps = [ - ":SkUniform_hdr", - "//include/core:SkRefCnt_hdr", - "//include/core:SkSpan_hdr", - ], -) - -generated_cc_atom( - name = "SkUniformData_src", - srcs = ["SkUniformData.cpp"], - visibility = ["//:__subpackages__"], - deps = [":SkUniformData_hdr"], -) - generated_cc_atom( name = "SkUniform_hdr", hdrs = ["SkUniform.h"], @@ -5716,11 +5698,13 @@ generated_cc_atom( hdrs = ["SkPipelineData.h"], visibility = ["//:__subpackages__"], deps = [ - ":SkUniformData_hdr", "//experimental/graphite/src:TextureProxy_hdr", "//experimental/graphite/src:UniformManager_hdr", + "//experimental/graphite/src/geom:VectorTypes_hdr", + "//include/core:SkPoint_hdr", "//include/core:SkRefCnt_hdr", "//include/core:SkSamplingOptions_hdr", + "//include/core:SkSpan_hdr", "//include/core:SkTileMode_hdr", "//include/private:SkColorData_hdr", "//src/gpu:Blend_hdr", diff --git a/src/core/SkKeyHelpers.cpp b/src/core/SkKeyHelpers.cpp index 85b38fb774..bc55e3ac5d 100644 --- a/src/core/SkKeyHelpers.cpp +++ b/src/core/SkKeyHelpers.cpp @@ -76,16 +76,10 @@ static const int kBlockDataSize = 0; void add_solid_uniform_data(const SkShaderCodeDictionary* dict, const SkPMColor4f& premulColor, SkPipelineDataGatherer* gatherer) { - - skgpu::UniformManager mgr(gatherer->layout()); - SkDEBUGCODE(auto uniforms = dict->getUniforms(SkBuiltInCodeSnippetID::kSolidColorShader);) - SkDEBUGCODE(mgr.setExpectedUniforms(uniforms);) - mgr.write(premulColor); - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(uniforms);) + gatherer->write(premulColor); + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) } #endif // SK_GRAPHITE_ENABLED @@ -133,81 +127,61 @@ static const int kBlockDataSize = 1; void add_linear_gradient_uniform_data(const SkShaderCodeDictionary* dict, const GradientData& gradData, SkPipelineDataGatherer* gatherer) { - skgpu::UniformManager mgr(gatherer->layout()); - SkDEBUGCODE(auto uniforms = dict->getUniforms(SkBuiltInCodeSnippetID::kLinearGradientShader);) - SkDEBUGCODE(mgr.setExpectedUniforms(uniforms);) - mgr.write(gradData.fColor4fs, GradientData::kMaxStops); - mgr.write(gradData.fOffsets, GradientData::kMaxStops); - mgr.write(gradData.fPoints[0]); - mgr.write(gradData.fPoints[1]); - mgr.write(gradData.fRadii[0]); // unused - mgr.write(gradData.fRadii[1]); // unused - mgr.write(SkPoint::Make(0.0f, 0.0f)); // padding - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(uniforms);) + gatherer->write(gradData.fColor4fs, GradientData::kMaxStops); + gatherer->write(gradData.fOffsets, GradientData::kMaxStops); + gatherer->write(gradData.fPoints[0]); + gatherer->write(gradData.fPoints[1]); + gatherer->write(gradData.fRadii[0]); // unused + gatherer->write(gradData.fRadii[1]); // unused + gatherer->write(SkPoint::Make(0.0f, 0.0f)); // padding + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) }; void add_radial_gradient_uniform_data(const SkShaderCodeDictionary* dict, const GradientData& gradData, SkPipelineDataGatherer* gatherer) { - skgpu::UniformManager mgr(gatherer->layout()); - SkDEBUGCODE(auto uniforms = dict->getUniforms(SkBuiltInCodeSnippetID::kRadialGradientShader);) - SkDEBUGCODE(mgr.setExpectedUniforms(uniforms);) - mgr.write(gradData.fColor4fs, GradientData::kMaxStops); - mgr.write(gradData.fOffsets, GradientData::kMaxStops); - mgr.write(gradData.fPoints[0]); - mgr.write(gradData.fPoints[1]); // unused - mgr.write(gradData.fRadii[0]); - mgr.write(gradData.fRadii[1]); // unused - mgr.write(SkPoint::Make(0.0f, 0.0f)); // padding - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(uniforms);) + gatherer->write(gradData.fColor4fs, GradientData::kMaxStops); + gatherer->write(gradData.fOffsets, GradientData::kMaxStops); + gatherer->write(gradData.fPoints[0]); + gatherer->write(gradData.fPoints[1]); // unused + gatherer->write(gradData.fRadii[0]); + gatherer->write(gradData.fRadii[1]); // unused + gatherer->write(SkPoint::Make(0.0f, 0.0f)); // padding + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) }; void add_sweep_gradient_uniform_data(const SkShaderCodeDictionary* dict, const GradientData& gradData, SkPipelineDataGatherer* gatherer) { - skgpu::UniformManager mgr(gatherer->layout()); - SkDEBUGCODE(auto uniforms = dict->getUniforms(SkBuiltInCodeSnippetID::kSweepGradientShader);) - SkDEBUGCODE(mgr.setExpectedUniforms(uniforms);) - mgr.write(gradData.fColor4fs, GradientData::kMaxStops); - mgr.write(gradData.fOffsets, GradientData::kMaxStops); - mgr.write(gradData.fPoints[0]); - mgr.write(gradData.fPoints[1]); // unused - mgr.write(gradData.fRadii[0]); // unused - mgr.write(gradData.fRadii[1]); // unused - mgr.write(SkPoint::Make(0.0f, 0.0f)); // padding - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(uniforms);) + gatherer->write(gradData.fColor4fs, GradientData::kMaxStops); + gatherer->write(gradData.fOffsets, GradientData::kMaxStops); + gatherer->write(gradData.fPoints[0]); + gatherer->write(gradData.fPoints[1]); // unused + gatherer->write(gradData.fRadii[0]); // unused + gatherer->write(gradData.fRadii[1]); // unused + gatherer->write(SkPoint::Make(0.0f, 0.0f)); // padding + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) }; void add_conical_gradient_uniform_data(const SkShaderCodeDictionary* dict, const GradientData& gradData, SkPipelineDataGatherer* gatherer) { - skgpu::UniformManager mgr(gatherer->layout()); - SkDEBUGCODE(auto uniforms = dict->getUniforms(SkBuiltInCodeSnippetID::kConicalGradientShader);) - SkDEBUGCODE(mgr.setExpectedUniforms(uniforms);) - mgr.write(gradData.fColor4fs, GradientData::kMaxStops); - mgr.write(gradData.fOffsets, GradientData::kMaxStops); - mgr.write(gradData.fPoints[0]); - mgr.write(gradData.fPoints[1]); - mgr.write(gradData.fRadii[0]); - mgr.write(gradData.fRadii[1]); - mgr.write(SkPoint::Make(0.0f, 0.0f)); // padding - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(uniforms);) + gatherer->write(gradData.fColor4fs, GradientData::kMaxStops); + gatherer->write(gradData.fOffsets, GradientData::kMaxStops); + gatherer->write(gradData.fPoints[0]); + gatherer->write(gradData.fPoints[1]); + gatherer->write(gradData.fRadii[0]); + gatherer->write(gradData.fRadii[1]); + gatherer->write(SkPoint::Make(0.0f, 0.0f)); // padding + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) }; #endif // SK_GRAPHITE_ENABLED @@ -330,15 +304,10 @@ namespace { void add_image_uniform_data(const SkShaderCodeDictionary* dict, const ImageData& imgData, SkPipelineDataGatherer* gatherer) { - skgpu::UniformManager mgr(gatherer->layout()); - SkDEBUGCODE(auto uniforms = dict->getUniforms(SkBuiltInCodeSnippetID::kImageShader);) - SkDEBUGCODE(mgr.setExpectedUniforms(uniforms);) - mgr.write(imgData.fSubset); - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(uniforms);) + gatherer->write(imgData.fSubset); + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) } #endif // SK_GRAPHITE_ENABLED @@ -412,18 +381,13 @@ namespace { void add_blendshader_uniform_data(SkShaderCodeDictionary* dict, SkBlendMode bm, SkPipelineDataGatherer* gatherer) { - skgpu::UniformManager mgr(gatherer->layout()); - SkDEBUGCODE(auto uniforms = dict->getUniforms(SkBuiltInCodeSnippetID::kBlendShader);) - SkDEBUGCODE(mgr.setExpectedUniforms(uniforms);) - mgr.write(SkTo(bm)); - mgr.write(0); // padding - remove - mgr.write(0); // padding - remove - mgr.write(0); // padding - remove - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(uniforms);) + gatherer->write(SkTo(bm)); + gatherer->write(0); // padding - remove + gatherer->write(0); // padding - remove + gatherer->write(0); // padding - remove + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) } #endif // SK_GRAPHITE_ENABLED diff --git a/src/core/SkPipelineData.cpp b/src/core/SkPipelineData.cpp index c93c517aa6..9e077bb794 100644 --- a/src/core/SkPipelineData.cpp +++ b/src/core/SkPipelineData.cpp @@ -9,66 +9,33 @@ #include "src/core/SkPipelineData.h" void SkPipelineDataGatherer::reset() { - fUniformDataBlock.reset(); #ifdef SK_GRAPHITE_ENABLED fTextureDataBlock.reset(); fBlendInfo = BlendInfo(); + fUniformManager.reset(); #endif } #ifdef SK_DEBUG void SkPipelineDataGatherer::checkReset() { - SkASSERT(fUniformDataBlock.empty()); #ifdef SK_GRAPHITE_ENABLED SkASSERT(fTextureDataBlock.empty()); SkASSERT(fBlendInfo == BlendInfo()); + SkDEBUGCODE(fUniformManager.checkReset()); #endif } -#endif +#endif // SK_DEBUG std::unique_ptr SkUniformDataBlock::Make(const SkUniformDataBlock& other, SkArenaAlloc* /* arena */) { - return std::make_unique(other); -} + char* newMem = new char[other.size()]; + memcpy(newMem, other.data(), other.size()); -void SkPipelineDataGatherer::add(sk_sp uniforms) { - SkASSERT(uniforms && uniforms->dataSize()); - fUniformDataBlock.add(std::move(uniforms)); -} - -size_t SkUniformDataBlock::totalUniformSize() const { - size_t total = 0; - - // TODO: It seems like we need to worry about alignment between the separate sets of uniforms - for (auto& u : fUniformData) { - total += u->dataSize(); - } - - return total; -} - -bool SkUniformDataBlock::operator==(const SkUniformDataBlock& other) const { - if (fUniformData.size() != other.fUniformData.size()) { - return false; - } - - for (size_t i = 0; i < fUniformData.size(); ++i) { - if (*fUniformData[i] != *other.fUniformData[i]) { - return false; - } - } - - return true; + return std::make_unique(SkSpan(newMem, other.size()), true); } uint32_t SkUniformDataBlock::hash() const { - uint32_t hash = 0; - - for (auto& u : fUniformData) { - hash = SkOpts::hash_fn(u->data(), u->dataSize(), hash); - } - - return hash; + return SkOpts::hash_fn(fData.data(), fData.size(), 0); } //////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkPipelineData.h b/src/core/SkPipelineData.h index a8de10dde5..9e8044d842 100644 --- a/src/core/SkPipelineData.h +++ b/src/core/SkPipelineData.h @@ -9,54 +9,52 @@ #define SkPipelineData_DEFINED #include +#include "include/core/SkPoint.h" #include "include/core/SkRefCnt.h" #include "include/core/SkSamplingOptions.h" +#include "include/core/SkSpan.h" #include "include/core/SkTileMode.h" #include "include/private/SkColorData.h" -#include "src/core/SkUniformData.h" #ifdef SK_GRAPHITE_ENABLED #include "experimental/graphite/src/TextureProxy.h" #include "experimental/graphite/src/UniformManager.h" +#include "experimental/graphite/src/geom/VectorTypes.h" #include "src/gpu/Blend.h" #endif class SkArenaAlloc; +class SkUniform; class SkUniformDataBlock { public: static std::unique_ptr Make(const SkUniformDataBlock&, SkArenaAlloc*); + SkUniformDataBlock(SkSpan data, bool ownMem) : fData(data), fOwnMem(ownMem) {} SkUniformDataBlock() = default; - SkUniformDataBlock(sk_sp initial) { - SkASSERT(initial && initial->dataSize()); - fUniformData.push_back(std::move(initial)); + ~SkUniformDataBlock() { + if (fOwnMem) { + delete [] fData.data(); + } } - bool empty() const { return fUniformData.empty(); } - size_t totalUniformSize() const; // TODO: cache this? + const char* data() const { return fData.data(); } + size_t size() const { return fData.size(); } - bool operator==(const SkUniformDataBlock&) const; - bool operator!=(const SkUniformDataBlock& other) const { return !(*this == other); } uint32_t hash() const; - using container = std::vector>; - using iterator = container::iterator; - - inline iterator begin() noexcept { return fUniformData.begin(); } - inline iterator end() noexcept { return fUniformData.end(); } - - void add(sk_sp ud) { - fUniformData.push_back(std::move(ud)); - } - - void reset() { - fUniformData.clear(); + bool operator==(const SkUniformDataBlock& that) const { + return fData.size() == that.fData.size() && + !memcmp(fData.data(), that.fData.data(), fData.size()); } + bool operator!=(const SkUniformDataBlock& that) const { return !(*this == that); } private: - // TODO: SkUniformData should be held uniquely - std::vector> fUniformData; + SkSpan fData; + + // This is only required until the uniform data is stored in the arena. Once there this + // class will never delete the data referenced w/in the span + bool fOwnMem = false; }; #ifdef SK_GRAPHITE_ENABLED @@ -130,7 +128,7 @@ public: #endif #ifdef SK_GRAPHITE_ENABLED - SkPipelineDataGatherer(skgpu::Layout layout) : fLayout(layout) {} + SkPipelineDataGatherer(skgpu::Layout layout) : fUniformManager(layout) {} #endif void reset(); @@ -138,8 +136,6 @@ public: SkDEBUGCODE(void checkReset();) #ifdef SK_GRAPHITE_ENABLED - skgpu::Layout layout() const { return fLayout; } - void setBlendInfo(const SkPipelineDataGatherer::BlendInfo& blendInfo) { fBlendInfo = blendInfo; } @@ -153,21 +149,32 @@ public: bool hasTextures() const { return !fTextureDataBlock.empty(); } const SkTextureDataBlock& textureDataBlock() { return fTextureDataBlock; } -#endif - void add(sk_sp); - bool hasUniforms() const { return !fUniformDataBlock.empty(); } +#ifdef SK_DEBUG + void setExpectedUniforms(SkSpan expectedUniforms) { + fUniformManager.setExpectedUniforms(expectedUniforms); + } + void doneWithExpectedUniforms() { fUniformManager.doneWithExpectedUniforms(); } +#endif // SK_DEBUG - SkUniformDataBlock& uniformDataBlock() { return fUniformDataBlock; } + void write(const SkColor4f* colors, int numColors) { fUniformManager.write(colors, numColors); } + void write(const SkPMColor4f& premulColor) { fUniformManager.write(&premulColor, 1); } + void write(const SkRect& rect) { fUniformManager.write(rect); } + void write(SkPoint point) { fUniformManager.write(point); } + void write(const float* floats, int count) { fUniformManager.write(floats, count); } + void write(float something) { fUniformManager.write(&something, 1); } + void write(int something) { fUniformManager.write(something); } + void write(skgpu::float2 something) { fUniformManager.write(something); } + + bool hasUniforms() const { return fUniformManager.size(); } + + SkUniformDataBlock peekUniformData() const { return fUniformManager.peekData(); } private: - SkUniformDataBlock fUniformDataBlock; - -#ifdef SK_GRAPHITE_ENABLED - SkTextureDataBlock fTextureDataBlock; - BlendInfo fBlendInfo; - skgpu::Layout fLayout; -#endif + SkTextureDataBlock fTextureDataBlock; + BlendInfo fBlendInfo; + skgpu::UniformManager fUniformManager; +#endif // SK_GRAPHITE_ENABLED }; #endif // SkPipelineData_DEFINED diff --git a/src/core/SkUniformData.cpp b/src/core/SkUniformData.cpp deleted file mode 100644 index 874cd0d8ea..0000000000 --- a/src/core/SkUniformData.cpp +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright 2022 Google LLC - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#include "src/core/SkUniformData.h" - -#include - -sk_sp SkUniformData::Make(const char* data, size_t size) { - // TODO: data should just be allocated right after UniformData in an arena - char* newData = new char[size]; - memcpy(newData, data, size); - return sk_sp(new SkUniformData(newData, size)); -} - -bool SkUniformData::operator==(const SkUniformData& other) const { - if (this->dataSize() != other.dataSize()) { - return false; - } - - return !memcmp(this->data(), other.data(), this->dataSize()); -} diff --git a/src/core/SkUniformData.h b/src/core/SkUniformData.h deleted file mode 100644 index c6684a8fa6..0000000000 --- a/src/core/SkUniformData.h +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2022 Google LLC - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef SkUniformData_DEFINED -#define SkUniformData_DEFINED - -#include "include/core/SkRefCnt.h" -#include "include/core/SkSpan.h" -#include "src/core/SkUniform.h" - -/* - * TODO: Here is the plan of record for SkUniformData - * allocate them (and their ancillary data in an arena - and rm SkRefCnt - * allow a scratch buffer to be used to collect the uniform data when processing a PaintParams - * - this can be reset for each draw and be copied into a cache if it needs to be preserved - * if possible, clear out the cache (and delete the arena) once the DrawPass is created/the - * uniforms are copied into a buffer - * - this will mean we'll get w/in DrawPass reuse but not cross-DrawPass reuse - * - we could also explore getting w/in a Recording (i.e. cross-DrawPass) and w/in a - * Recorder reuse (i.e., cross-Recordings, but single-threaded) - * have the SkUniformData's data layout match what is required by the DrawList so we can just - * copy it into the buffer - */ - -class SkUniformData : public SkRefCnt { -public: - - static sk_sp Make(const char* data, size_t size); - - ~SkUniformData() override { - // TODO: fData should just be allocated right after UniformData in an arena - delete [] fData; - } - - char* data() { return fData; } - const char* data() const { return fData; } - size_t dataSize() const { return fDataSize; } - - bool operator==(const SkUniformData&) const; - bool operator!=(const SkUniformData& other) const { return !(*this == other); } - -private: - SkUniformData(char* data, size_t dataSize) - : fData(data) - , fDataSize(dataSize) { - } - - char* fData; - const size_t fDataSize; -}; - -#endif // SkUniformData_DEFINED diff --git a/tests/graphite/BUILD.bazel b/tests/graphite/BUILD.bazel index ab52c39d95..c6c650f182 100644 --- a/tests/graphite/BUILD.bazel +++ b/tests/graphite/BUILD.bazel @@ -49,7 +49,6 @@ generated_cc_atom( "//src/core:SkKeyContext_hdr", "//src/core:SkKeyHelpers_hdr", "//src/core:SkShaderCodeDictionary_hdr", - "//src/core:SkUniformData_hdr", "//tests:Test_hdr", ], ) @@ -136,7 +135,6 @@ generated_cc_atom( "//src/core:SkKeyHelpers_hdr", "//src/core:SkPipelineData_hdr", "//src/core:SkShaderCodeDictionary_hdr", - "//src/core:SkUniformData_hdr", "//tests:Test_hdr", ], ) diff --git a/tests/graphite/CommandBufferTest.cpp b/tests/graphite/CommandBufferTest.cpp index fa82c0d6c6..a2b3cbed2c 100644 --- a/tests/graphite/CommandBufferTest.cpp +++ b/tests/graphite/CommandBufferTest.cpp @@ -32,7 +32,6 @@ #include "src/core/SkKeyContext.h" #include "src/core/SkKeyHelpers.h" #include "src/core/SkShaderCodeDictionary.h" -#include "src/core/SkUniformData.h" #if GRAPHITE_TEST_UTILS // set to 1 if you want to do GPU capture of the commandBuffer @@ -87,15 +86,10 @@ public: // TODO: A << API for uniforms would be nice, particularly if it could take pre-computed // offsets for each uniform. - skgpu::UniformManager mgr(gatherer->layout()); - - SkDEBUGCODE(mgr.setExpectedUniforms(SkMakeSpan(kRectUniforms, kNumRectUniforms));) - mgr.write(geom.shape().rect().size()); - mgr.write(geom.shape().rect().topLeft()); - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(SkMakeSpan(kRectUniforms, kNumRectUniforms));) + gatherer->write(geom.shape().rect().size()); + gatherer->write(geom.shape().rect().topLeft()); + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) } private: @@ -154,15 +148,10 @@ public: }; #endif - skgpu::UniformManager mgr(gatherer->layout()); - - SkDEBUGCODE(mgr.setExpectedUniforms(SkMakeSpan(kRectUniforms, kNumRectUniforms));) - mgr.write(SkPoint::Make(2.0f, 2.0f)); - mgr.write(SkPoint::Make(-1.0f, -1.0f)); - SkDEBUGCODE(mgr.doneWithExpectedUniforms();) - - sk_sp result = mgr.createUniformData(); - gatherer->add(std::move(result)); + SkDEBUGCODE(gatherer->setExpectedUniforms(SkMakeSpan(kRectUniforms, kNumRectUniforms));) + gatherer->write(SkPoint::Make(2.0f, 2.0f)); + gatherer->write(SkPoint::Make(-1.0f, -1.0f)); + SkDEBUGCODE(gatherer->doneWithExpectedUniforms();) } private: @@ -340,12 +329,9 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(CommandBufferTest, reporter, context) { SkDEBUGCODE(gatherer.checkReset()); step->writeUniforms(geom, &gatherer); if (gatherer.hasUniforms()) { - SkUniformDataBlock* renderStepUniforms = &gatherer.uniformDataBlock(); - auto [writer, bindInfo] = - bufferMgr.getUniformWriter(renderStepUniforms->totalUniformSize()); - for (const auto &u : *renderStepUniforms) { - writer.write(u->data(), u->dataSize()); - } + SkUniformDataBlock renderStepUniforms = gatherer.peekUniformData(); + auto [writer, bindInfo] = bufferMgr.getUniformWriter(renderStepUniforms.size()); + writer.write(renderStepUniforms.data(), renderStepUniforms.size()); commandBuffer->bindUniformBuffer(UniformSlot::kRenderStep, sk_ref_sp(bindInfo.fBuffer), bindInfo.fOffset); diff --git a/tests/graphite/PipelineDataCacheTest.cpp b/tests/graphite/PipelineDataCacheTest.cpp index ccf309ca80..e6cfaa61dc 100644 --- a/tests/graphite/PipelineDataCacheTest.cpp +++ b/tests/graphite/PipelineDataCacheTest.cpp @@ -19,8 +19,7 @@ using namespace skgpu; namespace { std::unique_ptr make_udb(const char* data, size_t size) { - sk_sp ud = SkUniformData::Make(data, size); - return std::make_unique(std::move(ud)); + return std::make_unique(SkMakeSpan(data, size), false); } } // anonymous namespace diff --git a/tests/graphite/UniformTest.cpp b/tests/graphite/UniformTest.cpp index 84749d64c4..009635f313 100644 --- a/tests/graphite/UniformTest.cpp +++ b/tests/graphite/UniformTest.cpp @@ -21,7 +21,6 @@ #include "src/core/SkKeyHelpers.h" #include "src/core/SkPipelineData.h" #include "src/core/SkShaderCodeDictionary.h" -#include "src/core/SkUniformData.h" namespace {