Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit bc24aed

Browse files
SenorBlancoSkia Commit-Bot
authored andcommitted
Dawn readpixels fixes.
The dawn backend was calling GrDawnGpu::flush() in a few places (in particular, for readpixels).Since GrGpu::submitToGpu() is now responsible for unmapping the staging buffers prior to command submission, internally calling GrDawnGpu::flush() is error-prone, since it requires you to unmapStagingBuffers() manually. The fix is to get rid of GrDawnGpu::flush(), and put its guts into GrDawnGpu::onSubmitToGpu(). Then we call submitToGpu(true) where we used to call flush(). Also, make the backend responsible for moving staging buffers from the active to the busy list. We do it just before calling mapAsync(), since the callback may return right away and we want the buffer to be in the busy state at that point. Change-Id: I99cfa5e7ea3a454b0383a30483fa93795191347a Reviewed-on: https://skia-review.googlesource.com/c/skia/+/288516 Reviewed-by: Greg Daniel <[email protected]> Commit-Queue: Stephen White <[email protected]>
1 parent 957189b commit bc24aed

File tree

3 files changed

+12
-22
lines changed

3 files changed

+12
-22
lines changed

src/gpu/GrGpu.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -692,12 +692,6 @@ bool GrGpu::submitToGpu(bool syncCpu) {
692692

693693
bool submitted = this->onSubmitToGpu(syncCpu);
694694

695-
// Move all active staging buffers to the busy list.
696-
// TODO: this should probably be handled inside of the onSubmitToGpu by the backends.
697-
while (GrStagingBuffer* buffer = fActiveStagingBuffers.head()) {
698-
fActiveStagingBuffers.remove(buffer);
699-
fBusyStagingBuffers.addToTail(buffer);
700-
}
701695
return submitted;
702696
}
703697

src/gpu/dawn/GrDawnGpu.cpp

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -460,21 +460,11 @@ void GrDawnGpu::deleteTestingOnlyBackendRenderTarget(const GrBackendRenderTarget
460460
}
461461

462462
void GrDawnGpu::testingOnly_flushGpuAndSync() {
463-
this->flush();
463+
this->submitToGpu(true);
464464
}
465465

466466
#endif
467467

468-
void GrDawnGpu::flush() {
469-
this->flushCopyEncoder();
470-
if (!fCommandBuffers.empty()) {
471-
fQueue.Submit(fCommandBuffers.size(), &fCommandBuffers.front());
472-
fCommandBuffers.clear();
473-
}
474-
this->mapStagingBuffers();
475-
fDevice.Tick();
476-
}
477-
478468
void GrDawnGpu::addFinishedProc(GrGpuFinishedProc finishedProc,
479469
GrGpuFinishedContext finishedContext) {
480470
fFinishCallbacks.add(finishedProc, finishedContext);
@@ -485,7 +475,12 @@ static void callback(WGPUFenceCompletionStatus status, void* userData) {
485475
}
486476

487477
bool GrDawnGpu::onSubmitToGpu(bool syncCpu) {
488-
this->flush();
478+
this->flushCopyEncoder();
479+
if (!fCommandBuffers.empty()) {
480+
fQueue.Submit(fCommandBuffers.size(), &fCommandBuffers.front());
481+
fCommandBuffers.clear();
482+
}
483+
this->mapStagingBuffers();
489484
if (syncCpu) {
490485
wgpu::FenceDescriptor desc;
491486
wgpu::Fence fence = fQueue.CreateFence(&desc);
@@ -541,7 +536,7 @@ bool GrDawnGpu::onReadPixels(GrSurface* surface, int left, int top, int width, i
541536
wgpu::Texture tex = get_dawn_texture_from_surface(surface);
542537
SkASSERT(tex);
543538

544-
if (0 == rowBytes) {
539+
if (!tex || 0 == rowBytes) {
545540
return false;
546541
}
547542
size_t origRowBytes = rowBytes;
@@ -567,7 +562,7 @@ bool GrDawnGpu::onReadPixels(GrSurface* surface, int left, int top, int width, i
567562

568563
wgpu::Extent3D copySize = {(uint32_t) width, (uint32_t) height, 1};
569564
this->getCopyEncoder().CopyTextureToBuffer(&srcTexture, &dstBuffer, &copySize);
570-
this->flush();
565+
this->submitToGpu(true);
571566

572567
const void *readPixelsPtr = nullptr;
573568
buf.MapReadAsync(callback, &readPixelsPtr);
@@ -721,7 +716,9 @@ void GrDawnGpu::flushCopyEncoder() {
721716

722717
void GrDawnGpu::mapStagingBuffers() {
723718
// Map all active buffers, so we get a callback when they're done.
724-
for (auto buffer : fActiveStagingBuffers) {
719+
while (auto buffer = fActiveStagingBuffers.head()) {
720+
fActiveStagingBuffers.remove(buffer);
721+
fBusyStagingBuffers.addToTail(buffer);
725722
static_cast<GrDawnStagingBuffer*>(buffer)->mapAsync();
726723
}
727724
}

src/gpu/dawn/GrDawnGpu.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ class GrDawnGpu : public GrGpu {
5353

5454
void testingOnly_flushGpuAndSync() override;
5555
#endif
56-
void flush();
5756
std::unique_ptr<GrStagingBuffer> createStagingBuffer(size_t size) override;
5857

5958
GrStencilAttachment* createStencilAttachmentForRenderTarget(const GrRenderTarget*,

0 commit comments

Comments
 (0)