diff --git a/experimental/graphite/src/CommandBuffer.cpp b/experimental/graphite/src/CommandBuffer.cpp index 0f6a98dc9b..4bd5b18d53 100644 --- a/experimental/graphite/src/CommandBuffer.cpp +++ b/experimental/graphite/src/CommandBuffer.cpp @@ -71,6 +71,14 @@ void CommandBuffer::bindIndexBuffer(sk_sp indexBuffer, size_t bufferOffs fHasWork = true; } +void CommandBuffer::bindDrawBuffers(BindBufferInfo vertices, + BindBufferInfo instances, + BindBufferInfo indices) { + this->bindVertexBuffers(sk_ref_sp(vertices.fBuffer), vertices.fOffset, + sk_ref_sp(instances.fBuffer), instances.fOffset); + this->bindIndexBuffer(sk_ref_sp(indices.fBuffer), indices.fOffset); +} + static bool check_max_blit_width(int widthInPixels) { if (widthInPixels > 32767) { SkASSERT(false); // surfaces should not be this wide anyway diff --git a/experimental/graphite/src/CommandBuffer.h b/experimental/graphite/src/CommandBuffer.h index 926cd5b9df..7db5eda3ee 100644 --- a/experimental/graphite/src/CommandBuffer.h +++ b/experimental/graphite/src/CommandBuffer.h @@ -9,6 +9,7 @@ #define skgpu_CommandBuffer_DEFINED #include "experimental/graphite/src/DrawTypes.h" +#include "experimental/graphite/src/DrawWriter.h" #include "include/core/SkColor.h" #include "include/core/SkRect.h" #include "include/core/SkRefCnt.h" @@ -53,7 +54,7 @@ struct RenderPassDesc { // * input attachments }; -class CommandBuffer : public SkRefCnt { +class CommandBuffer : public SkRefCnt, private DrawDispatcher { public: ~CommandBuffer() override { this->releaseResources(); @@ -73,9 +74,10 @@ public: //--------------------------------------------------------------- void bindGraphicsPipeline(sk_sp graphicsPipeline); void bindUniformBuffer(UniformSlot, sk_sp, size_t bufferOffset); - void bindVertexBuffers(sk_sp vertexBuffer, size_t vertexOffset, - sk_sp instanceBuffer, size_t instanceOffset); - void bindIndexBuffer(sk_sp indexBuffer, size_t bufferOffset); + + void bindDrawBuffers(BindBufferInfo vertices, + BindBufferInfo instances, + BindBufferInfo indices) final; // TODO: do we want to handle multiple scissor rects and viewports? void setScissor(unsigned int left, unsigned int top, unsigned int width, unsigned int height) { @@ -100,28 +102,33 @@ public: fHasWork = true; } - void draw(PrimitiveType type, unsigned int baseVertex, unsigned int vertexCount) { + void draw(PrimitiveType type, unsigned int baseVertex, unsigned int vertexCount) final { this->onDraw(type, baseVertex, vertexCount); fHasWork = true; } void drawIndexed(PrimitiveType type, unsigned int baseIndex, unsigned int indexCount, - unsigned int baseVertex) { + unsigned int baseVertex) final { this->onDrawIndexed(type, baseIndex, indexCount, baseVertex); fHasWork = true; } void drawInstanced(PrimitiveType type, unsigned int baseVertex, unsigned int vertexCount, - unsigned int baseInstance, unsigned int instanceCount) { + unsigned int baseInstance, unsigned int instanceCount) final { this->onDrawInstanced(type, baseVertex, vertexCount, baseInstance, instanceCount); fHasWork = true; } void drawIndexedInstanced(PrimitiveType type, unsigned int baseIndex, unsigned int indexCount, unsigned int baseVertex, unsigned int baseInstance, - unsigned int instanceCount) { + unsigned int instanceCount) final { this->onDrawIndexedInstanced(type, baseIndex, indexCount, baseVertex, baseInstance, instanceCount); fHasWork = true; } + // When using a DrawWriter dispatching directly to a CommandBuffer, binding of pipelines and + // uniforms must be coordinated with forNewPipeline() and forDynamicStateChange(). The direct + // draw calls and vertex buffer binding calls on CB should not be intermingled with the writer. + DrawDispatcher* asDrawDispatcher() { return this; } + //--------------------------------------------------------------- // Can only be used outside renderpasses //--------------------------------------------------------------- @@ -137,6 +144,13 @@ protected: private: void releaseResources(); + // TODO: Once all buffer use goes through the DrawBufferManager, we likely do not need to track + // refs every time a buffer is bound, since the DBM will transfer ownership for any used buffer + // to the CommandBuffer. + void bindVertexBuffers(sk_sp vertexBuffer, size_t vertexOffset, + sk_sp instanceBuffer, size_t instanceOffset); + void bindIndexBuffer(sk_sp indexBuffer, size_t bufferOffset); + virtual void onBeginRenderPass(const RenderPassDesc&) = 0; virtual void onBindGraphicsPipeline(const GraphicsPipeline*) = 0; diff --git a/experimental/graphite/src/DrawList.cpp b/experimental/graphite/src/DrawList.cpp index 43a3ca84b5..e69928bacd 100644 --- a/experimental/graphite/src/DrawList.cpp +++ b/experimental/graphite/src/DrawList.cpp @@ -84,23 +84,4 @@ void DrawList::strokePath(const Transform& localToDevice, // fRenderStepCount += Renderer::StrokePath().numRenderSteps(); } -size_t DrawList::Draw::requiredVertexSpace(int renderStep) const { - SkASSERT(renderStep < fRenderer.numRenderSteps()); - return fRenderer.steps()[renderStep]->requiredVertexSpace(fShape); -} - -size_t DrawList::Draw::requiredIndexSpace(int renderStep) const { - SkASSERT(renderStep < fRenderer.numRenderSteps()); - return fRenderer.steps()[renderStep]->requiredIndexSpace(fShape); -} - -void DrawList::Draw::writeVertices(VertexWriter vertexWriter, - IndexWriter indexWriter, - int renderStep) const { - SkASSERT(renderStep < fRenderer.numRenderSteps()); - fRenderer.steps()[renderStep]->writeVertices(std::move(vertexWriter), - std::move(indexWriter), - fShape); -} - } // namespace skgpu diff --git a/experimental/graphite/src/DrawList.h b/experimental/graphite/src/DrawList.h index 08a3f7a84b..f0d2144ba5 100644 --- a/experimental/graphite/src/DrawList.h +++ b/experimental/graphite/src/DrawList.h @@ -25,9 +25,7 @@ struct SkIRect; namespace skgpu { -struct IndexWriter; class Renderer; -struct VertexWriter; // TBD: If occlusion culling is eliminated as a phase, we can easily move the paint conversion // back to Device when the command is recorded (similar to SkPaint -> GrPaint), and then @@ -216,11 +214,6 @@ private: , fOrder(order) , fPaintParams(paint ? skstd::optional(*paint) : skstd::nullopt) , fStrokeParams(stroke ? skstd::optional(*stroke) : skstd::nullopt) {} - - size_t requiredVertexSpace(int renderStep) const; - size_t requiredIndexSpace(int renderStep) const; - - void writeVertices(VertexWriter, IndexWriter, int renderStep) const; }; // The returned Transform reference remains valid for the lifetime of the DrawList. diff --git a/experimental/graphite/src/DrawPass.cpp b/experimental/graphite/src/DrawPass.cpp index 0997a191e2..95016d83f5 100644 --- a/experimental/graphite/src/DrawPass.cpp +++ b/experimental/graphite/src/DrawPass.cpp @@ -9,10 +9,12 @@ #include "experimental/graphite/include/GraphiteTypes.h" #include "experimental/graphite/src/Buffer.h" +#include "experimental/graphite/src/ContextPriv.h" #include "experimental/graphite/src/ContextUtils.h" #include "experimental/graphite/src/DrawBufferManager.h" #include "experimental/graphite/src/DrawContext.h" #include "experimental/graphite/src/DrawList.h" +#include "experimental/graphite/src/DrawWriter.h" #include "experimental/graphite/src/ProgramCache.h" #include "experimental/graphite/src/Recorder.h" #include "experimental/graphite/src/Renderer.h" @@ -78,6 +80,7 @@ public: pipelineIndex} , fUniformKey{geomUniformIndex, shadingUniformIndex} , fDraw(draw) { + SkASSERT(renderStep <= draw->fRenderer.numRenderSteps()); } bool operator<(const SortKey& k) const { @@ -88,7 +91,9 @@ public: const DrawList::Draw* draw() const { return fDraw; } uint32_t pipeline() const { return fPipelineKey.fPipeline; } - int renderStep() const { return static_cast(fPipelineKey.fRenderStep); } + const RenderStep& renderStep() const { + return *fDraw->fRenderer.steps()[fPipelineKey.fRenderStep]; + } uint32_t geometryUniforms() const { return fUniformKey.fGeometryIndex; } uint32_t shadingUniforms() const { return fUniformKey.fShadingIndex; } @@ -120,6 +125,40 @@ private: static_assert(30 >= SkNextLog2_portable(Renderer::kMaxRenderSteps * DrawList::kMaxDraws)); }; +class DrawPass::Drawer final : public DrawDispatcher { +public: + Drawer() {} + ~Drawer() override = default; + + void bindDrawBuffers(BindBufferInfo vertexAttribs, + BindBufferInfo instanceAttribs, + BindBufferInfo indices) override { + // TODO: Actually record bind vertex buffers struct into DrawPass' command list + } + + void draw(PrimitiveType type, unsigned int baseVertex, unsigned int vertexCount) override { + // TODO: Actually record draw struct into DrawPass' command list + } + + void drawIndexed(PrimitiveType type, unsigned int baseIndex, + unsigned int indexCount, unsigned int baseVertex) override { + // TODO: Actually record draw struct into DrawPass' command list + } + + void drawInstanced(PrimitiveType type, + unsigned int baseVertex, unsigned int vertexCount, + unsigned int baseInstance, unsigned int instanceCount) override { + // TODO: Actually record draw struct into DrawPass' command list + } + + void drawIndexedInstanced(PrimitiveType type, + unsigned int baseIndex, unsigned int indexCount, + unsigned int baseVertex, unsigned int baseInstance, + unsigned int instanceCount) override { + // TODO: Actually record draw struct into DrawPass' command list + } +}; + /////////////////////////////////////////////////////////////////////////////////////////////////// namespace { @@ -230,79 +269,65 @@ std::unique_ptr DrawPass::Make(Recorder* recorder, DrawBufferManager* bufferMgr = recorder->drawBufferManager(); + // Used to record vertex/instance data, buffer binds, and draw calls + Drawer drawer; + DrawWriter drawWriter(&drawer, bufferMgr); + + // Used to track when a new pipeline or dynamic state needs recording between draw steps uint32_t lastPipeline = 0; uint32_t lastShadingUniforms = UniformData::kInvalidUniformID; uint32_t lastGeometryUniforms = 0; SkIRect lastScissor = SkIRect::MakeSize(target->dimensions()); - Buffer* lastBoundVertexBuffer = nullptr; - Buffer* lastBoundIndexBuffer = nullptr; for (const SortKey& key : keys) { const DrawList::Draw& draw = *key.draw(); - int renderStep = key.renderStep(); + const RenderStep& renderStep = key.renderStep(); - size_t vertexSize = draw.requiredVertexSpace(renderStep); - size_t indexSize = draw.requiredIndexSpace(renderStep); - auto [vertexWriter, vertexInfo] = bufferMgr->getVertexWriter(vertexSize); - auto [indexWriter, indexInfo] = bufferMgr->getIndexWriter(indexSize); - // TODO: handle the case where we fail to get a vertex or index writer besides asserting - SkASSERT(!vertexSize || (vertexWriter && vertexInfo.fBuffer)); - SkASSERT(!indexSize || (indexWriter && indexInfo.fBuffer)); - draw.writeVertices(std::move(vertexWriter), std::move(indexWriter), renderStep); + const bool pipelineChange = key.pipeline() != lastPipeline; + const bool stateChange = key.geometryUniforms() != lastGeometryUniforms || + key.shadingUniforms() != lastShadingUniforms || + draw.fClip.scissor() != lastScissor; - if (vertexSize) { - if (lastBoundVertexBuffer != vertexInfo.fBuffer) { - // TODO: Record a vertex bind call that stores the vertexInfo.fBuffer. - } - // TODO: Store the vertexInfo.fOffset so the draw will know its vertex offset when it - // executes. - } - if (indexSize) { - if (lastBoundIndexBuffer != indexInfo.fBuffer) { - // TODO: Record a vertex bind call that stores the vertexInfo.fBuffer. - } - // TODO: Store the vertexInfo.fOffset so the draw will know its vertex offset when it - // executes. + // Update DrawWriter *before* we actually change any state so that accumulated draws from + // the previous state use the proper state. + if (pipelineChange) { + drawWriter.newPipelineState(renderStep.primitiveType(), + renderStep.vertexStride(), + renderStep.instanceStride()); + } else if (stateChange) { + drawWriter.newDynamicState(); } - // TODO: Have the render step write out vertices and figure out what draw call function and - // primitive type it uses. The vertex buffer binding/offset and draw params will be examined - // to determine if the active draw can be updated to include the new vertices, or if it has - // to be ended and a new one begun for this step. In addition to checking this state, must - // also check if pipeline, uniform, scissor etc. would require the active draw to end. - // - // const RenderStep* const step = draw.fRenderer.steps()[key.renderStep()]; - - if (key.pipeline() != lastPipeline) { + // Make state changes before accumulating new draw data + if (pipelineChange) { // TODO: Look up pipeline description from key's index and record binding it lastPipeline = key.pipeline(); lastShadingUniforms = UniformData::kInvalidUniformID; lastGeometryUniforms = 0; } - if (key.geometryUniforms() != lastGeometryUniforms) { - // TODO: Look up uniform buffer binding info corresponding to key's index and record it - lastGeometryUniforms = key.geometryUniforms(); - } - if (key.shadingUniforms() != lastShadingUniforms) { - auto ud = lookup(recorder, key.shadingUniforms()); + if (stateChange) { + if (key.geometryUniforms() != lastGeometryUniforms) { + // TODO: Look up uniform buffer binding info corresponding to key's index and record + lastGeometryUniforms = key.geometryUniforms(); + } + if (key.shadingUniforms() != lastShadingUniforms) { + auto ud = lookup(recorder, key.shadingUniforms()); - auto [writer, bufferInfo] = bufferMgr->getUniformWriter(ud->dataSize()); - writer.write(ud->data(), ud->dataSize()); - // TODO: recording 'bufferInfo' somewhere to allow a later uniform bind call + auto [writer, bufferInfo] = bufferMgr->getUniformWriter(ud->dataSize()); + writer.write(ud->data(), ud->dataSize()); + // TODO: recording 'bufferInfo' somewhere to allow a later uniform bind call - lastShadingUniforms = key.shadingUniforms(); + lastShadingUniforms = key.shadingUniforms(); + } + if (draw.fClip.scissor() != lastScissor) { + // TODO: Record new scissor rectangle + } } - if (draw.fClip.scissor() != lastScissor) { - // TODO: Record new scissor rectangle - } - - // TODO: Write vertex and index data for the draw step + renderStep.writeVertices(&drawWriter, draw.fShape); } - - // if (currentDraw) { - // TODO: End the current draw if it has pending vertices - // } + // Finish recording draw calls for any collected data at the end of the loop + drawWriter.flush(); passBounds.roundOut(); SkIRect pxPassBounds = SkIRect::MakeLTRB((int) passBounds.left(), (int) passBounds.top(), diff --git a/experimental/graphite/src/DrawPass.h b/experimental/graphite/src/DrawPass.h index 3ee1487941..4b6c27ebd8 100644 --- a/experimental/graphite/src/DrawPass.h +++ b/experimental/graphite/src/DrawPass.h @@ -74,6 +74,7 @@ public: private: class SortKey; + class Drawer; DrawPass(sk_sp target, const SkIRect& bounds, diff --git a/experimental/graphite/src/DrawTypes.h b/experimental/graphite/src/DrawTypes.h index a14c95a136..3e9992c033 100644 --- a/experimental/graphite/src/DrawTypes.h +++ b/experimental/graphite/src/DrawTypes.h @@ -215,8 +215,10 @@ static constexpr inline size_t VertexAttribTypeSize(VertexAttribType type) { * CommandBuffer. */ struct BindBufferInfo { - Buffer* fBuffer = nullptr; + const Buffer* fBuffer = nullptr; size_t fOffset = 0; + + operator bool() const { return SkToBool(fBuffer); } }; }; // namespace skgpu diff --git a/experimental/graphite/src/DrawWriter.cpp b/experimental/graphite/src/DrawWriter.cpp new file mode 100644 index 0000000000..5da16658b6 --- /dev/null +++ b/experimental/graphite/src/DrawWriter.cpp @@ -0,0 +1,164 @@ +/* + * Copyright 2021 Google LLC + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "experimental/graphite/src/DrawWriter.h" + +#include "experimental/graphite/src/DrawBufferManager.h" +#include "src/gpu/BufferWriter.h" + +namespace skgpu { + +DrawWriter::DrawWriter(DrawDispatcher* dispatcher, DrawBufferManager* bufferManager) + : DrawWriter(dispatcher, bufferManager, PrimitiveType::kTriangles, 0, 0) {} + +DrawWriter::DrawWriter(DrawDispatcher* dispatcher, + DrawBufferManager* bufferManager, + PrimitiveType primitiveType, + size_t vertexStride, + size_t instanceStride) + : fDispatcher(dispatcher) + , fManager(bufferManager) + , fPrimitiveType(primitiveType) + , fVertexStride(vertexStride) + , fInstanceStride(instanceStride) { + SkASSERT(dispatcher && bufferManager); +} + +void DrawWriter::setTemplateInternal(BindBufferInfo vertices, + BindBufferInfo indices, + unsigned int count, + bool drawPendingVertices) { + SkASSERT(!vertices || fVertexStride > 0); + if (vertices != fFixedVertexBuffer || + indices != fFixedIndexBuffer || + count != fFixedVertexCount) { + // Issue any accumulated data that referred to the old template. + if (drawPendingVertices) { + this->drawPendingVertices(); + } + + fFixedBuffersDirty = true; + + fFixedVertexBuffer = vertices; + fFixedIndexBuffer = indices; + fFixedVertexCount = count; + } +} + +void DrawWriter::drawInternal(BindBufferInfo instances, + unsigned int base, + unsigned int instanceCount) { + // Draw calls that are only 1 instance and have no extra instance data get routed to + // the simpler draw APIs. + // TODO: Is there any benefit to this? Does it help hint to drivers? Avoid more bugs? + // Or should we always call drawInstanced and drawIndexedInstanced? + const bool useNonInstancedDraw = + !SkToBool(instances) && base == 0 && instanceCount == 1; + SkASSERT(!useNonInstancedDraw || fInstanceStride == 0); + + // Issue new buffer binds only as necessary + // TODO: Should this instead be the responsibility of the CB or DrawDispatcher to remember + // what was last bound? + if (fFixedBuffersDirty || instances != fLastInstanceBuffer) { + fDispatcher->bindDrawBuffers(fFixedVertexBuffer, instances, fFixedIndexBuffer); + fFixedBuffersDirty = false; + fLastInstanceBuffer = instances; + } + + if (useNonInstancedDraw) { + if (fFixedIndexBuffer) { + // Should only get here from a direct draw, in which case base should be 0 and any + // offset needs to be embedded in the BindBufferInfo by caller. + SkASSERT(base == 0); + fDispatcher->drawIndexed(fPrimitiveType, 0, fFixedVertexCount, 0); + } else { + // 'base' offsets accumulated vertex data from another DrawWriter across a state change. + fDispatcher->draw(fPrimitiveType, base, fFixedVertexCount); + } + } else { + // 'base' offsets accumulated instance data (or is 0 for a direct instanced draw). It is + // assumed that any base vertex and index have been folded into the BindBufferInfos already. + if (fFixedIndexBuffer) { + fDispatcher->drawIndexedInstanced(fPrimitiveType, 0, fFixedVertexCount, 0, + base, instanceCount); + } else { + fDispatcher->drawInstanced(fPrimitiveType, 0, fFixedVertexCount, base, instanceCount); + } + } +} + +void DrawWriter::drawPendingVertices() { + if (fPendingCount > 0) { + if (fPendingMode == VertexMode::kInstances) { + // This uses instanced draws, so 'base' will be interpreted in instance units. + this->drawInternal(fPendingAttrs, fPendingBaseVertex, fPendingCount); + } else { + // This triggers a non-instanced draw call so 'base' passed to drawInternal is + // interpreted in vertex units. + this->setTemplateInternal(fPendingAttrs, {}, fPendingCount, /*drawPending=*/false); + this->drawInternal({}, fPendingBaseVertex, 1); + } + + fPendingCount = 0; + fPendingBaseVertex = 0; + fPendingAttrs = {}; + } +} + +VertexWriter DrawWriter::appendData(VertexMode mode, size_t stride, unsigned int count) { + if (fPendingMode != mode) { + // Switched between accumulating vertices and instances, so issue draws for the old data. + this->drawPendingVertices(); + fPendingMode = mode; + } + + auto [writer, nextChunk] = fManager->getVertexWriter(count * stride); + // Check if next chunk's data is contiguous with what's previously been appended + if (nextChunk.fBuffer == fPendingAttrs.fBuffer && + fPendingAttrs.fOffset + (fPendingBaseVertex + fPendingCount) * stride + == nextChunk.fOffset) { + // It is, so the next chunk's vertices that will be written can be folded into the next draw + fPendingCount += count; + } else { + // Alignment mismatch, or the old buffer filled up + this->drawPendingVertices(); + fPendingCount = count; + fPendingBaseVertex = 0; + fPendingAttrs = nextChunk; + } + + return std::move(writer); +} + +void DrawWriter::newDynamicState() { + // Remember where we left off after we draw, since drawPendingVertices() resets all pending data + BindBufferInfo base = fPendingAttrs; + unsigned int baseVertex = fPendingBaseVertex + fPendingCount; + // Draw anything that used the previous dynamic state + this->drawPendingVertices(); + + fPendingAttrs = base; + fPendingBaseVertex = baseVertex; +} + +void DrawWriter::newPipelineState(PrimitiveType type, + size_t vertexStride, + size_t instanceStride) { + // Draw anything that used the previous pipeline + this->drawPendingVertices(); + + // For simplicity, if there's a new pipeline, just forget about any previous buffer bindings, + // in which case the new writer only needs to use the dispatcher and buffer manager. + this->setTemplateInternal({}, {}, 0, false); + fLastInstanceBuffer = {}; + + fPrimitiveType = type; + fVertexStride = vertexStride; + fInstanceStride = instanceStride; +} + +} // namespace skgpu diff --git a/experimental/graphite/src/DrawWriter.h b/experimental/graphite/src/DrawWriter.h new file mode 100644 index 0000000000..4d6a4e8190 --- /dev/null +++ b/experimental/graphite/src/DrawWriter.h @@ -0,0 +1,219 @@ +/* + * Copyright 2021 Google LLC + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef skgpu_DrawWriter_DEFINED +#define skgpu_DrawWriter_DEFINED + +#include "experimental/graphite/src/DrawTypes.h" +#include "src/gpu/BufferWriter.h" + +namespace skgpu { + +class DrawBufferManager; + +class DrawDispatcher; // Forward declaration, handles virtual dispatch of binds/draws + +/** + * DrawWriter is a helper around recording draws (to a temporary buffer or directly to a + * CommandBuffer), particularly when the number of draws is not known ahead of time, or the vertex + * and instance data is computed at record time and does not have a known size. + * + * To use, construct the DrawWriter with the current pipeline layout or call newPipelineState() on + * an existing DrawWriter and then bind that matching pipeline. When other dynamic state needs to + * change between draw calls, notify the DrawWriter using newDynamicState() before recording the + * modifications. See the listing below for how to append dynamic data or draw with existing buffers + * + * CommandBuffer::draw(vertices) + * - dynamic vertex data -> DrawWriter::appendVertices(n) + * - fixed vertex data -> DrawWriter::draw(vertices, {}, vertexCount) + * + * CommandBuffer::drawIndexed(vertices, indices) + * - dynamic vertex data -> unsupported + * - fixed vertex,index data -> DrawWriter::draw(vertices, indices, indexCount) + * + * CommandBuffer::drawInstances(vertices, instances) + * - dynamic instance data + fixed vertex data -> + * DrawWriter::setInstanceTemplate(vertices, {}, vertexCount) then + * DrawWriter::appendInstances(n) + * - fixed vertex and instance data -> + * DrawWriter::setInstanceTemplate(vertices, {}, vertexCount) then + * DrawWriter::drawInstanced(instances, instanceCount) + * + * CommandBuffer::drawIndexedInstanced(vertices, indices, instances) + * - dynamic instance data + fixed vertex, index data -> + * DrawWriter::setInstanceTemplate(vertices, indices, indexCount) then + * DrawWriter::appendInstances(n) + * - fixed vertex, index, and instance data -> + * DrawWriter::setInstanceTemplate(vertices, indices, indexCount) then + * DrawWriter::drawInstanced(instances, instanceCount) + */ +class DrawWriter { +public: + // NOTE: This constructor creates a writer that has 0 vertex and instance stride, so can only + // be used to draw triangles with pipelines that rely solely on the vertex and instance ID. + DrawWriter(DrawDispatcher*, DrawBufferManager*); + + DrawWriter(DrawDispatcher*, DrawBufferManager*, + PrimitiveType type, size_t vertexStride, size_t instanceStride); + + // Cannot move or copy + DrawWriter(const DrawWriter&) = delete; + DrawWriter(DrawWriter&&) = delete; + + // flush() should be called before the writer is destroyed + ~DrawWriter() { SkASSERT(fPendingCount == 0); } + + DrawBufferManager* bufferManager() { return fManager; } + + // Notify the DrawWriter that dynamic state that does not affect the pipeline needs to be + // changed. This issues draw calls for pending vertex/instance data that referred to the old + // state, so this must be called *before* changing the dynamic state. + // + // This preserves the last bound buffers and accounts for any offsets using the base vertex or + // base instance passed to draw calls to avoid re-binding buffers unnecessarily. + void newDynamicState(); + + // Notify the DrawWriter that a new pipeline needs to be bound, providing the primitive type and + // attribute strides of that pipeline. This issues draw calls for pending data that relied on + // the old pipeline, so this must be called *before* binding the new pipeline. + void newPipelineState(PrimitiveType type, size_t vertexStride, size_t instanceStride); + + // Issue draw calls for any pending vertex and instance data collected by the writer. + void flush() { this->drawPendingVertices(); } + + // Collects new vertex data for a call to CommandBuffer::draw(). Automatically accumulates + // vertex data into a buffer, issuing draw and bind calls as needed when a new buffer is + // required, so that it is seamless to the caller. + // + // Since this accumulates vertex data (and does not use instances or indices), this overrides + // the instance template when finally drawn. + // + // This should not be used when the vertex stride is 0. + VertexWriter appendVertices(unsigned int numVertices) { + SkASSERT(fVertexStride > 0); + return this->appendData(VertexMode::kVertices, fVertexStride, numVertices); + } + + // Collects new instance data for a call to CommandBuffer::drawInstanced() or + // drawIndexedInstanced(). The specific draw call that's issued depends on the buffers passed to + // setInstanceTemplate(). If the template has a non-null index buffer, the eventual draw calls + // correspond to drawindexedInstanced(), otherwise to drawInstanced(). + // + // Like appendVertices(), this automatically manages an internal instance buffer and merges + // the appended data into as few buffer binds and draw calls as possible, while remaining + // seamless to the caller. + // + // This requires that an instance template be specified before appending instance data. However, + // the fixed vertex buffer can be null (or have a stride of 0) if the vertex shader only relies + // on the vertex ID and no other per-vertex data. + // + // This should not be used when the instance stride is 0. + VertexWriter appendInstances(unsigned int numInstances) { + SkASSERT(fInstanceStride > 0); + return this->appendData(VertexMode::kInstances, fInstanceStride, numInstances); + } + + // Set the fixed vertex and index buffers referenced when appending instance data or calling + // drawIndexed(). 'count' is the number of vertices in the template, which is either the + // vertex count (when 'indices' has a null buffer), or the index count when 'indices' are + // provided. + void setInstanceTemplate(BindBufferInfo vertices, BindBufferInfo indices, unsigned int count) { + this->setTemplateInternal(vertices, indices, count, /*drawPending=*/true); + } + + // Issues a draw with fully specified data. This can be used when all instance data has already + // been written to known buffers, or when the vertex shader only depends on the vertex or + // instance IDs. + // + // The specific draw call issued depends on the buffers set via 'setInstanceTemplate' and the + // 'instances' parameter. If the template has a non-null index buffer, it will use + // drawIndexedInstanced(), otherwise it will use drawInstanced(). + // + // This will not merge with any already appended instance or vertex data, pending data is issued + // in its own draw call first. + void drawInstanced(BindBufferInfo instances, unsigned int count) { + this->drawPendingVertices(); + this->drawInternal(instances, 0, count); + } + + // Issues a non-instanced draw call with existing, fully specified data. The specific draw call + // depends on the buffers passed to this function. If a non-null index buffer is specified, it + // will use drawIndexed(), otherwise it will use the vertex-only draw(). + // + // This will not merge with any existing appended instance or vertex data, which will issue it + // own draw call. This overrides what was last set for the instance template. + void draw(BindBufferInfo vertices, BindBufferInfo indices, unsigned int count) { + this->setInstanceTemplate(vertices, indices, count); // will draw pending if needed + this->drawInstanced({}, 1); + } + +private: + enum class VertexMode : unsigned { + kVertices, kInstances + }; + + // Both of these pointers must outlive the DrawWriter. + DrawDispatcher* fDispatcher; + DrawBufferManager* fManager; + + // Must be constructed to match the pipeline that's bound + PrimitiveType fPrimitiveType; + size_t fVertexStride; + size_t fInstanceStride; + + // State tracking appended vertices or instances + VertexMode fPendingMode = VertexMode::kVertices; + unsigned int fPendingCount = 0; // vertex or instance count depending on mode + unsigned int fPendingBaseVertex = 0; // or instance + BindBufferInfo fPendingAttrs = {}; + + // State to track the instance template that is re-used across drawn instances. These are not + // yet bound if fFixedBuffersDirty is true. Non-instanced draw buffers (e.g. draw() and + // drawIndexed()) are treated as drawing one instance, with no extra instance attributes. + BindBufferInfo fFixedVertexBuffer = {}; + BindBufferInfo fFixedIndexBuffer = {}; + unsigned int fFixedVertexCount = 0; // or index count if fFixedIndexBuffer is non-null + + bool fFixedBuffersDirty = true; + + // Will either be 'fPendingAttrData' or the arg last passed to drawInstanced(), since it may + // change even if the fixed vertex and index buffers have not. + BindBufferInfo fLastInstanceBuffer = {}; + + VertexWriter appendData(VertexMode mode, size_t stride, unsigned int count); + void setTemplateInternal(BindBufferInfo vertices, BindBufferInfo indices, + unsigned int count, bool drawPending); + void drawInternal(BindBufferInfo instances, unsigned int base, unsigned int instanceCount); + void drawPendingVertices(); +}; + +// Mirrors the CommandBuffer API, since a DrawWriter is meant to aggregate and then map onto +// CommandBuffer commands, although these are virtual to allow for recording to intermediate +// storage before a CommandBuffer is available. +class DrawDispatcher { +public: + virtual ~DrawDispatcher() = default; + + virtual void bindDrawBuffers(BindBufferInfo vertexAttribs, + BindBufferInfo instanceAttribs, + BindBufferInfo indices) = 0; + + virtual void draw(PrimitiveType type, unsigned int baseVertex, unsigned int vertexCount) = 0; + virtual void drawIndexed(PrimitiveType type, unsigned int baseIndex, + unsigned int indexCount, unsigned int baseVertex) = 0; + virtual void drawInstanced(PrimitiveType type, + unsigned int baseVertex, unsigned int vertexCount, + unsigned int baseInstance, unsigned int instanceCount) = 0; + virtual void drawIndexedInstanced(PrimitiveType type, + unsigned int baseIndex, unsigned int indexCount, + unsigned int baseVertex, unsigned int baseInstance, + unsigned int instanceCount) = 0; +}; + +} // namespace skgpu + +#endif // skgpu_DrawWriter_DEFINED diff --git a/experimental/graphite/src/Renderer.h b/experimental/graphite/src/Renderer.h index 7d067c7c0d..7cb9e9c418 100644 --- a/experimental/graphite/src/Renderer.h +++ b/experimental/graphite/src/Renderer.h @@ -8,6 +8,8 @@ #ifndef skgpu_Renderer_DEFINED #define skgpu_Renderer_DEFINED +#include "experimental/graphite/src/DrawTypes.h" + #include "include/core/SkSpan.h" #include "include/core/SkString.h" #include "include/core/SkTypes.h" @@ -16,9 +18,10 @@ namespace skgpu { -struct IndexWriter; +class DrawWriter; +class ResourceProvider; class Shape; -struct VertexWriter; +class Uniform; class RenderStep { public: @@ -29,22 +32,21 @@ public: virtual bool requiresMSAA() const = 0; virtual bool performsShading() const = 0; - virtual size_t requiredVertexSpace(const Shape&) const = 0; - virtual size_t requiredIndexSpace(const Shape&) const = 0; - virtual void writeVertices(VertexWriter, IndexWriter, const Shape&) const = 0; + // The DrawWriter is configured with the vertex and instance strides of the RenderStep, and its + // primitive type. The recorded draws will be executed with a graphics pipeline compatible with + // this RenderStep. + virtual void writeVertices(DrawWriter*, const Shape&) const = 0; + + // While RenderStep does not define the full program that's run for a draw, it defines the + // entire vertex layout of the pipeline. + virtual size_t vertexStride() const = 0; + virtual size_t instanceStride() const = 0; + virtual PrimitiveType primitiveType() const = 0; // TODO: Actual API to do things // 1. Provide stencil settings // 2. Provide shader key or MSL(?) for the vertex stage - // 3. Write vertex data given a Shape/Transform/Stroke info // 4. Write uniform data given a Shape/Transform/Stroke info - // 5. Somehow specify the draw call that needs to be made, although this can't just be "record" - // the draw call, since we want multiple draws to accumulate into the same vertex buffer, - // and the final draw totals aren't known until we have done #3 for another draw and it - // requires binding a new vertex buffer/offset. - // - maybe if it just says what it's primitive type is and instanced/indexed/etc. and then - // the DrawPass building is able to track the total number of vertices/indices written for - // the draws in the batch and can handle recording the draw command itself. // 6. Some Renderers benefit from being able to share vertices between RenderSteps. Must find a // way to support that. It may mean that RenderSteps get state per draw. // - Does Renderer make RenderStepFactories that create steps for each DrawList::Draw? diff --git a/experimental/graphite/src/render/StencilAndFillPathRenderer.cpp b/experimental/graphite/src/render/StencilAndFillPathRenderer.cpp index b4c55af7d6..8435d887db 100644 --- a/experimental/graphite/src/render/StencilAndFillPathRenderer.cpp +++ b/experimental/graphite/src/render/StencilAndFillPathRenderer.cpp @@ -7,6 +7,7 @@ #include "experimental/graphite/src/Renderer.h" +#include "experimental/graphite/src/DrawWriter.h" #include "experimental/graphite/src/geom/Shape.h" #include "src/gpu/BufferWriter.h" @@ -52,7 +53,7 @@ private: // TODO: Hand off to csmartdalton, this should roughly correspond to the fCoverBBoxProgram stage // of skgpu::v1::PathStencilCoverOp. -class FillBoundsRenderStep : public RenderStep { +class FillBoundsRenderStep final : public RenderStep { public: FillBoundsRenderStep() {} @@ -64,21 +65,24 @@ public: bool requiresMSAA() const override { return false; } bool performsShading() const override { return true; } - size_t requiredVertexSpace(const Shape&) const override { - return 8 * sizeof(float); + PrimitiveType primitiveType() const override { return PrimitiveType::kTriangleStrip; } + size_t instanceStride() const override { return 0; } + size_t vertexStride() const override { + return VertexAttribTypeSize(VertexAttribType::kFloat2); } - size_t requiredIndexSpace(const Shape&) const override { - return 0; + void writeVertices(DrawWriter* writer, const Shape& shape) const override { + // TODO: Need to account for the transform eventually, but that requires more plumbing + writer->appendVertices(4) + .writeQuad(VertexWriter::TriStripFromRect(shape.bounds().asSkRect())); + // TODO: triangle strip/fan is actually tricky to merge correctly, since we want strips + // for everything that was appended by the RenderStep, but no connection across RenderSteps, + // so either we need a way to end it here, or switch to instance rendering w/o instance + // data so that vertices are still clustered appripriately. But that would require updating + // the DrawWriter to support appending both vertex and instance data simultaneously, which + // would need to return 2 vertex writers? } - void writeVertices(VertexWriter vertexWriter, - IndexWriter indexWriter, - const Shape& shape) const override { - vertexWriter.writeQuad(VertexWriter::TriStripFromRect(shape.bounds().asSkRect())); - } - - private: }; diff --git a/gn/graphite.gni b/gn/graphite.gni index 66bff975d7..5c1f8cf9f7 100644 --- a/gn/graphite.gni +++ b/gn/graphite.gni @@ -43,6 +43,8 @@ skia_graphite_sources = [ "$_src/DrawPass.cpp", "$_src/DrawPass.h", "$_src/DrawTypes.h", + "$_src/DrawWriter.cpp", + "$_src/DrawWriter.h", "$_src/EnumBitMask.h", "$_src/Gpu.cpp", "$_src/Gpu.h", diff --git a/src/gpu/BufferWriter.h b/src/gpu/BufferWriter.h index df10441d73..40e9193a66 100644 --- a/src/gpu/BufferWriter.h +++ b/src/gpu/BufferWriter.h @@ -325,6 +325,8 @@ inline IndexWriter& operator<<(IndexWriter& w, uint16_t val) { return w; } +inline IndexWriter& operator<<(IndexWriter& w, int val) { return (w << SkTo(val)); } + /////////////////////////////////////////////////////////////////////////////////////////////////// struct UniformWriter : public BufferWriter { diff --git a/tests/graphite/CommandBufferTest.cpp b/tests/graphite/CommandBufferTest.cpp index d60521128f..674d3ffae8 100644 --- a/tests/graphite/CommandBufferTest.cpp +++ b/tests/graphite/CommandBufferTest.cpp @@ -13,6 +13,8 @@ #include "experimental/graphite/include/mtl/MtlTypes.h" #include "experimental/graphite/src/Buffer.h" #include "experimental/graphite/src/CommandBuffer.h" +#include "experimental/graphite/src/DrawBufferManager.h" +#include "experimental/graphite/src/DrawWriter.h" #include "experimental/graphite/src/Gpu.h" #include "experimental/graphite/src/GraphicsPipeline.h" #include "experimental/graphite/src/ResourceProvider.h" @@ -66,8 +68,13 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(CommandBufferTest, reporter, context) { renderPassDesc.fClearColor = { 1, 0, 0, 1 }; // red target->instantiate(gpu->resourceProvider()); + DrawBufferManager bufferMgr(gpu->resourceProvider(), 4); + + commandBuffer->beginRenderPass(renderPassDesc); + DrawWriter drawWriter(commandBuffer->asDrawDispatcher(), &bufferMgr); + // Shared uniform buffer struct UniformData { SkPoint fScale; @@ -82,19 +89,25 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(CommandBufferTest, reporter, context) { GraphicsPipelineDesc pipelineDesc; pipelineDesc.setTestingOnlyShaderIndex(0); auto graphicsPipeline = gpu->resourceProvider()->findOrCreateGraphicsPipeline(pipelineDesc); + drawWriter.newPipelineState(PrimitiveType::kTriangleStrip, + pipelineDesc.vertexStride(), + pipelineDesc.instanceStride()); commandBuffer->bindGraphicsPipeline(std::move(graphicsPipeline)); - commandBuffer->draw(PrimitiveType::kTriangleStrip, 0, 4); + drawWriter.draw({}, {}, 4); // Draw inset yellow rectangle using uniforms pipelineDesc.setTestingOnlyShaderIndex(1); graphicsPipeline = gpu->resourceProvider()->findOrCreateGraphicsPipeline(pipelineDesc); + drawWriter.newPipelineState(PrimitiveType::kTriangleStrip, + pipelineDesc.vertexStride(), + pipelineDesc.instanceStride()); commandBuffer->bindGraphicsPipeline(std::move(graphicsPipeline)); UniformData* uniforms = (UniformData*)uniformBuffer->map(); uniforms->fScale = SkPoint({1.8, 1.8}); uniforms->fTranslate = SkPoint({-0.9, -0.9}); uniforms->fColor = SkColors::kYellow; commandBuffer->bindUniformBuffer(UniformSlot::kRenderStep, uniformBuffer, uniformOffset); - commandBuffer->draw(PrimitiveType::kTriangleStrip, 0, 4); + drawWriter.draw({}, {}, 4); uniformOffset += sizeof(UniformData); ++uniforms; @@ -105,36 +118,25 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(CommandBufferTest, reporter, context) { }; pipelineDesc.setVertexAttributes(vertexAttributes, 1); graphicsPipeline = gpu->resourceProvider()->findOrCreateGraphicsPipeline(pipelineDesc); + drawWriter.newPipelineState(PrimitiveType::kTriangles, + pipelineDesc.vertexStride(), + pipelineDesc.instanceStride()); commandBuffer->bindGraphicsPipeline(std::move(graphicsPipeline)); - struct VertexData { - SkPoint fPosition; - }; - sk_sp vertexBuffer = gpu->resourceProvider()->findOrCreateBuffer( - 4*sizeof(VertexData), BufferType::kVertex, PrioritizeGpuReads::kNo); - sk_sp indexBuffer = gpu->resourceProvider()->findOrCreateBuffer( - 6*sizeof(uint16_t), BufferType::kIndex, PrioritizeGpuReads::kNo); - auto vertices = (VertexData*)vertexBuffer->map(); - vertices[0].fPosition = SkPoint({0.25f, 0.25f}); - vertices[1].fPosition = SkPoint({0.25f, 0.75f}); - vertices[2].fPosition = SkPoint({0.75f, 0.25f}); - vertices[3].fPosition = SkPoint({0.75f, 0.75f}); - vertexBuffer->unmap(); - auto indices = (uint16_t*)indexBuffer->map(); - indices[0] = 0; - indices[1] = 1; - indices[2] = 2; - indices[3] = 2; - indices[4] = 1; - indices[5] = 3; - indexBuffer->unmap(); - commandBuffer->bindVertexBuffers(vertexBuffer, 0, nullptr, 0); - commandBuffer->bindIndexBuffer(indexBuffer, 0); + auto [vertexWriter, vertices] = bufferMgr.getVertexWriter(4 * pipelineDesc.vertexStride()); + vertexWriter << SkPoint{0.25f, 0.25f} + << SkPoint{0.25f, 0.75f} + << SkPoint{0.75f, 0.25f} + << SkPoint{0.75f, 0.75f}; + auto [indexWriter, indices] = bufferMgr.getIndexWriter(6 * sizeof(uint16_t)); + indexWriter << 0 << 1 << 2 + << 2 << 1 << 3; uniforms->fScale = SkPoint({2, 2}); uniforms->fTranslate = SkPoint({-1, -1}); uniforms->fColor = SkColors::kMagenta; commandBuffer->bindUniformBuffer(UniformSlot::kRenderStep, uniformBuffer, uniformOffset); - commandBuffer->drawIndexed(PrimitiveType::kTriangles, 0, 6, 0); + + drawWriter.draw(vertices, indices, 6); // draw rects using instance buffer pipelineDesc.setTestingOnlyShaderIndex(3); @@ -146,31 +148,22 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(CommandBufferTest, reporter, context) { pipelineDesc.setVertexAttributes(nullptr, 0); pipelineDesc.setInstanceAttributes(instanceAttributes, 3); graphicsPipeline = gpu->resourceProvider()->findOrCreateGraphicsPipeline(pipelineDesc); + drawWriter.newPipelineState(PrimitiveType::kTriangles, + pipelineDesc.vertexStride(), + pipelineDesc.instanceStride()); commandBuffer->bindGraphicsPipeline(std::move(graphicsPipeline)); - struct InstanceData { - SkPoint fPosition; - SkPoint fDims; - SkColor4f fColor; - }; - sk_sp instanceBuffer = gpu->resourceProvider()->findOrCreateBuffer( - 2*sizeof(InstanceData), BufferType::kVertex, PrioritizeGpuReads::kNo); - auto instances = (InstanceData*)instanceBuffer->map(); - instances[0].fPosition = SkPoint({-0.4, -0.4}); - instances[0].fDims = SkPoint({0.4, 0.4}); - instances[0].fColor = SkColors::kGreen; - instances[1].fPosition = SkPoint({0, 0}); - instances[1].fDims = SkPoint({0.25, 0.25}); - instances[1].fColor = SkColors::kCyan; - instanceBuffer->unmap(); - commandBuffer->bindVertexBuffers(nullptr, 0, instanceBuffer, 0); -// commandBuffer->drawInstanced(PrimitiveType::kTriangleStrip, 0, 4, 0, 2); - commandBuffer->drawIndexedInstanced(PrimitiveType::kTriangles, 0, 6, 0, 0, 2); - - commandBuffer->endRenderPass(); + drawWriter.setInstanceTemplate({}, indices, 6); + auto instanceWriter = drawWriter.appendInstances(2); + instanceWriter << SkPoint{-0.4f, -0.4f} << SkPoint{0.4f, 0.4f} << SkColors::kGreen + << SkPoint{0.f, 0.f} << SkPoint{0.25f, 0.25f} << SkColors::kCyan; + drawWriter.flush(); uniformBuffer->unmap(); + bufferMgr.transferToCommandBuffer(commandBuffer.get()); + commandBuffer->endRenderPass(); + // Do readback // TODO: add 4-byte transfer buffer alignment for Mac to Caps