Reland "Initiate regeneration of mipmaps from proxy DAG land"
This is a reland of fe19203eb7d7e8bef528287494d82c2eb4efff0e
Original change's description:
> Initiate regeneration of mipmaps from proxy DAG land
>
> This allows us to resolve all the textures before executing a command
> buffer, rather than interrupting execution as is currently done in the
> GL backend.
>
> Change-Id: I998546b312d24cf47fc2c837e7c94fbf95571af0
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/230636
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
Change-Id: Ic904d0b1bcb451bcb74cfae2204fb7297eaff108
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/234016
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index c82d575..84fc97e 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -30,6 +30,7 @@
#include "src/gpu/GrTexturePriv.h"
#include "src/gpu/GrTextureProxy.h"
#include "src/gpu/GrTextureProxyPriv.h"
+#include "src/gpu/GrTextureResolveRenderTask.h"
#include "src/gpu/GrTracing.h"
#include "src/gpu/ccpr/GrCoverageCountingPathRenderer.h"
#include "src/gpu/text/GrTextContext.h"
@@ -81,8 +82,16 @@
return false;
}
-void GrDrawingManager::RenderTaskDAG::add(sk_sp<GrRenderTask> renderTask) {
- fRenderTasks.emplace_back(std::move(renderTask));
+GrRenderTask* GrDrawingManager::RenderTaskDAG::add(sk_sp<GrRenderTask> renderTask) {
+ return fRenderTasks.emplace_back(std::move(renderTask)).get();
+}
+
+GrRenderTask* GrDrawingManager::RenderTaskDAG::addBeforeLast(sk_sp<GrRenderTask> renderTask) {
+ SkASSERT(!fRenderTasks.empty());
+ // Release 'fRenderTasks.back()' and grab the raw pointer. If we pass in a reference to an array
+ // element, and the array grows during emplace_back, then the reference will become invalidated.
+ fRenderTasks.emplace_back(fRenderTasks.back().release());
+ return (fRenderTasks[fRenderTasks.count() - 2] = std::move(renderTask)).get();
}
void GrDrawingManager::RenderTaskDAG::add(const SkTArray<sk_sp<GrRenderTask>>& opLists) {
@@ -499,17 +508,18 @@
if (!proxies[i]->isInstantiated()) {
return result;
}
- }
-
- for (int i = 0; i < numProxies; ++i) {
GrSurface* surface = proxies[i]->peekSurface();
if (auto* rt = surface->asRenderTarget()) {
gpu->resolveRenderTarget(rt);
}
- if (auto* tex = surface->asTexture()) {
- if (tex->texturePriv().mipMapped() == GrMipMapped::kYes &&
- tex->texturePriv().mipMapsAreDirty()) {
- gpu->regenerateMipMapLevels(tex);
+ // If, after a flush, any of the proxies of interest have dirty mipmaps, regenerate them in
+ // case their backend textures are being stolen.
+ // (This special case is exercised by the ReimportImageTextureWithMipLevels test.)
+ // FIXME: It may be more ideal to plumb down a "we're going to steal the backends" flag.
+ if (auto* textureProxy = proxies[i]->asTextureProxy()) {
+ if (textureProxy->mipMapsAreDirty()) {
+ gpu->regenerateMipMapLevels(textureProxy->peekTexture());
+ textureProxy->markMipMapsClean();
}
}
}
@@ -669,6 +679,27 @@
return opList;
}
+GrRenderTask* GrDrawingManager::newTextureResolveRenderTask(
+ sk_sp<GrTextureProxy> textureProxy, GrTextureResolveFlags flags, const GrCaps& caps) {
+ // Unlike in the "new opList" cases, we do not want to close the active opList, nor (if we are
+ // in sorting and opList reduction mode) the render tasks that depend on the proxy's current
+ // state. This is because those opLists can still receive new ops and because if they refer to
+ // the mipmapped version of 'textureProxy', they will then come to depend on the render task
+ // being created here.
+ // NOTE: In either case, 'textureProxy' should already be closed at this point (i.e., its state
+ // is finished))
+ SkDEBUGCODE(auto* previousTaskBeforeMipsResolve = textureProxy->getLastRenderTask();)
+ auto textureResolveTask = GrTextureResolveRenderTask::Make(textureProxy, flags, caps);
+ // GrTextureResolveRenderTask::Make should have closed the texture proxy's previous task.
+ SkASSERT(!previousTaskBeforeMipsResolve || previousTaskBeforeMipsResolve->isClosed());
+ SkASSERT(textureProxy->getLastRenderTask() == textureResolveTask.get());
+
+ // Add the new textureResolveTask before the fActiveOpList (if not in
+ // sorting/opList-splitting-reduction mode) because it will depend upon this resolve task.
+ // NOTE: Putting it here will also reduce the amount of work required by the topological sort.
+ return fDAG.addBeforeLast(std::move(textureResolveTask));
+}
+
GrTextContext* GrDrawingManager::getTextContext() {
if (!fTextContext) {
fTextContext = GrTextContext::Make(fOptionsForTextContext);
diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h
index 43c0874..80f006e 100644
--- a/src/gpu/GrDrawingManager.h
+++ b/src/gpu/GrDrawingManager.h
@@ -52,6 +52,13 @@
sk_sp<GrRenderTargetOpList> newRTOpList(sk_sp<GrRenderTargetProxy>, bool managedOpList);
sk_sp<GrTextureOpList> newTextureOpList(sk_sp<GrTextureProxy>);
+ // Create a new, specialized, render task that will regenerate mipmap levels and/or resolve
+ // MSAA (depending on GrTextureResolveFlags). This method will add the new render task to the
+ // list of render tasks and make it depend on the target texture proxy. It is up to the caller
+ // to add any dependencies on the new render task.
+ GrRenderTask* newTextureResolveRenderTask(
+ sk_sp<GrTextureProxy>, GrTextureResolveFlags, const GrCaps&);
+
GrRecordingContext* getContext() { return fContext; }
GrTextContext* getTextContext();
@@ -130,7 +137,8 @@
GrRenderTask* back() { return fRenderTasks.back().get(); }
const GrRenderTask* back() const { return fRenderTasks.back().get(); }
- void add(sk_sp<GrRenderTask>);
+ GrRenderTask* add(sk_sp<GrRenderTask>);
+ GrRenderTask* addBeforeLast(sk_sp<GrRenderTask>);
void add(const SkTArray<sk_sp<GrRenderTask>>&);
void swap(SkTArray<sk_sp<GrRenderTask>>* renderTasks);
diff --git a/src/gpu/GrRenderTargetContext.h b/src/gpu/GrRenderTargetContext.h
index 62b02f8..b334a16 100644
--- a/src/gpu/GrRenderTargetContext.h
+++ b/src/gpu/GrRenderTargetContext.h
@@ -502,6 +502,7 @@
#if GR_TEST_UTILS
bool testingOnly_IsInstantiated() const { return fRenderTargetProxy->isInstantiated(); }
void testingOnly_SetPreserveOpsOnFullClear() { fPreserveOpsOnFullClear_TestingOnly = true; }
+ GrRenderTargetOpList* testingOnly_PeekLastOpList() { return fOpList.get(); }
#endif
protected:
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index 1e170d9..77d42b9 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -356,6 +356,12 @@
: INHERITED(std::move(opMemoryPool), std::move(proxy), auditTrail)
, fLastClipStackGenID(SK_InvalidUniqueID)
SkDEBUGCODE(, fNumClips(0)) {
+ if (GrTextureProxy* textureProxy = fTarget->asTextureProxy()) {
+ if (GrMipMapped::kYes == textureProxy->mipMapped()) {
+ textureProxy->markMipMapsDirty();
+ }
+ }
+ fTarget->setLastRenderTask(this);
}
void GrRenderTargetOpList::deleteOps() {
diff --git a/src/gpu/GrRenderTask.cpp b/src/gpu/GrRenderTask.cpp
index 66b6c3e..46091f7 100644
--- a/src/gpu/GrRenderTask.cpp
+++ b/src/gpu/GrRenderTask.cpp
@@ -24,7 +24,6 @@
: fTarget(std::move(target))
, fUniqueID(CreateUniqueID())
, fFlags(0) {
- fTarget->setLastRenderTask(this);
}
GrRenderTask::~GrRenderTask() {
@@ -69,13 +68,51 @@
}
// Convert from a GrSurface-based dependency to a GrRenderTask one
-void GrRenderTask::addDependency(GrSurfaceProxy* dependedOn, GrMipMapped, GrTextureResolveManager,
+void GrRenderTask::addDependency(GrSurfaceProxy* dependedOn, GrMipMapped mipMapped,
+ GrTextureResolveManager textureResolveManager,
const GrCaps& caps) {
- if (dependedOn->getLastRenderTask()) {
+ GrRenderTask* dependedOnTask = dependedOn->getLastRenderTask();
+ GrTextureProxy* textureProxy = dependedOn->asTextureProxy();
+
+ if (GrMipMapped::kYes == mipMapped) {
+ SkASSERT(textureProxy);
+ if (GrMipMapped::kYes != textureProxy->mipMapped()) {
+ // There are some cases where we might be given a non-mipmapped texture with a mipmap
+ // filter. See skbug.com/7094.
+ mipMapped = GrMipMapped::kNo;
+ }
+ }
+
+ // Does this proxy have mipmaps that need to be regenerated?
+ if (GrMipMapped::kYes == mipMapped && textureProxy->mipMapsAreDirty()) {
+ // We only read our own target during dst reads, and we shouldn't use mipmaps in that case.
+ SkASSERT(dependedOnTask != this);
+
+ // Create an opList that resolves the texture's mipmap data.
+ GrRenderTask* textureResolveTask = textureResolveManager.newTextureResolveRenderTask(
+ sk_ref_sp(textureProxy), GrTextureResolveFlags::kMipMaps, caps);
+
+ // The GrTextureResolveRenderTask factory should have called addDependency (in this
+ // instance, recursively) on the textureProxy.
+ SkASSERT(!dependedOnTask || textureResolveTask->dependsOn(dependedOnTask));
+ SkASSERT(!textureProxy->texPriv().isDeferred() ||
+ textureResolveTask->fDeferredProxies.back() == textureProxy);
+
+ // The GrTextureResolveRenderTask factory should have also marked the mipmaps clean and set
+ // the last opList on the textureProxy to textureResolveTask.
+ SkASSERT(!textureProxy->mipMapsAreDirty());
+ SkASSERT(textureProxy->getLastRenderTask() == textureResolveTask);
+
+ // Fall through and add textureResolveTask as a dependency of "this".
+ dependedOnTask = textureResolveTask;
+ } else if (textureProxy && textureProxy->texPriv().isDeferred()) {
+ fDeferredProxies.push_back(textureProxy);
+ }
+
+ if (dependedOnTask) {
// If it is still receiving dependencies, this GrRenderTask shouldn't be closed
SkASSERT(!this->isClosed());
- GrRenderTask* dependedOnTask = dependedOn->getLastRenderTask();
if (dependedOnTask == this) {
// self-read - presumably for dst reads. We can't make it closed in the self-read case.
} else {
@@ -87,12 +124,6 @@
dependedOnTask->makeClosed(caps);
}
}
-
- if (GrTextureProxy* textureProxy = dependedOn->asTextureProxy()) {
- if (textureProxy->texPriv().isDeferred()) {
- fDeferredProxies.push_back(textureProxy);
- }
- }
}
bool GrRenderTask::dependsOn(const GrRenderTask* dependedOn) const {
diff --git a/src/gpu/GrTextureOpList.cpp b/src/gpu/GrTextureOpList.cpp
index 2b411ec..60c0883 100644
--- a/src/gpu/GrTextureOpList.cpp
+++ b/src/gpu/GrTextureOpList.cpp
@@ -29,6 +29,10 @@
: INHERITED(std::move(opMemoryPool), proxy, auditTrail) {
SkASSERT(fOpMemoryPool);
SkASSERT(!proxy->readOnly());
+ if (GrMipMapped::kYes == proxy->mipMapped()) {
+ proxy->markMipMapsDirty();
+ }
+ fTarget->setLastRenderTask(this);
}
void GrTextureOpList::deleteOp(int index) {
diff --git a/src/gpu/GrTextureProxy.h b/src/gpu/GrTextureProxy.h
index 091d0ca..b6a77be 100644
--- a/src/gpu/GrTextureProxy.h
+++ b/src/gpu/GrTextureProxy.h
@@ -41,6 +41,14 @@
(GrMipMapsStatus::kNotAllocated == fMipMapsStatus));
return GrMipMapped::kYes == fMipMapped && GrMipMapsStatus::kValid != fMipMapsStatus;
}
+ void markMipMapsDirty() {
+ SkASSERT(GrMipMapped::kYes == fMipMapped);
+ fMipMapsStatus = GrMipMapsStatus::kDirty;
+ }
+ void markMipMapsClean() {
+ SkASSERT(GrMipMapped::kYes == fMipMapped);
+ fMipMapsStatus = GrMipMapsStatus::kValid;
+ }
// Returns the GrMipMapped value of the proxy from creation time regardless of whether it has
// been instantiated or not.
diff --git a/src/gpu/GrTextureResolveManager.h b/src/gpu/GrTextureResolveManager.h
index 4b0eea6..c0da50a 100644
--- a/src/gpu/GrTextureResolveManager.h
+++ b/src/gpu/GrTextureResolveManager.h
@@ -9,7 +9,7 @@
#define GrTextureResolveManager_DEFINED
#include "include/core/SkRefCnt.h"
-#include "include/gpu/GrTypes.h"
+#include "src/gpu/GrDrawingManager.h"
class GrCaps;
class GrDrawingManager;
@@ -26,19 +26,14 @@
explicit GrTextureResolveManager(GrDrawingManager* drawingManager)
: fDrawingManager(drawingManager) {}
- enum class ResolveFlags : bool {
- kNone = 0,
- kMipMaps = 1 << 0,
- // TODO: kMSAA = 1 << 1
- };
-
GrRenderTask* newTextureResolveRenderTask(
- sk_sp<GrTextureProxy>, ResolveFlags, const GrCaps&) const;
+ sk_sp<GrTextureProxy> proxy, GrTextureResolveFlags flags, const GrCaps& caps) const {
+ SkASSERT(fDrawingManager);
+ return fDrawingManager->newTextureResolveRenderTask(std::move(proxy), flags, caps);
+ }
private:
GrDrawingManager* fDrawingManager;
};
-GR_MAKE_BITFIELD_CLASS_OPS(GrTextureResolveManager::ResolveFlags);
-
#endif
diff --git a/src/gpu/GrTextureResolveRenderTask.cpp b/src/gpu/GrTextureResolveRenderTask.cpp
new file mode 100644
index 0000000..08df318
--- /dev/null
+++ b/src/gpu/GrTextureResolveRenderTask.cpp
@@ -0,0 +1,64 @@
+/*
+ * Copyright 2019 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "src/gpu/GrTextureResolveRenderTask.h"
+
+#include "src/gpu/GrGpu.h"
+#include "src/gpu/GrMemoryPool.h"
+#include "src/gpu/GrOpFlushState.h"
+#include "src/gpu/GrResourceAllocator.h"
+#include "src/gpu/GrTexturePriv.h"
+
+sk_sp<GrRenderTask> GrTextureResolveRenderTask::Make(
+ sk_sp<GrTextureProxy> textureProxyPtr, GrTextureResolveFlags flags, const GrCaps& caps) {
+ GrTextureProxy* textureProxy = textureProxyPtr.get();
+ sk_sp<GrTextureResolveRenderTask> resolveTask(
+ new GrTextureResolveRenderTask(std::move(textureProxyPtr), flags));
+
+ // Add the target as a dependency: We will read the existing contents of this texture while
+ // generating mipmap levels and/or resolving MSAA.
+ //
+ // NOTE: This must be called before makeClosed.
+ resolveTask->addDependency(
+ textureProxy, GrMipMapped::kNo, GrTextureResolveManager(nullptr), caps);
+ textureProxy->setLastRenderTask(resolveTask.get());
+
+ // We only resolve the texture; nobody should try to do anything else with this opList.
+ resolveTask->makeClosed(caps);
+
+ if (GrTextureResolveFlags::kMipMaps & flags) {
+ SkASSERT(GrMipMapped::kYes == textureProxy->mipMapped());
+ SkASSERT(textureProxy->mipMapsAreDirty());
+ textureProxy->markMipMapsClean();
+ }
+
+ // On some old ISO C++11 compilers, 'resolveTask' will require an explicit std::move() when
+ // returned from this function. This is because its type (sk_sp<GrTextureResolveRenderTask>)
+ // does not match the return type (sk_sp<GrRenderTask>).
+ return std::move(resolveTask);
+}
+
+void GrTextureResolveRenderTask::gatherProxyIntervals(GrResourceAllocator* alloc) const {
+ // This renderTask doesn't have "normal" ops. In this case we still need to add an interval (so
+ // fEndOfOpListOpIndices will remain in sync), so we create a fake op# to capture the fact that
+ // we manipulate fTarget.
+ alloc->addInterval(fTarget.get(), alloc->curOp(), alloc->curOp(),
+ GrResourceAllocator::ActualUse::kYes);
+ alloc->incOps();
+}
+
+bool GrTextureResolveRenderTask::onExecute(GrOpFlushState* flushState) {
+ GrTexture* texture = fTarget->peekTexture();
+ SkASSERT(texture);
+
+ if (GrTextureResolveFlags::kMipMaps & fResolveFlags) {
+ SkASSERT(texture->texturePriv().mipMapsAreDirty());
+ flushState->gpu()->regenerateMipMapLevels(texture);
+ }
+
+ return true;
+}
diff --git a/src/gpu/GrTextureResolveRenderTask.h b/src/gpu/GrTextureResolveRenderTask.h
new file mode 100644
index 0000000..eadcdca
--- /dev/null
+++ b/src/gpu/GrTextureResolveRenderTask.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2019 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrTextureResolveRenderTask_DEFINED
+#define GrTextureResolveRenderTask_DEFINED
+
+#include "src/gpu/GrRenderTask.h"
+
+class GrTextureResolveRenderTask final : public GrRenderTask {
+public:
+ static sk_sp<GrRenderTask> Make(
+ sk_sp<GrTextureProxy>, GrTextureResolveFlags, const GrCaps&);
+
+private:
+ GrTextureResolveRenderTask(sk_sp<GrTextureProxy> textureProxy, GrTextureResolveFlags flags)
+ : GrRenderTask(std::move(textureProxy))
+ , fResolveFlags(flags) {
+ SkASSERT(GrTextureResolveFlags::kNone != fResolveFlags);
+ }
+
+ void onPrepare(GrOpFlushState*) override {}
+ bool onIsUsed(GrSurfaceProxy* proxy) const override {
+ SkASSERT(proxy != fTarget.get()); // This case should be handled by GrRenderTask.
+ return false;
+ }
+ void handleInternalAllocationFailure() override {}
+ void gatherProxyIntervals(GrResourceAllocator*) const override;
+ bool onExecute(GrOpFlushState*) override;
+
+ const GrTextureResolveFlags fResolveFlags;
+};
+
+#endif
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 8bb7713..a2280b4 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -1859,17 +1859,26 @@
int numPrimitiveProcessorTextureSets) {
auto genLevelsIfNeeded = [this](GrTexture* tex, const GrSamplerState& sampler) {
SkASSERT(tex);
- if (sampler.filter() == GrSamplerState::Filter::kMipMap &&
- tex->texturePriv().mipMapped() == GrMipMapped::kYes &&
- tex->texturePriv().mipMapsAreDirty()) {
- SkASSERT(this->caps()->mipMapSupport());
- this->regenerateMipMapLevels(static_cast<GrGLTexture*>(tex));
- SkASSERT(!tex->asRenderTarget() || !tex->asRenderTarget()->needsResolve());
- } else if (auto* rt = tex->asRenderTarget()) {
- if (rt->needsResolve()) {
- this->resolveRenderTarget(rt);
+ auto* rt = tex->asRenderTarget();
+ if (rt && rt->needsResolve()) {
+ this->resolveRenderTarget(rt);
+ // TEMPORARY: MSAA resolve will have dirtied mipmaps. This goes away once we switch
+ // to resolving MSAA from the opList as well.
+ if (GrSamplerState::Filter::kMipMap == sampler.filter() &&
+ (tex->width() != 1 || tex->height() != 1)) {
+ SkASSERT(tex->texturePriv().mipMapped() == GrMipMapped::kYes);
+ SkASSERT(tex->texturePriv().mipMapsAreDirty());
+ this->regenerateMipMapLevels(tex);
}
}
+ // Ensure mipmaps were all resolved ahead of time by the opList.
+ if (GrSamplerState::Filter::kMipMap == sampler.filter() &&
+ (tex->width() != 1 || tex->height() != 1)) {
+ // There are some cases where we might be given a non-mipmapped texture with a mipmap
+ // filter. See skbug.com/7094.
+ SkASSERT(tex->texturePriv().mipMapped() != GrMipMapped::kYes ||
+ !tex->texturePriv().mipMapsAreDirty());
+ }
};
for (int set = 0, tex = 0; set < numPrimitiveProcessorTextureSets; ++set) {
diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
index 1d7789e..ee9f285 100644
--- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
@@ -744,17 +744,23 @@
GrVkTexture* vkTexture = static_cast<GrVkTexture*>(texture);
// We may need to resolve the texture first if it is also a render target
GrVkRenderTarget* texRT = static_cast<GrVkRenderTarget*>(vkTexture->asRenderTarget());
- if (texRT) {
+ if (texRT && texRT->needsResolve()) {
fGpu->resolveRenderTargetNoFlush(texRT);
+ // TEMPORARY: MSAA resolve will have dirtied mipmaps. This goes away once we switch
+ // to resolving MSAA from the opList as well.
+ if (GrSamplerState::Filter::kMipMap == filter &&
+ (vkTexture->width() != 1 || vkTexture->height() != 1)) {
+ SkASSERT(vkTexture->texturePriv().mipMapped() == GrMipMapped::kYes);
+ SkASSERT(vkTexture->texturePriv().mipMapsAreDirty());
+ fGpu->regenerateMipMapLevels(vkTexture);
+ }
}
- // Check if we need to regenerate any mip maps
+ // Ensure mip maps were all resolved ahead of time by the opList.
if (GrSamplerState::Filter::kMipMap == filter &&
(vkTexture->width() != 1 || vkTexture->height() != 1)) {
SkASSERT(vkTexture->texturePriv().mipMapped() == GrMipMapped::kYes);
- if (vkTexture->texturePriv().mipMapsAreDirty()) {
- fGpu->regenerateMipMapLevels(vkTexture);
- }
+ SkASSERT(!vkTexture->texturePriv().mipMapsAreDirty());
}
cbInfo.fSampledTextures.push_back(vkTexture);