Use clipped bounds for reordering decisions
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2137543002
NOTREECHECKS=true
NOPRESUBMIT=true
Review-Url: https://codereview.chromium.org/2137543002
diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp
index 356c480..8beaade 100644
--- a/src/gpu/GrDrawTarget.cpp
+++ b/src/gpu/GrDrawTarget.cpp
@@ -115,15 +115,19 @@
SkDebugf("%d, ", fDependencies[i]->fDebugID);
}
SkDebugf("\n");
- SkDebugf("batches (%d):\n", fBatches.count());
- for (int i = 0; i < fBatches.count(); ++i) {
+ SkDebugf("batches (%d):\n", fRecordedBatches.count());
+ for (int i = 0; i < fRecordedBatches.count(); ++i) {
SkDebugf("*******************************\n");
- if (!fBatches[i]) {
+ if (!fRecordedBatches[i].fBatch) {
SkDebugf("%d: <combined forward>\n", i);
} else {
- SkDebugf("%d: %s\n", i, fBatches[i]->name());
- SkString str = fBatches[i]->dumpInfo();
+ SkDebugf("%d: %s\n", i, fRecordedBatches[i].fBatch->name());
+ SkString str = fRecordedBatches[i].fBatch->dumpInfo();
SkDebugf("%s\n", str.c_str());
+ const SkRect& clippedBounds = fRecordedBatches[i].fClippedBounds;
+ SkDebugf("ClippedBounds: [L: %.2f, T: %.2f, R: %.2f, B: %.2f]\n",
+ clippedBounds.fLeft, clippedBounds.fTop, clippedBounds.fRight,
+ clippedBounds.fBottom);
}
}
}
@@ -199,9 +203,9 @@
this->makeClosed();
// Loop over the batches that haven't yet generated their geometry
- for (int i = 0; i < fBatches.count(); ++i) {
- if (fBatches[i]) {
- fBatches[i]->prepare(flushState);
+ for (int i = 0; i < fRecordedBatches.count(); ++i) {
+ if (fRecordedBatches[i].fBatch) {
+ fRecordedBatches[i].fBatch->prepare(flushState);
}
}
@@ -216,11 +220,11 @@
GrRenderTarget* currentRT = nullptr;
SkAutoTDelete<GrGpuCommandBuffer> commandBuffer;
SkRect bounds = SkRect::MakeEmpty();
- for (int i = 0; i < fBatches.count(); ++i) {
- if (!fBatches[i]) {
+ for (int i = 0; i < fRecordedBatches.count(); ++i) {
+ if (!fRecordedBatches[i].fBatch) {
continue;
}
- if (fBatches[i]->renderTarget() != currentRT) {
+ if (fRecordedBatches[i].fBatch->renderTarget() != currentRT) {
if (commandBuffer) {
commandBuffer->end();
if (bounds.intersect(0, 0,
@@ -233,7 +237,7 @@
commandBuffer.reset();
}
bounds.setEmpty();
- currentRT = fBatches[i]->renderTarget();
+ currentRT = fRecordedBatches[i].fBatch->renderTarget();
if (currentRT) {
static const GrGpuCommandBuffer::LoadAndStoreInfo kBasicLoadStoreInfo
{ GrGpuCommandBuffer::LoadOp::kLoad,GrGpuCommandBuffer::StoreOp::kStore,
@@ -245,19 +249,19 @@
flushState->setCommandBuffer(commandBuffer);
}
if (commandBuffer) {
- bounds.join(fBatches[i]->bounds());
+ bounds.join(fRecordedBatches[i].fClippedBounds);
}
if (fDrawBatchBounds) {
- const SkRect& batchBounds = fBatches[i]->bounds();
- SkIRect iBatchBounds;
- batchBounds.roundOut(&iBatchBounds);
+ const SkRect& bounds = fRecordedBatches[i].fClippedBounds;
+ SkIRect ibounds;
+ bounds.roundOut(&ibounds);
// In multi-draw buffer all the batches use the same render target and we won't need to
// get the batchs bounds.
- if (GrRenderTarget* rt = fBatches[i]->renderTarget()) {
- fGpu->drawDebugWireRect(rt, iBatchBounds, 0xFF000000 | random.nextU());
+ if (GrRenderTarget* rt = fRecordedBatches[i].fBatch->renderTarget()) {
+ fGpu->drawDebugWireRect(rt, ibounds, 0xFF000000 | random.nextU());
}
}
- fBatches[i]->draw(flushState);
+ fRecordedBatches[i].fBatch->draw(flushState);
}
if (commandBuffer) {
commandBuffer->end();
@@ -275,7 +279,7 @@
}
void GrDrawTarget::reset() {
- fBatches.reset();
+ fRecordedBatches.reset();
if (fInstancedRendering) {
fInstancedRendering->endFlush();
}
@@ -307,6 +311,16 @@
}
}
+static inline bool intersect(SkRect* out, const SkRect& a, const SkRect& b) {
+ SkASSERT(a.fLeft <= a.fRight && a.fTop <= a.fBottom);
+ SkASSERT(b.fLeft <= b.fRight && b.fTop <= b.fBottom);
+ out->fLeft = SkTMax(a.fLeft, b.fLeft);
+ out->fTop = SkTMax(a.fTop, b.fTop);
+ out->fRight = SkTMin(a.fRight, b.fRight);
+ out->fBottom = SkTMin(a.fBottom, b.fBottom);
+ return (out->fLeft <= out->fRight && out->fTop <= out->fBottom);
+}
+
void GrDrawTarget::drawBatch(const GrPipelineBuilder& pipelineBuilder,
GrDrawContext* drawContext,
const GrClip& clip,
@@ -384,8 +398,9 @@
SkASSERT(fRenderTarget);
batch->pipeline()->addDependenciesTo(fRenderTarget);
#endif
-
- this->recordBatch(batch);
+ SkRect clippedBounds;
+ SkAssertResult(intersect(&clippedBounds, bounds, appliedClip.deviceBounds()));
+ this->recordBatch(batch, clippedBounds);
}
void GrDrawTarget::stencilPath(const GrPipelineBuilder& pipelineBuilder,
@@ -424,7 +439,7 @@
appliedClip.scissorState(),
drawContext->accessRenderTarget(),
path);
- this->recordBatch(batch);
+ this->recordBatch(batch, appliedClip.deviceBounds());
batch->unref();
}
@@ -465,7 +480,7 @@
this->drawBatch(pipelineBuilder, drawContext, GrNoClip(), batch);
} else {
GrBatch* batch = new GrClearBatch(*rect, color, drawContext->accessRenderTarget());
- this->recordBatch(batch);
+ this->recordBatch(batch, batch->bounds());
batch->unref();
}
}
@@ -473,7 +488,7 @@
void GrDrawTarget::discard(GrRenderTarget* renderTarget) {
if (this->caps()->discardRenderTargetSupport()) {
GrBatch* batch = new GrDiscardBatch(renderTarget);
- this->recordBatch(batch);
+ this->recordBatch(batch, batch->bounds());
batch->unref();
}
}
@@ -492,25 +507,26 @@
this->addDependency(src);
#endif
- this->recordBatch(batch);
+ this->recordBatch(batch, batch->bounds());
batch->unref();
return true;
}
-static inline bool exclusive_no_intersection(const SkRect& a, const SkRect& b) {
+static inline bool can_reorder(const SkRect& a, const SkRect& b) {
return a.fRight <= b.fLeft || a.fBottom <= b.fTop ||
b.fRight <= a.fLeft || b.fBottom <= a.fTop;
}
-static inline bool can_reorder(const GrBatch* a, const GrBatch* b) {
- SkRect ra;
- SkRect rb;
- batch_bounds(&ra, a);
- batch_bounds(&rb, a);
- return exclusive_no_intersection(ra, rb);
+static void join(SkRect* out, const SkRect& a, const SkRect& b) {
+ SkASSERT(a.fLeft <= a.fRight && a.fTop <= a.fBottom);
+ SkASSERT(b.fLeft <= b.fRight && b.fTop <= b.fBottom);
+ out->fLeft = SkTMin(a.fLeft, b.fLeft);
+ out->fTop = SkTMin(a.fTop, b.fTop);
+ out->fRight = SkTMax(a.fRight, b.fRight);
+ out->fBottom = SkTMax(a.fBottom, b.fBottom);
}
-void GrDrawTarget::recordBatch(GrBatch* batch) {
+void GrDrawTarget::recordBatch(GrBatch* batch, const SkRect& clippedBounds) {
// A closed drawTarget should never receive new/more batches
SkASSERT(!this->isClosed());
@@ -526,12 +542,15 @@
batch->bounds().fLeft, batch->bounds().fRight,
batch->bounds().fTop, batch->bounds().fBottom);
GrBATCH_INFO(SkTabString(batch->dumpInfo(), 1).c_str());
+ GrBATCH_INFO("\tClipped Bounds: [L: %.2f, T: %.2f, R: %.2f, B: %.2f]\n",
+ clippedBounds.fLeft, clippedBounds.fTop, clippedBounds.fRight,
+ clippedBounds.fBottom);
GrBATCH_INFO("\tOutcome:\n");
- int maxCandidates = SkTMin(fMaxBatchLookback, fBatches.count());
+ int maxCandidates = SkTMin(fMaxBatchLookback, fRecordedBatches.count());
if (maxCandidates) {
int i = 0;
while (true) {
- GrBatch* candidate = fBatches.fromBack(i);
+ GrBatch* candidate = fRecordedBatches.fromBack(i).fBatch.get();
// We cannot continue to search backwards if the render target changes
if (candidate->renderTargetUniqueID() != batch->renderTargetUniqueID()) {
GrBATCH_INFO("\t\tBreaking because of (%s, B%u) Rendertarget\n",
@@ -542,12 +561,13 @@
GrBATCH_INFO("\t\tCombining with (%s, B%u)\n", candidate->name(),
candidate->uniqueID());
GR_AUDIT_TRAIL_BATCHING_RESULT_COMBINED(fAuditTrail, candidate, batch);
+ join(&fRecordedBatches.fromBack(i).fClippedBounds,
+ fRecordedBatches.fromBack(i).fClippedBounds, clippedBounds);
return;
}
// Stop going backwards if we would cause a painter's order violation.
- // TODO: The bounds used here do not fully consider the clip. It may be advantageous
- // to clip each batch's bounds to the clip.
- if (!can_reorder(candidate, batch)) {
+ const SkRect& candidateBounds = fRecordedBatches.fromBack(i).fClippedBounds;
+ if (!can_reorder(candidateBounds, clippedBounds)) {
GrBATCH_INFO("\t\tIntersects with (%s, B%u)\n", candidate->name(),
candidate->uniqueID());
break;
@@ -562,16 +582,17 @@
GrBATCH_INFO("\t\tFirstBatch\n");
}
GR_AUDIT_TRAIL_BATCHING_RESULT_NEW(fAuditTrail, batch);
- fBatches.push_back().reset(SkRef(batch));
+ fRecordedBatches.emplace_back(RecordedBatch{sk_ref_sp(batch), clippedBounds});
}
void GrDrawTarget::forwardCombine() {
- for (int i = 0; i < fBatches.count() - 2; ++i) {
- GrBatch* batch = fBatches[i];
- int maxCandidateIdx = SkTMin(i + fMaxBatchLookahead, fBatches.count() - 1);
+ for (int i = 0; i < fRecordedBatches.count() - 2; ++i) {
+ GrBatch* batch = fRecordedBatches[i].fBatch.get();
+ const SkRect& batchBounds = fRecordedBatches[i].fClippedBounds;
+ int maxCandidateIdx = SkTMin(i + fMaxBatchLookahead, fRecordedBatches.count() - 1);
int j = i + 1;
while (true) {
- GrBatch* candidate = fBatches[j];
+ GrBatch* candidate = fRecordedBatches[j].fBatch.get();
// We cannot continue to search if the render target changes
if (candidate->renderTargetUniqueID() != batch->renderTargetUniqueID()) {
GrBATCH_INFO("\t\tBreaking because of (%s, B%u) Rendertarget\n",
@@ -586,14 +607,14 @@
GrBATCH_INFO("\t\tCombining with (%s, B%u)\n", candidate->name(),
candidate->uniqueID());
GR_AUDIT_TRAIL_BATCHING_RESULT_COMBINED(fAuditTrail, batch, candidate);
- fBatches[j].reset(SkRef(batch));
- fBatches[i].reset(nullptr);
+ fRecordedBatches[j].fBatch = std::move(fRecordedBatches[i].fBatch);
+ join(&fRecordedBatches[j].fClippedBounds, fRecordedBatches[j].fClippedBounds,
+ batchBounds);
break;
}
// Stop going traversing if we would cause a painter's order violation.
- // TODO: The bounds used here do not fully consider the clip. It may be advantageous
- // to clip each batch's bounds to the clip.
- if (!can_reorder(candidate, batch)) {
+ const SkRect& candidateBounds = fRecordedBatches[j].fClippedBounds;
+ if (!can_reorder(candidateBounds, batchBounds)) {
GrBATCH_INFO("\t\tIntersects with (%s, B%u)\n", candidate->name(),
candidate->uniqueID());
break;
@@ -611,6 +632,6 @@
void GrDrawTarget::clearStencilClip(const SkIRect& rect, bool insideClip, GrRenderTarget* rt) {
GrBatch* batch = new GrClearStencilClipBatch(rect, insideClip, rt);
- this->recordBatch(batch);
+ this->recordBatch(batch, batch->bounds());
batch->unref();
}