Make GrGpuCommandBuffer infer its render target from first draw
This is a temporary workaround to allow removal of GrBatch::renderTarget().
Change-Id: Ic14710a369802064cf6446e8191a98ea3595556d
Reviewed-on: https://skia-review.googlesource.com/5342
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 80beffa..b892480 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -367,9 +367,11 @@
}
// Creates a GrGpuCommandBuffer in which the GrOpList can send draw commands to instead of
- // directly to the Gpu object.
+ // directly to the Gpu object. This currently does not take a GrRenderTarget. The command buffer
+ // is expected to infer the render target from the first draw, clear, or discard. This is an
+ // awkward workaround that goes away after MDB is complete and the render target is known from
+ // the GrRenderTargetOpList.
virtual GrGpuCommandBuffer* createCommandBuffer(
- GrRenderTarget* target,
const GrGpuCommandBuffer::LoadAndStoreInfo& colorInfo,
const GrGpuCommandBuffer::LoadAndStoreInfo& stencilInfo) = 0;
diff --git a/src/gpu/GrGpuCommandBuffer.cpp b/src/gpu/GrGpuCommandBuffer.cpp
index d2a4e6e..9ef459f 100644
--- a/src/gpu/GrGpuCommandBuffer.cpp
+++ b/src/gpu/GrGpuCommandBuffer.cpp
@@ -19,20 +19,19 @@
this->onSubmit();
}
-void GrGpuCommandBuffer::clear(const GrFixedClip& clip, GrColor color) {
+void GrGpuCommandBuffer::clear(GrRenderTarget* rt, const GrFixedClip& clip, GrColor color) {
#ifdef SK_DEBUG
- GrRenderTarget* rt = this->renderTarget();
SkASSERT(rt);
SkASSERT(!clip.scissorEnabled() ||
(SkIRect::MakeWH(rt->width(), rt->height()).contains(clip.scissorRect()) &&
SkIRect::MakeWH(rt->width(), rt->height()) != clip.scissorRect()));
#endif
- this->onClear(clip, color);
+ this->onClear(rt, clip, color);
}
-void GrGpuCommandBuffer::clearStencilClip(const GrFixedClip& clip,
+void GrGpuCommandBuffer::clearStencilClip(GrRenderTarget* rt, const GrFixedClip& clip,
bool insideStencilMask) {
- this->onClearStencilClip(clip, insideStencilMask);
+ this->onClearStencilClip(rt, clip, insideStencilMask);
}
bool GrGpuCommandBuffer::draw(const GrPipeline& pipeline,
diff --git a/src/gpu/GrGpuCommandBuffer.h b/src/gpu/GrGpuCommandBuffer.h
index ef4f428..f8c938b 100644
--- a/src/gpu/GrGpuCommandBuffer.h
+++ b/src/gpu/GrGpuCommandBuffer.h
@@ -26,6 +26,12 @@
* the same render target. It is possible that these commands execute immediately (GL), or get
* buffered up for later execution (Vulkan). GrBatches will execute their draw commands into a
* GrGpuCommandBuffer.
+ *
+ * Ideally we'd know the GrRenderTarget, or at least its properties when the GrGpuCommandBuffer, is
+ * created. We also then wouldn't include it in the GrPipeline or as a parameter to the clear and
+ * discard methods. The logical place for that will be in GrRenderTargetOpList post-MDB. For now
+ * the render target is redundantly passed to each operation, though it will always be the same
+ * render target for a given command buffer even pre-MDB.
*/
class GrGpuCommandBuffer {
public:
@@ -72,16 +78,16 @@
/**
* Clear the passed in render target. Ignores the draw state and clip.
*/
- void clear(const GrFixedClip&, GrColor);
+ void clear(GrRenderTarget*, const GrFixedClip&, GrColor);
- void clearStencilClip(const GrFixedClip&, bool insideStencilMask);
+ void clearStencilClip(GrRenderTarget*, const GrFixedClip&, bool insideStencilMask);
/**
* Discards the contents render target. nullptr indicates that the current render target should
* be discarded.
*/
// TODO: This should be removed in the future to favor using the load and store ops for discard
- virtual void discard() = 0;
+ virtual void discard(GrRenderTarget*) = 0;
private:
virtual GrGpu* gpu() = 0;
@@ -97,9 +103,9 @@
const SkRect& bounds) = 0;
// overridden by backend-specific derived class to perform the clear.
- virtual void onClear(const GrFixedClip&, GrColor) = 0;
+ virtual void onClear(GrRenderTarget*, const GrFixedClip&, GrColor) = 0;
- virtual void onClearStencilClip(const GrFixedClip&,
+ virtual void onClearStencilClip(GrRenderTarget*, const GrFixedClip&,
bool insideStencilMask) = 0;
};
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index aab71f9..b69d915 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -204,8 +204,7 @@
static const GrGpuCommandBuffer::LoadAndStoreInfo kBasicLoadStoreInfo
{ GrGpuCommandBuffer::LoadOp::kLoad,GrGpuCommandBuffer::StoreOp::kStore,
GrColor_ILLEGAL };
- commandBuffer.reset(fGpu->createCommandBuffer(currentRT,
- kBasicLoadStoreInfo, // Color
+ commandBuffer.reset(fGpu->createCommandBuffer(kBasicLoadStoreInfo, // Color
kBasicLoadStoreInfo)); // Stencil
}
flushState->setCommandBuffer(commandBuffer.get());
diff --git a/src/gpu/batches/GrClearBatch.h b/src/gpu/batches/GrClearBatch.h
index ef38e74..724981e 100644
--- a/src/gpu/batches/GrClearBatch.h
+++ b/src/gpu/batches/GrClearBatch.h
@@ -100,7 +100,7 @@
void onPrepare(GrBatchFlushState*) override {}
void onDraw(GrBatchFlushState* state, const SkRect& /*bounds*/) override {
- state->commandBuffer()->clear(fClip, fColor);
+ state->commandBuffer()->clear(fRenderTarget.get(), fClip, fColor);
}
GrFixedClip fClip;
diff --git a/src/gpu/batches/GrClearStencilClipBatch.h b/src/gpu/batches/GrClearStencilClipBatch.h
index f1e5b50..7f69f64 100644
--- a/src/gpu/batches/GrClearStencilClipBatch.h
+++ b/src/gpu/batches/GrClearStencilClipBatch.h
@@ -55,7 +55,7 @@
void onPrepare(GrBatchFlushState*) override {}
void onDraw(GrBatchFlushState* state, const SkRect& /*bounds*/) override {
- state->commandBuffer()->clearStencilClip(fClip, fInsideStencilMask);
+ state->commandBuffer()->clearStencilClip(fRenderTarget.get(), fClip, fInsideStencilMask);
}
const GrFixedClip fClip;
diff --git a/src/gpu/batches/GrDiscardBatch.h b/src/gpu/batches/GrDiscardBatch.h
index 54f41b9..799c746 100644
--- a/src/gpu/batches/GrDiscardBatch.h
+++ b/src/gpu/batches/GrDiscardBatch.h
@@ -47,7 +47,7 @@
void onPrepare(GrBatchFlushState*) override {}
void onDraw(GrBatchFlushState* state, const SkRect& /*bounds*/) override {
- state->commandBuffer()->discard();
+ state->commandBuffer()->discard(fRenderTarget.get());
}
GrPendingIOResource<GrRenderTarget, kWrite_GrIOType> fRenderTarget;
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 67414a4..d3f13d0 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2645,10 +2645,9 @@
}
GrGpuCommandBuffer* GrGLGpu::createCommandBuffer(
- GrRenderTarget* target,
const GrGpuCommandBuffer::LoadAndStoreInfo& colorInfo,
const GrGpuCommandBuffer::LoadAndStoreInfo& stencilInfo) {
- return new GrGLGpuCommandBuffer(this, static_cast<GrGLRenderTarget*>(target));
+ return new GrGLGpuCommandBuffer(this);
}
void GrGLGpu::finishOpList() {
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index c6e7935..0c3d060 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -122,7 +122,6 @@
void clearStencil(GrRenderTarget*) override;
GrGpuCommandBuffer* createCommandBuffer(
- GrRenderTarget* target,
const GrGpuCommandBuffer::LoadAndStoreInfo& colorInfo,
const GrGpuCommandBuffer::LoadAndStoreInfo& stencilInfo) override;
diff --git a/src/gpu/gl/GrGLGpuCommandBuffer.h b/src/gpu/gl/GrGLGpuCommandBuffer.h
index 069eea5..d1e1fe8 100644
--- a/src/gpu/gl/GrGLGpuCommandBuffer.h
+++ b/src/gpu/gl/GrGLGpuCommandBuffer.h
@@ -22,13 +22,19 @@
* pass through functions to corresponding calls in the GrGLGpu class.
*/
public:
- GrGLGpuCommandBuffer(GrGLGpu* gpu, GrGLRenderTarget* rt) : fGpu(gpu), fRenderTarget(rt) {}
+ GrGLGpuCommandBuffer(GrGLGpu* gpu) : fGpu(gpu), fRenderTarget(nullptr) {}
virtual ~GrGLGpuCommandBuffer() {}
void end() override {}
- void discard() override {}
+ void discard(GrRenderTarget* rt) override {
+ GrGLRenderTarget* target = static_cast<GrGLRenderTarget*>(rt);
+ if (!fRenderTarget) {
+ fRenderTarget = target;
+ }
+ SkASSERT(target == fRenderTarget);
+ }
void inlineUpload(GrBatchFlushState* state, GrDrawBatch::DeferredUploadFn& upload) override {
state->doUpload(upload);
@@ -45,15 +51,30 @@
const GrMesh* mesh,
int meshCount,
const SkRect& bounds) override {
+ GrGLRenderTarget* target = static_cast<GrGLRenderTarget*>(pipeline.getRenderTarget());
+ if (!fRenderTarget) {
+ fRenderTarget = target;
+ }
+ SkASSERT(target == fRenderTarget);
fGpu->draw(pipeline, primProc, mesh, meshCount);
}
- void onClear(const GrFixedClip& clip, GrColor color) override {
+ void onClear(GrRenderTarget* rt, const GrFixedClip& clip, GrColor color) override {
+ GrGLRenderTarget* target = static_cast<GrGLRenderTarget*>(rt);
+ if (!fRenderTarget) {
+ fRenderTarget = target;
+ }
+ SkASSERT(target == fRenderTarget);
fGpu->clear(clip, color, fRenderTarget);
}
- void onClearStencilClip(const GrFixedClip& clip,
+ void onClearStencilClip(GrRenderTarget* rt, const GrFixedClip& clip,
bool insideStencilMask) override {
+ GrGLRenderTarget* target = static_cast<GrGLRenderTarget*>(rt);
+ if (!fRenderTarget) {
+ fRenderTarget = target;
+ }
+ SkASSERT(target == fRenderTarget);
fGpu->clearStencilClip(clip, insideStencilMask, fRenderTarget);
}
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 674e8e4..9fab21d 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -210,11 +210,9 @@
///////////////////////////////////////////////////////////////////////////////
GrGpuCommandBuffer* GrVkGpu::createCommandBuffer(
- GrRenderTarget* target,
const GrGpuCommandBuffer::LoadAndStoreInfo& colorInfo,
const GrGpuCommandBuffer::LoadAndStoreInfo& stencilInfo) {
- GrVkRenderTarget* vkRT = static_cast<GrVkRenderTarget*>(target);
- return new GrVkGpuCommandBuffer(this, vkRT, colorInfo, stencilInfo);
+ return new GrVkGpuCommandBuffer(this, colorInfo, stencilInfo);
}
void GrVkGpu::submitCommandBuffer(SyncQueue sync) {
diff --git a/src/gpu/vk/GrVkGpu.h b/src/gpu/vk/GrVkGpu.h
index dd1eaa1..fce0100 100644
--- a/src/gpu/vk/GrVkGpu.h
+++ b/src/gpu/vk/GrVkGpu.h
@@ -102,7 +102,6 @@
void clearStencil(GrRenderTarget* target) override;
GrGpuCommandBuffer* createCommandBuffer(
- GrRenderTarget* target,
const GrGpuCommandBuffer::LoadAndStoreInfo& colorInfo,
const GrGpuCommandBuffer::LoadAndStoreInfo& stencilInfo) override;
diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
index 9b70c0b..7725e8e 100644
--- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
@@ -53,21 +53,28 @@
}
GrVkGpuCommandBuffer::GrVkGpuCommandBuffer(GrVkGpu* gpu,
- GrVkRenderTarget* target,
const LoadAndStoreInfo& colorInfo,
const LoadAndStoreInfo& stencilInfo)
: fGpu(gpu)
- , fRenderTarget(target) {
- VkAttachmentLoadOp vkLoadOp;
- VkAttachmentStoreOp vkStoreOp;
+ , fRenderTarget(nullptr)
+ , fClearColor(GrColor4f::FromGrColor(colorInfo.fClearColor)){
- get_vk_load_store_ops(colorInfo, &vkLoadOp, &vkStoreOp);
- GrVkRenderPass::LoadStoreOps vkColorOps(vkLoadOp, vkStoreOp);
+ get_vk_load_store_ops(colorInfo, &fVkColorLoadOp, &fVkColorStoreOp);
- get_vk_load_store_ops(stencilInfo, &vkLoadOp, &vkStoreOp);
- GrVkRenderPass::LoadStoreOps vkStencilOps(vkLoadOp, vkStoreOp);
+ get_vk_load_store_ops(stencilInfo, &fVkStencilLoadOp, &fVkStencilStoreOp);
+
+ fCurrentCmdBuffer = -1;
+}
+
+void GrVkGpuCommandBuffer::init(GrVkRenderTarget* target) {
+ SkASSERT(!fRenderTarget);
+ fRenderTarget = target;
+
+ GrVkRenderPass::LoadStoreOps vkColorOps(fVkColorLoadOp, fVkColorStoreOp);
+ GrVkRenderPass::LoadStoreOps vkStencilOps(fVkStencilLoadOp, fVkStencilStoreOp);
CommandBufferInfo& cbInfo = fCommandBufferInfos.push_back();
+ SkASSERT(fCommandBufferInfos.count() == 1);
fCurrentCmdBuffer = 0;
const GrVkResourceProvider::CompatibleRPHandle& rpHandle = target->compatibleRenderPassHandle();
@@ -81,16 +88,20 @@
vkStencilOps);
}
- GrColorToRGBAFloat(colorInfo.fClearColor, cbInfo.fColorClearValue.color.float32);
+ cbInfo.fColorClearValue.color.float32[0] = fClearColor.fRGBA[0];
+ cbInfo.fColorClearValue.color.float32[1] = fClearColor.fRGBA[1];
+ cbInfo.fColorClearValue.color.float32[2] = fClearColor.fRGBA[2];
+ cbInfo.fColorClearValue.color.float32[3] = fClearColor.fRGBA[3];
cbInfo.fBounds.setEmpty();
cbInfo.fIsEmpty = true;
cbInfo.fStartsWithClear = false;
- cbInfo.fCommandBuffer = gpu->resourceProvider().findOrCreateSecondaryCommandBuffer();
- cbInfo.fCommandBuffer->begin(gpu, target->framebuffer(), cbInfo.fRenderPass);
+ cbInfo.fCommandBuffer = fGpu->resourceProvider().findOrCreateSecondaryCommandBuffer();
+ cbInfo.fCommandBuffer->begin(fGpu, target->framebuffer(), cbInfo.fRenderPass);
}
+
GrVkGpuCommandBuffer::~GrVkGpuCommandBuffer() {
for (int i = 0; i < fCommandBufferInfos.count(); ++i) {
CommandBufferInfo& cbInfo = fCommandBufferInfos[i];
@@ -103,10 +114,15 @@
GrRenderTarget* GrVkGpuCommandBuffer::renderTarget() { return fRenderTarget; }
void GrVkGpuCommandBuffer::end() {
- fCommandBufferInfos[fCurrentCmdBuffer].fCommandBuffer->end(fGpu);
+ if (fCurrentCmdBuffer >= 0) {
+ fCommandBufferInfos[fCurrentCmdBuffer].fCommandBuffer->end(fGpu);
+ }
}
void GrVkGpuCommandBuffer::onSubmit() {
+ if (!fRenderTarget) {
+ return;
+ }
// Change layout of our render target so it can be used as the color attachment. Currently
// we don't attach the resolve to the framebuffer so no need to change its layout.
GrVkImage* targetImage = fRenderTarget->msaaImage() ? fRenderTarget->msaaImage()
@@ -162,7 +178,13 @@
}
}
-void GrVkGpuCommandBuffer::discard() {
+void GrVkGpuCommandBuffer::discard(GrRenderTarget* rt) {
+ GrVkRenderTarget* target = static_cast<GrVkRenderTarget*>(rt);
+ if (!fRenderTarget) {
+ this->init(target);
+ }
+ SkASSERT(target == fRenderTarget);
+
CommandBufferInfo& cbInfo = fCommandBufferInfos[fCurrentCmdBuffer];
if (cbInfo.fIsEmpty) {
// We will change the render pass to do a clear load instead
@@ -192,10 +214,16 @@
}
}
-void GrVkGpuCommandBuffer::onClearStencilClip(const GrFixedClip& clip,
+void GrVkGpuCommandBuffer::onClearStencilClip(GrRenderTarget* rt, const GrFixedClip& clip,
bool insideStencilMask) {
SkASSERT(!clip.hasWindowRectangles());
+ GrVkRenderTarget* target = static_cast<GrVkRenderTarget*>(rt);
+ if (!fRenderTarget) {
+ this->init(target);
+ }
+ SkASSERT(target == fRenderTarget);
+
CommandBufferInfo& cbInfo = fCommandBufferInfos[fCurrentCmdBuffer];
GrStencilAttachment* sb = fRenderTarget->renderTargetPriv().getStencilAttachment();
@@ -253,10 +281,16 @@
}
}
-void GrVkGpuCommandBuffer::onClear(const GrFixedClip& clip, GrColor color) {
+void GrVkGpuCommandBuffer::onClear(GrRenderTarget* rt, const GrFixedClip& clip, GrColor color) {
// parent class should never let us get here with no RT
SkASSERT(!clip.hasWindowRectangles());
+ GrVkRenderTarget* target = static_cast<GrVkRenderTarget*>(rt);
+ if (!fRenderTarget) {
+ this->init(target);
+ }
+ SkASSERT(target == fRenderTarget);
+
CommandBufferInfo& cbInfo = fCommandBufferInfos[fCurrentCmdBuffer];
VkClearColorValue vkColor;
@@ -462,6 +496,12 @@
const GrMesh* meshes,
int meshCount,
const SkRect& bounds) {
+ GrVkRenderTarget* target = static_cast<GrVkRenderTarget*>(pipeline.getRenderTarget());
+ if (!fRenderTarget) {
+ this->init(target);
+ }
+ SkASSERT(target == fRenderTarget);
+
if (!meshCount) {
return;
}
diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.h b/src/gpu/vk/GrVkGpuCommandBuffer.h
index 160e4ca..9c6f03d 100644
--- a/src/gpu/vk/GrVkGpuCommandBuffer.h
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.h
@@ -24,7 +24,6 @@
class GrVkGpuCommandBuffer : public GrGpuCommandBuffer {
public:
GrVkGpuCommandBuffer(GrVkGpu* gpu,
- GrVkRenderTarget*,
const LoadAndStoreInfo& colorInfo,
const LoadAndStoreInfo& stencilInfo);
@@ -32,11 +31,14 @@
void end() override;
- void discard() override;
+ void discard(GrRenderTarget*) override;
void inlineUpload(GrBatchFlushState* state, GrDrawBatch::DeferredUploadFn& upload) override;
private:
+ // Performs lazy initialization on the first operation seen by the command buffer.
+ void init(GrVkRenderTarget* rt);
+
GrGpu* gpu() override;
GrRenderTarget* renderTarget() override;
@@ -55,9 +57,9 @@
int meshCount,
const SkRect& bounds) override;
- void onClear(const GrFixedClip&, GrColor color) override;
+ void onClear(GrRenderTarget*, const GrFixedClip&, GrColor color) override;
- void onClearStencilClip(const GrFixedClip&, bool insideStencilMask) override;
+ void onClearStencilClip(GrRenderTarget*, const GrFixedClip&, bool insideStencilMask) override;
void addAdditionalCommandBuffer();
@@ -85,6 +87,11 @@
GrVkGpu* fGpu;
GrVkRenderTarget* fRenderTarget;
+ VkAttachmentLoadOp fVkColorLoadOp;
+ VkAttachmentStoreOp fVkColorStoreOp;
+ VkAttachmentLoadOp fVkStencilLoadOp;
+ VkAttachmentStoreOp fVkStencilStoreOp;
+ GrColor4f fClearColor;
typedef GrGpuCommandBuffer INHERITED;
};