[graphite] Add geometry uniforms to RenderStep
RenderSteps expose the uniforms they use for their portion of the
program, and assume will be bound independently of the other uniform
sets needed for the paint, etc.
This wires them up in DrawPass, although the RenderSteps currently don't
define any. At the moment this CL is really just laying out the API and
then it will be tested more thoroughly in a follow up when
CommandBufferTest moves its hard-coded test shaders into the RenderStep
paradigm.
Bug: skia:12466
Change-Id: I8287dadb9a458127ba9f376ba351ca1932788c71
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474137
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
diff --git a/experimental/graphite/src/DrawPass.cpp b/experimental/graphite/src/DrawPass.cpp
index efbd516..a54d506 100644
--- a/experimental/graphite/src/DrawPass.cpp
+++ b/experimental/graphite/src/DrawPass.cpp
@@ -20,6 +20,7 @@
#include "experimental/graphite/src/Renderer.h"
#include "experimental/graphite/src/TextureProxy.h"
#include "experimental/graphite/src/UniformCache.h"
+#include "experimental/graphite/src/UniformManager.h"
#include "experimental/graphite/src/geom/BoundsManager.h"
#include "src/core/SkMathPriv.h"
@@ -209,6 +210,10 @@
bool requiresMSAA = false;
Rect passBounds = Rect::InfiniteInverted();
+ DrawBufferManager* bufferMgr = recorder->drawBufferManager();
+ UniformCache geometryUniforms;
+ std::unordered_map<uint32_t, BindBufferInfo> geometryUniformBindings;
+
std::vector<SortKey> keys;
keys.reserve(draws->renderStepCount()); // will not exceed but may use less with occluded draws
@@ -234,9 +239,20 @@
// TODO ask step to generate a pipeline description based on the above shading code, and
// have pipelineIndex point to that description in the accumulated list of descs
uint32_t pipelineIndex = 0;
- // TODO step writes out geometry uniforms and have geomIndex point to that buffer data,
- // providing shape, transform, scissor, and paint depth to RenderStep
- uint32_t geometryIndex = 0;
+
+ uint32_t geometryIndex = UniformCache::kInvalidUniformID;
+ if (step->numUniforms() > 0) {
+ // TODO: Get layout from the GPU
+ sk_sp<UniformData> uniforms = step->writeUniforms(Layout::kMetal, draw.fShape);
+ geometryIndex = geometryUniforms.insert(uniforms);
+
+ // Upload the data to the GPU if it's the first time encountered
+ if (geometryUniformBindings.find(geometryIndex) == geometryUniformBindings.end()) {
+ auto [writer, bufferInfo] = bufferMgr->getUniformWriter(uniforms->dataSize());
+ writer.write(uniforms->data(), uniforms->dataSize());
+ geometryUniformBindings.insert({geometryIndex, bufferInfo});
+ }
+ }
uint32_t shadingIndex = UniformCache::kInvalidUniformID;
@@ -267,8 +283,6 @@
// bugs in the DrawOrder determination code?
std::sort(keys.begin(), keys.end());
- DrawBufferManager* bufferMgr = recorder->drawBufferManager();
-
// Used to record vertex/instance data, buffer binds, and draw calls
Drawer drawer;
DrawWriter drawWriter(&drawer, bufferMgr);
@@ -307,10 +321,17 @@
}
if (stateChange) {
if (key.geometryUniforms() != lastGeometryUniforms) {
- // TODO: Look up uniform buffer binding info corresponding to key's index and record
+ if (key.geometryUniforms() != UniformCache::kInvalidUniformID) {
+ auto binding = geometryUniformBindings.find(key.geometryUniforms())->second;
+ // TODO: Record bind 'binding' buffer + offset to kRenderStep slot
+ (void) binding;
+ }
lastGeometryUniforms = key.geometryUniforms();
}
if (key.shadingUniforms() != lastShadingUniforms) {
+ // TODO: We should not re-upload the uniforms for every draw that referenced them,
+ // they should be uploaded first time seen in the earlier loop of DrawPass::Make and
+ // then this can lookup the cached BindBufferInfo, similar to geometryUniformBinding
auto ud = lookup(recorder, key.shadingUniforms());
auto [writer, bufferInfo] = bufferMgr->getUniformWriter(ud->dataSize());
diff --git a/experimental/graphite/src/Renderer.h b/experimental/graphite/src/Renderer.h
index 162ffd3..52cfc09 100644
--- a/experimental/graphite/src/Renderer.h
+++ b/experimental/graphite/src/Renderer.h
@@ -11,6 +11,7 @@
#include "experimental/graphite/src/Attribute.h"
#include "experimental/graphite/src/DrawTypes.h"
#include "experimental/graphite/src/EnumBitMask.h"
+#include "experimental/graphite/src/Uniform.h"
#include "include/core/SkSpan.h"
#include "include/core/SkString.h"
@@ -25,7 +26,9 @@
class DrawWriter;
class ResourceProvider;
class Shape;
-class Uniform;
+class UniformData;
+
+enum class Layout;
class RenderStep {
public:
@@ -36,6 +39,17 @@
// this RenderStep.
virtual void writeVertices(DrawWriter*, const Shape&) const = 0;
+ // Write out the uniform values (aligned for the layout). These values will be de-duplicated
+ // across all draws using the RenderStep before uploading to the GPU, but it can be assumed the
+ // uniforms will be bound before the draws recorded in 'writeVertices' are executed.
+ // TODO: We definitely want this to return CPU memory since it's better for the caller to handle
+ // the de-duplication and GPU upload/binding (DrawPass tracks all this). However, a RenderStep's
+ // uniforms aren't going to change, and the Layout won't change during a process, so it would be
+ // nice if we could remember the offsets for the layout/gpu and reuse them across draws.
+ // Similarly, it would be nice if this could write into reusable storage and then DrawPass or
+ // UniformCache handles making an sk_sp if we need to assign a new unique ID to the uniform data
+ virtual sk_sp<UniformData> writeUniforms(Layout layout, const Shape&) const = 0;
+
virtual const char* name() const = 0;
bool requiresStencil() const { return fFlags & Flags::kRequiresStencil; }
@@ -46,9 +60,13 @@
size_t vertexStride() const { return fVertexStride; }
size_t instanceStride() const { return fInstanceStride; }
+ size_t numUniforms() const { return fUniforms.size(); }
size_t numVertexAttributes() const { return fVertexAttrs.size(); }
size_t numInstanceAttributes() const { return fInstanceAttrs.size(); }
+ // The uniforms of a RenderStep are bound to the kRenderStep slot, the rest of the pipeline
+ // may still use uniforms bound to other slots.
+ SkSpan<const Uniform> uniforms() const { return SkMakeSpan(fUniforms); }
SkSpan<const Attribute> vertexAttributes() const { return SkMakeSpan(fVertexAttrs); }
SkSpan<const Attribute> instanceAttributes() const { return SkMakeSpan(fInstanceAttrs); }
@@ -74,11 +92,14 @@
// While RenderStep does not define the full program that's run for a draw, it defines the
// entire vertex layout of the pipeline. This is not allowed to change, so can be provided to
// the RenderStep constructor by subclasses.
- RenderStep(Mask<Flags> flags, PrimitiveType primitiveType,
+ RenderStep(Mask<Flags> flags,
+ std::initializer_list<Uniform> uniforms,
+ PrimitiveType primitiveType,
std::initializer_list<Attribute> vertexAttrs,
std::initializer_list<Attribute> instanceAttrs)
: fFlags(flags)
, fPrimitiveType(primitiveType)
+ , fUniforms(uniforms)
, fVertexAttrs(vertexAttrs)
, fInstanceAttrs(instanceAttrs)
, fVertexStride(0)
@@ -105,6 +126,7 @@
// could just have this be std::array and keep all attributes inline with the RenderStep memory.
// On the other hand, the attributes are only needed when creating a new pipeline so it's not
// that performance sensitive.
+ std::vector<Uniform> fUniforms;
std::vector<Attribute> fVertexAttrs;
std::vector<Attribute> fInstanceAttrs;
diff --git a/experimental/graphite/src/render/StencilAndFillPathRenderer.cpp b/experimental/graphite/src/render/StencilAndFillPathRenderer.cpp
index 20cae66..a855804 100644
--- a/experimental/graphite/src/render/StencilAndFillPathRenderer.cpp
+++ b/experimental/graphite/src/render/StencilAndFillPathRenderer.cpp
@@ -7,7 +7,9 @@
#include "experimental/graphite/src/Renderer.h"
+#include "experimental/graphite/src/ContextUtils.h"
#include "experimental/graphite/src/DrawWriter.h"
+#include "experimental/graphite/src/UniformManager.h"
#include "experimental/graphite/src/geom/Shape.h"
#include "src/gpu/BufferWriter.h"
@@ -58,7 +60,9 @@
// TODO: Will need to add kRequiresStencil when we support specifying stencil settings and
// the Renderer includes the stenciling step first.
FillBoundsRenderStep()
- : RenderStep(Flags::kPerformsShading, PrimitiveType::kTriangleStrip,
+ : RenderStep(Flags::kPerformsShading,
+ /*uniforms=*/{},
+ PrimitiveType::kTriangleStrip,
/*vertexAttrs=*/{{"position", VertexAttribType::kFloat2, SLType::kFloat2}},
/*instanceAttrs=*/{}) {}
@@ -77,6 +81,13 @@
// the DrawWriter to support appending both vertex and instance data simultaneously, which
// would need to return 2 vertex writers?
}
+
+ sk_sp<UniformData> writeUniforms(Layout layout, const Shape&) const override {
+ // TODO: Return the uniform data that is needed for this draw, but since there are no
+ // declared uniforms right now, just return nullptr. Eventually, the uniforms should include
+ // the draw's transform (at least until we use storage buffers).
+ return nullptr;
+ }
};
} // anonymous namespace
