From 8a02f65c5cc16d010f188c34861b03d96cb8ec10 Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Fri, 12 May 2017 14:49:16 -0400 Subject: [PATCH] Switch ImageStorageAccess over to GrTextureProxies Split out of: https://skia-review.googlesource.com/c/10484/ (Omnibus: Push instantiation of GrTextures later (post TextureSampler)) Change-Id: I341de6ae121620d30e50bff21450878a18bdf4f2 Reviewed-on: https://skia-review.googlesource.com/16714 Reviewed-by: Brian Salomon Commit-Queue: Robert Phillips --- include/private/GrTextureProxy.h | 8 +++++ src/gpu/GrProcessor.cpp | 27 ++++++++++------ src/gpu/GrProcessor.h | 27 ++++++++++------ src/gpu/GrProgramDesc.cpp | 2 +- src/gpu/GrSurfaceProxyPriv.h | 2 +- src/gpu/glsl/GrGLSLProgramBuilder.cpp | 2 +- tests/ImageStorageTest.cpp | 16 ++++------ tests/ProcessorTest.cpp | 45 ++++++++++++++------------- 8 files changed, 76 insertions(+), 53 deletions(-) diff --git a/include/private/GrTextureProxy.h b/include/private/GrTextureProxy.h index 61e8020b5a..e14e285d87 100644 --- a/include/private/GrTextureProxy.h +++ b/include/private/GrTextureProxy.h @@ -28,6 +28,14 @@ public: GrSamplerParams::FilterMode highestFilterMode() const; + GrSLType imageStorageType() const { + if (GrPixelConfigIsSint(this->config())) { + return kIImageStorage2D_GrSLType; + } else { + return kImageStorage2D_GrSLType; + } + } + protected: friend class GrSurfaceProxy; // for ctors diff --git a/src/gpu/GrProcessor.cpp b/src/gpu/GrProcessor.cpp index 57a8008dae..1533257bad 100644 --- a/src/gpu/GrProcessor.cpp +++ b/src/gpu/GrProcessor.cpp @@ -129,6 +129,8 @@ void GrProcessor::operator delete(void* target) { /////////////////////////////////////////////////////////////////////////////// void GrResourceIOProcessor::addTextureSampler(const TextureSampler* access) { + // MDB TODO: this 'isBad' call checks to ensure the underlying texture exists. It needs to + // be moved later. if (access->isBad()) { this->markAsBad(); } @@ -140,7 +142,13 @@ void GrResourceIOProcessor::addBufferAccess(const BufferAccess* access) { fBufferAccesses.push_back(access); } -void GrResourceIOProcessor::addImageStorageAccess(const ImageStorageAccess* access) { +void GrResourceIOProcessor::addImageStorageAccess(GrResourceProvider* resourceProvider, + const ImageStorageAccess* access) { + // MDB TODO: this 'isBad' call attempts to instantiate 'access'. It needs to be moved later. + if (access->isBad(resourceProvider)) { + this->markAsBad(); + } + fImageStorageAccesses.push_back(access); } @@ -152,7 +160,7 @@ void GrResourceIOProcessor::addPendingIOs() const { buffer->programBuffer()->markPendingIO(); } for (const auto& imageStorage : fImageStorageAccesses) { - imageStorage->programTexture()->markPendingIO(); + imageStorage->programProxy()->markPendingIO(); } } @@ -164,7 +172,7 @@ void GrResourceIOProcessor::removeRefs() const { buffer->programBuffer()->removeRef(); } for (const auto& imageStorage : fImageStorageAccesses) { - imageStorage->programTexture()->removeRef(); + imageStorage->programProxy()->removeRef(); } } @@ -176,7 +184,7 @@ void GrResourceIOProcessor::pendingIOComplete() const { buffer->programBuffer()->pendingIOComplete(); } for (const auto& imageStorage : fImageStorageAccesses) { - imageStorage->programTexture()->pendingIOComplete(); + imageStorage->programProxy()->pendingIOComplete(); } } @@ -272,19 +280,20 @@ void GrResourceIOProcessor::TextureSampler::reset(GrResourceProvider* resourcePr /////////////////////////////////////////////////////////////////////////////////////////////////// -GrResourceIOProcessor::ImageStorageAccess::ImageStorageAccess(sk_sp texture, +GrResourceIOProcessor::ImageStorageAccess::ImageStorageAccess(sk_sp proxy, GrIOType ioType, GrSLMemoryModel memoryModel, GrSLRestrict restrict, - GrShaderFlags visibility) { - SkASSERT(texture); - fTexture.set(texture.release(), ioType); + GrShaderFlags visibility) + : fProxyRef(std::move(proxy), ioType) { + SkASSERT(fProxyRef.getProxy()); + fMemoryModel = memoryModel; fRestrict = restrict; fVisibility = visibility; // We currently infer this from the config. However, we could allow the client to specify // a format that is different but compatible with the config. - switch (fTexture.get()->config()) { + switch (fProxyRef.getProxy()->config()) { case kRGBA_8888_GrPixelConfig: fFormat = GrImageStorageFormat::kRGBA8; break; diff --git a/src/gpu/GrProcessor.h b/src/gpu/GrProcessor.h index 7011267c76..027a34e334 100644 --- a/src/gpu/GrProcessor.h +++ b/src/gpu/GrProcessor.h @@ -15,6 +15,7 @@ #include "GrProgramElement.h" #include "GrSamplerParams.h" #include "GrShaderVar.h" +#include "GrSurfaceProxyPriv.h" #include "SkMath.h" #include "SkString.h" #include "../private/SkAtomics.h" @@ -189,7 +190,7 @@ protected: */ void addTextureSampler(const TextureSampler*); void addBufferAccess(const BufferAccess*); - void addImageStorageAccess(const ImageStorageAccess*); + void addImageStorageAccess(GrResourceProvider* resourceProvider, const ImageStorageAccess*); bool hasSameSamplersAndAccesses(const GrResourceIOProcessor&) const; @@ -324,33 +325,39 @@ private: */ class GrResourceIOProcessor::ImageStorageAccess : public SkNoncopyable { public: - ImageStorageAccess(sk_sp texture, GrIOType ioType, GrSLMemoryModel, GrSLRestrict, + ImageStorageAccess(sk_sp, GrIOType, GrSLMemoryModel, GrSLRestrict, GrShaderFlags visibility = kFragment_GrShaderFlag); bool operator==(const ImageStorageAccess& that) const { - return this->texture() == that.texture() && fVisibility == that.fVisibility; + return this->proxy() == that.proxy() && fVisibility == that.fVisibility; } bool operator!=(const ImageStorageAccess& that) const { return !(*this == that); } - GrTexture* texture() const { return fTexture.get(); } + GrTexture* texture() const { return fProxyRef.getProxy()->priv().peekTexture(); } + GrTextureProxy* proxy() const { return fProxyRef.getProxy()->asTextureProxy(); } GrShaderFlags visibility() const { return fVisibility; } - GrIOType ioType() const { return fTexture.ioType(); } + GrIOType ioType() const { return fProxyRef.ioType(); } GrImageStorageFormat format() const { return fFormat; } GrSLMemoryModel memoryModel() const { return fMemoryModel; } GrSLRestrict restrict() const { return fRestrict; } + // MDB: In the future this should be renamed instantiate + bool isBad(GrResourceProvider* resourceProvider) const { + return SkToBool(!fProxyRef.getProxy()->instantiate(resourceProvider)); + } + /** * For internal use by GrProcessor. */ - const GrGpuResourceRef* programTexture() const { return &fTexture; } + const GrSurfaceProxyRef* programProxy() const { return &fProxyRef; } private: - GrTGpuResourceRef fTexture; - GrShaderFlags fVisibility; + GrSurfaceProxyRef fProxyRef; + GrShaderFlags fVisibility; GrImageStorageFormat fFormat; - GrSLMemoryModel fMemoryModel; - GrSLRestrict fRestrict; + GrSLMemoryModel fMemoryModel; + GrSLRestrict fRestrict; typedef SkNoncopyable INHERITED; }; diff --git a/src/gpu/GrProgramDesc.cpp b/src/gpu/GrProgramDesc.cpp index 86c653d647..41993e190d 100644 --- a/src/gpu/GrProgramDesc.cpp +++ b/src/gpu/GrProgramDesc.cpp @@ -62,7 +62,7 @@ static uint16_t sampler_key(GrSLType samplerType, GrPixelConfig config, GrShader } static uint16_t storage_image_key(const GrResourceIOProcessor::ImageStorageAccess& imageAccess) { - GrSLType type = imageAccess.texture()->texturePriv().imageStorageType(); + GrSLType type = imageAccess.proxy()->imageStorageType(); return image_storage_or_sampler_uniform_type_key(type) | (int)imageAccess.format() << kSamplerOrImageTypeKeyBits; } diff --git a/src/gpu/GrSurfaceProxyPriv.h b/src/gpu/GrSurfaceProxyPriv.h index 24c69939b7..0d0da4ec0d 100644 --- a/src/gpu/GrSurfaceProxyPriv.h +++ b/src/gpu/GrSurfaceProxyPriv.h @@ -17,7 +17,7 @@ class GrSurfaceProxyPriv { public: // If the proxy is already instantiated, return its backing GrTexture; if not, // return null - const GrTexture* peekTexture() const { + GrTexture* peekTexture() const { return fProxy->fTarget ? fProxy->fTarget->asTexture() : nullptr; } diff --git a/src/gpu/glsl/GrGLSLProgramBuilder.cpp b/src/gpu/glsl/GrGLSLProgramBuilder.cpp index 5c3bee1b4e..96b3359d4e 100644 --- a/src/gpu/glsl/GrGLSLProgramBuilder.cpp +++ b/src/gpu/glsl/GrGLSLProgramBuilder.cpp @@ -358,7 +358,7 @@ GrGLSLProgramBuilder::ImageStorageHandle GrGLSLProgramBuilder::emitImageStorage( if (access.visibility() & kFragment_GrShaderFlag) { ++fNumFragmentImageStorages; } - GrSLType uniformType = access.texture()->texturePriv().imageStorageType(); + GrSLType uniformType = access.proxy()->imageStorageType(); return this->uniformHandler()->addImageStorage(access.visibility(), uniformType, access.format(), access.memoryModel(), access.restrict(), access.ioType(), name); diff --git a/tests/ImageStorageTest.cpp b/tests/ImageStorageTest.cpp index dc4d21294d..aa81118b69 100644 --- a/tests/ImageStorageTest.cpp +++ b/tests/ImageStorageTest.cpp @@ -24,23 +24,19 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ImageStorageLoad, reporter, ctxInfo) { sk_sp proxy, GrSLMemoryModel mm, GrSLRestrict restrict) { - // MDB TODO: remove this once ImageStorageAccess is converted to GrTextureProxy - sk_sp tex(sk_ref_sp(proxy->instantiate(resourceProvider))); - if (!tex) { - return nullptr; - } - - return sk_sp(new TestFP(std::move(tex), mm, restrict)); + return sk_sp(new TestFP(resourceProvider, + std::move(proxy), mm, restrict)); } const char* name() const override { return "Image Load Test FP"; } private: - TestFP(sk_sp texture, GrSLMemoryModel mm, GrSLRestrict restrict) + TestFP(GrResourceProvider* resourceProvider, + sk_sp proxy, GrSLMemoryModel mm, GrSLRestrict restrict) : INHERITED(kNone_OptimizationFlags) - , fImageStorageAccess(std::move(texture), kRead_GrIOType, mm, restrict) { + , fImageStorageAccess(std::move(proxy), kRead_GrIOType, mm, restrict) { this->initClassID(); - this->addImageStorageAccess(&fImageStorageAccess); + this->addImageStorageAccess(resourceProvider, &fImageStorageAccess); } void onGetGLSLProcessorKey(const GrShaderCaps&, GrProcessorKeyBuilder*) const override {} diff --git a/tests/ProcessorTest.cpp b/tests/ProcessorTest.cpp index ee89ff3739..51696152c2 100644 --- a/tests/ProcessorTest.cpp +++ b/tests/ProcessorTest.cpp @@ -47,8 +47,8 @@ private: class TestFP : public GrFragmentProcessor { public: struct Image { - Image(sk_sp texture, GrIOType ioType) : fTexture(texture), fIOType(ioType) {} - sk_sp fTexture; + Image(sk_sp proxy, GrIOType ioType) : fProxy(proxy), fIOType(ioType) {} + sk_sp fProxy; GrIOType fIOType; }; static sk_sp Make(sk_sp child) { @@ -82,8 +82,9 @@ private: this->addBufferAccess(&fBuffers.emplace_back(kRGBA_8888_GrPixelConfig, buffer.get())); } for (const Image& image : images) { - this->addImageStorageAccess(&fImages.emplace_back( - image.fTexture, image.fIOType, GrSLMemoryModel::kNone, GrSLRestrict::kNo)); + fImages.emplace_back(image.fProxy, image.fIOType, + GrSLMemoryModel::kNone, GrSLRestrict::kNo); + this->addImageStorageAccess(context->resourceProvider(), &fImages.back()); } } @@ -143,15 +144,17 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(ProcessorRefTest, reporter, ctxInfo) { bool texelBufferSupport = context->caps()->shaderCaps()->texelBufferSupport(); bool imageLoadStoreSupport = context->caps()->shaderCaps()->imageLoadStoreSupport(); sk_sp proxy1(GrSurfaceProxy::MakeDeferred(context->resourceProvider(), - desc, - SkBackingFit::kExact, + desc, SkBackingFit::kExact, + SkBudgeted::kYes)); + sk_sp proxy2(GrSurfaceProxy::MakeDeferred(context->resourceProvider(), + desc, SkBackingFit::kExact, + SkBudgeted::kYes)); + sk_sp proxy3(GrSurfaceProxy::MakeDeferred(context->resourceProvider(), + desc, SkBackingFit::kExact, + SkBudgeted::kYes)); + sk_sp proxy4(GrSurfaceProxy::MakeDeferred(context->resourceProvider(), + desc, SkBackingFit::kExact, SkBudgeted::kYes)); - sk_sp texture2 = - context->resourceProvider()->createTexture(desc, SkBudgeted::kYes); - sk_sp texture3 = - context->resourceProvider()->createTexture(desc, SkBudgeted::kYes); - sk_sp texture4 = - context->resourceProvider()->createTexture(desc, SkBudgeted::kYes); sk_sp buffer(texelBufferSupport ? context->resourceProvider()->createBuffer( 1024, GrBufferType::kTexel_GrBufferType, @@ -166,9 +169,9 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(ProcessorRefTest, reporter, ctxInfo) { buffers.push_back(buffer); } if (imageLoadStoreSupport) { - images.emplace_back(texture2, GrIOType::kRead_GrIOType); - images.emplace_back(texture3, GrIOType::kWrite_GrIOType); - images.emplace_back(texture4, GrIOType::kRW_GrIOType); + images.emplace_back(proxy2, GrIOType::kRead_GrIOType); + images.emplace_back(proxy3, GrIOType::kWrite_GrIOType); + images.emplace_back(proxy4, GrIOType::kRW_GrIOType); } std::unique_ptr op(TestOp::Make()); GrPaint paint; @@ -196,17 +199,17 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(ProcessorRefTest, reporter, ctxInfo) { } if (imageLoadStoreSupport) { - testingOnly_getIORefCnts(texture2.get(), &refCnt, &readCnt, &writeCnt); + testingOnly_getIORefCnts(proxy2.get(), &refCnt, &readCnt, &writeCnt); REPORTER_ASSERT(reporter, 1 == refCnt); REPORTER_ASSERT(reporter, 1 == readCnt); REPORTER_ASSERT(reporter, 0 == writeCnt); - testingOnly_getIORefCnts(texture3.get(), &refCnt, &readCnt, &writeCnt); + testingOnly_getIORefCnts(proxy3.get(), &refCnt, &readCnt, &writeCnt); REPORTER_ASSERT(reporter, 1 == refCnt); REPORTER_ASSERT(reporter, 0 == readCnt); REPORTER_ASSERT(reporter, 1 == writeCnt); - testingOnly_getIORefCnts(texture4.get(), &refCnt, &readCnt, &writeCnt); + testingOnly_getIORefCnts(proxy4.get(), &refCnt, &readCnt, &writeCnt); REPORTER_ASSERT(reporter, 1 == refCnt); REPORTER_ASSERT(reporter, 1 == readCnt); REPORTER_ASSERT(reporter, 1 == writeCnt); @@ -227,17 +230,17 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(ProcessorRefTest, reporter, ctxInfo) { } if (texelBufferSupport) { - testingOnly_getIORefCnts(texture2.get(), &refCnt, &readCnt, &writeCnt); + testingOnly_getIORefCnts(proxy2.get(), &refCnt, &readCnt, &writeCnt); REPORTER_ASSERT(reporter, 1 == refCnt); REPORTER_ASSERT(reporter, 0 == readCnt); REPORTER_ASSERT(reporter, 0 == writeCnt); - testingOnly_getIORefCnts(texture3.get(), &refCnt, &readCnt, &writeCnt); + testingOnly_getIORefCnts(proxy3.get(), &refCnt, &readCnt, &writeCnt); REPORTER_ASSERT(reporter, 1 == refCnt); REPORTER_ASSERT(reporter, 0 == readCnt); REPORTER_ASSERT(reporter, 0 == writeCnt); - testingOnly_getIORefCnts(texture4.get(), &refCnt, &readCnt, &writeCnt); + testingOnly_getIORefCnts(proxy4.get(), &refCnt, &readCnt, &writeCnt); REPORTER_ASSERT(reporter, 1 == refCnt); REPORTER_ASSERT(reporter, 0 == readCnt); REPORTER_ASSERT(reporter, 0 == writeCnt);