From c50fefbba79ae29edeb02e291301b3cc3a29ad1d Mon Sep 17 00:00:00 2001 From: Chris Dalton Date: Fri, 3 Sep 2021 19:46:02 -0600 Subject: [PATCH] Defer the attachment of GL stencil buffers This will allow us to use a single FBO for EXT_multisampled_render_to_texture, that we modify on-demand depending on whether we need MSAA. Bug: chromium:1222095 Change-Id: Ife2d743e28833521d785e4bf0e20de593c492a9a Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442736 Commit-Queue: Jim Van Verth Reviewed-by: Brian Salomon --- src/gpu/gl/GrGLCaps.cpp | 3 +- src/gpu/gl/GrGLGpu.cpp | 51 +++++--------- src/gpu/gl/GrGLGpu.h | 5 +- src/gpu/gl/GrGLRenderTarget.cpp | 118 +++++++++++++++++++------------- src/gpu/gl/GrGLRenderTarget.h | 36 +++++++++- 5 files changed, 122 insertions(+), 91 deletions(-) diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp index ca9d98854e..c0b535dbc6 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/src/gpu/gl/GrGLCaps.cpp @@ -4277,8 +4277,7 @@ GrCaps::SurfaceReadPixelsSupport GrGLCaps::surfaceSupportsReadPixels( } else if (auto rt = static_cast(surface->asRenderTarget())) { // glReadPixels does not allow reading back from a MSAA framebuffer. If the underlying // GrSurface doesn't have a second FBO to resolve to then we must make a copy. - if (rt->numSamples() > 1 && - rt->singleSampleFBOID() == GrGLRenderTarget::kUnresolvableFBOID) { + if (rt->numSamples() > 1 && !rt->asTexture()) { return SurfaceReadPixelsSupport::kCopyToTexture2D; } } diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp index 754273e3af..24a4235aed 100644 --- a/src/gpu/gl/GrGLGpu.cpp +++ b/src/gpu/gl/GrGLGpu.cpp @@ -1996,14 +1996,12 @@ void GrGLGpu::endCommandBuffer(GrGLRenderTarget* rt, bool useMultisampleFBO, if (GrGLCaps::kNone_InvalidateFBType != this->glCaps().invalidateFBType()) { SkSTArray<2, GrGLenum> discardAttachments; if (GrStoreOp::kDiscard == colorLoadStore.fStoreOp) { - GrGLuint renderFBOID = (useMultisampleFBO) ? rt->multisampleFBOID() - : rt->singleSampleFBOID(); - discardAttachments.push_back((!renderFBOID) ? GR_GL_COLOR : GR_GL_COLOR_ATTACHMENT0); + discardAttachments.push_back( + rt->isFBO0(useMultisampleFBO) ? GR_GL_COLOR : GR_GL_COLOR_ATTACHMENT0); } if (GrStoreOp::kDiscard == stencilLoadStore.fStoreOp) { - GrGLuint renderFBOID = (useMultisampleFBO) ? rt->multisampleFBOID() - : rt->singleSampleFBOID(); - discardAttachments.push_back((!renderFBOID) ? GR_GL_STENCIL : GR_GL_STENCIL_ATTACHMENT); + discardAttachments.push_back( + rt->isFBO0(useMultisampleFBO) ? GR_GL_STENCIL : GR_GL_STENCIL_ATTACHMENT); } @@ -2101,11 +2099,11 @@ bool GrGLGpu::readOrTransferPixelsFrom(GrSurface* surface, if (renderTarget) { // Always bind the single sample FBO since we can't read pixels from an MSAA framebuffer. - if (renderTarget->numSamples() > 1 && - renderTarget->singleSampleFBOID() == GrGLRenderTarget::kUnresolvableFBOID) { + constexpr bool useMultisampleFBO = false; + if (renderTarget->numSamples() > 1 && renderTarget->isFBO0(useMultisampleFBO)) { return false; } - this->flushRenderTargetNoColorWrites(renderTarget, false/*useMultisampleFBO*/); + this->flushRenderTargetNoColorWrites(renderTarget, useMultisampleFBO); } else { // Use a temporary FBO. this->bindSurfaceFBOForPixelOps(surface, 0, GR_GL_FRAMEBUFFER, kSrc_TempFBOTarget); @@ -2206,8 +2204,7 @@ void GrGLGpu::flushRenderTargetNoColorWrites(GrGLRenderTarget* target, bool useM SkASSERT(target); GrGpuResource::UniqueID rtID = target->uniqueID(); if (fHWBoundRenderTargetUniqueID != rtID || fHWBoundFramebufferIsMSAA != useMultisampleFBO) { - this->bindFramebuffer(GR_GL_FRAMEBUFFER, (useMultisampleFBO) ? target->multisampleFBOID() - : target->singleSampleFBOID()); + target->bind(useMultisampleFBO); #ifdef SK_DEBUG // don't do this check in Chromium -- this is causing // lots of repeated command buffer flushes when the compositor is @@ -2310,23 +2307,10 @@ void GrGLGpu::resolveRenderFBOs(GrGLRenderTarget* rt, const SkIRect& resolveRect ResolveDirection resolveDirection, bool invalidateReadBufferAfterBlit) { this->handleDirtyContext(); - - // If the multisample FBO is nonzero, it means we always have something to resolve (even if the - // single sample buffer is FBO 0). If it's zero, then there's nothing to resolve. - SkASSERT(rt->multisampleFBOID() != 0); + rt->bindForResolve(resolveDirection); const GrGLCaps& caps = this->glCaps(); - if (resolveDirection == ResolveDirection::kMSAAToSingle) { - this->bindFramebuffer(GR_GL_READ_FRAMEBUFFER, rt->multisampleFBOID()); - this->bindFramebuffer(GR_GL_DRAW_FRAMEBUFFER, rt->singleSampleFBOID()); - } else { - SkASSERT(resolveDirection == ResolveDirection::kSingleToMSAA); - SkASSERT(this->glCaps().canResolveSingleToMSAA()); - this->bindFramebuffer(GR_GL_READ_FRAMEBUFFER, rt->singleSampleFBOID()); - this->bindFramebuffer(GR_GL_DRAW_FRAMEBUFFER, rt->multisampleFBOID()); - } - // make sure we go through flushRenderTarget() since we've modified // the bound DRAW FBO ID. fHWBoundRenderTargetUniqueID.makeInvalid(); @@ -2360,16 +2344,15 @@ void GrGLGpu::resolveRenderFBOs(GrGLRenderTarget* rt, const SkIRect& resolveRect invalidateReadBufferAfterBlit) { // Invalidate the read FBO attachment after the blit, in hopes that this allows the driver // to perform tiling optimizations. - GrGLenum colorDiscardAttachment = (rt->multisampleFBOID() == 0) ? GR_GL_COLOR - : GR_GL_COLOR_ATTACHMENT0; + bool readBufferIsMSAA = resolveDirection == ResolveDirection::kMSAAToSingle; + GrGLenum colorDiscardAttachment = rt->isFBO0(readBufferIsMSAA) ? GR_GL_COLOR + : GR_GL_COLOR_ATTACHMENT0; if (caps.invalidateFBType() == GrGLCaps::kInvalidate_InvalidateFBType) { GL_CALL(InvalidateFramebuffer(GR_GL_READ_FRAMEBUFFER, 1, &colorDiscardAttachment)); } else { SkASSERT(caps.invalidateFBType() == GrGLCaps::kDiscard_InvalidateFBType); // glDiscardFramebuffer only accepts GL_FRAMEBUFFER. - GrGLuint discardFBO = (resolveDirection == ResolveDirection::kMSAAToSingle) - ? rt->multisampleFBOID() : rt->singleSampleFBOID(); - this->bindFramebuffer(GR_GL_FRAMEBUFFER, discardFBO); + rt->bind(readBufferIsMSAA); GL_CALL(DiscardFramebuffer(GR_GL_FRAMEBUFFER, 1, &colorDiscardAttachment)); } } @@ -2858,7 +2841,7 @@ static bool rt_has_msaa_render_buffer(const GrGLRenderTarget* rt, const GrGLCaps // 1) It's multisampled // 2) We're using an extension with separate MSAA renderbuffers // 3) It's not FBO 0, which is special and always auto-resolves - return rt->numSamples() > 1 && glCaps.usesMSAARenderBuffers() && rt->multisampleFBOID() != 0; + return rt->numSamples() > 1 && glCaps.usesMSAARenderBuffers() && !rt->isFBO0(true/*msaa*/); } static inline bool can_copy_texsubimage(const GrSurface* dst, const GrSurface* src, @@ -2914,10 +2897,8 @@ void GrGLGpu::bindSurfaceFBOForPixelOps(GrSurface* surface, int mipLevel, GrGLen if (mipLevel == 0) { texture->baseLevelWasBoundToFBO(); } - } else if (rt->numSamples() > 1) { - this->bindFramebuffer(fboTarget, rt->multisampleFBOID()); } else { - this->bindFramebuffer(fboTarget, rt->singleSampleFBOID()); + rt->bindForPixelOps(fboTarget); } } @@ -3540,7 +3521,7 @@ void GrGLGpu::xferBarrier(GrRenderTarget* rt, GrXferBarrierType type) { case kTexture_GrXferBarrierType: { GrGLRenderTarget* glrt = static_cast(rt); SkASSERT(glrt->asTexture()); - SkASSERT(glrt->singleSampleFBOID() != 0); + SkASSERT(!glrt->isFBO0(false/*multisample*/)); if (glrt->requiresManualMSAAResolve()) { // The render target uses separate storage so no need for glTextureBarrier. // FIXME: The render target will resolve automatically when its texture is bound, diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h index 8d279f09aa..cd4e7a5d4a 100644 --- a/src/gpu/gl/GrGLGpu.h +++ b/src/gpu/gl/GrGLGpu.h @@ -103,10 +103,7 @@ public: // Applies any necessary workarounds and returns the GL primitive type to use in draw calls. GrGLenum prepareToDraw(GrPrimitiveType primitiveType); - enum class ResolveDirection : bool { - kSingleToMSAA, // glCaps.canResolveSingleToMSAA() must be true. - kMSAAToSingle - }; + using ResolveDirection = GrGLRenderTarget::ResolveDirection; // Resolves the render target's single sample FBO into the MSAA, or vice versa. // If glCaps.framebufferResolvesMustBeFullSize() is true, resolveRect must be equal the render diff --git a/src/gpu/gl/GrGLRenderTarget.cpp b/src/gpu/gl/GrGLRenderTarget.cpp index fd1716aaed..616f05dfc5 100644 --- a/src/gpu/gl/GrGLRenderTarget.cpp +++ b/src/gpu/gl/GrGLRenderTarget.cpp @@ -18,6 +18,7 @@ #define GPUGL static_cast(this->getGpu()) #define GL_CALL(X) GR_GL_CALL(GPUGL->glInterface(), X) +#define GL_CALL_RET(RET, X) GR_GL_CALL_RET(GPUGL->glInterface(), RET, X) // Because this class is virtually derived from GrSurface we must explicitly call its constructor. // Constructor for wrapped render targets. @@ -131,54 +132,8 @@ size_t GrGLRenderTarget::onGpuMemorySize() const { } bool GrGLRenderTarget::completeStencilAttachment(GrAttachment* stencil, bool useMultisampleFBO) { - GrGLGpu* gpu = this->getGLGpu(); - const GrGLInterface* interface = gpu->glInterface(); - - if (this->numSamples() == 1 && useMultisampleFBO) { - // We will be rendering to the dynamic msaa fbo. Make sure to initialize it first. - if (!this->ensureDynamicMSAAAttachment()) { - return false; - } - } - - GrGLuint stencilFBOID = (useMultisampleFBO) ? fMultisampleFBOID : fSingleSampleFBOID; - gpu->bindFramebuffer(GR_GL_FRAMEBUFFER, stencilFBOID); - - if (nullptr == stencil) { - GR_GL_CALL(interface, FramebufferRenderbuffer(GR_GL_FRAMEBUFFER, - GR_GL_STENCIL_ATTACHMENT, - GR_GL_RENDERBUFFER, 0)); - GR_GL_CALL(interface, FramebufferRenderbuffer(GR_GL_FRAMEBUFFER, - GR_GL_DEPTH_ATTACHMENT, - GR_GL_RENDERBUFFER, 0)); - } else { - const GrGLAttachment* glStencil = static_cast(stencil); - GrGLuint rb = glStencil->renderbufferID(); - GR_GL_CALL(interface, FramebufferRenderbuffer(GR_GL_FRAMEBUFFER, - GR_GL_STENCIL_ATTACHMENT, - GR_GL_RENDERBUFFER, rb)); - if (GrGLFormatIsPackedDepthStencil(glStencil->format())) { - GR_GL_CALL(interface, FramebufferRenderbuffer(GR_GL_FRAMEBUFFER, - GR_GL_DEPTH_ATTACHMENT, - GR_GL_RENDERBUFFER, rb)); - } else { - GR_GL_CALL(interface, FramebufferRenderbuffer(GR_GL_FRAMEBUFFER, - GR_GL_DEPTH_ATTACHMENT, - GR_GL_RENDERBUFFER, 0)); - } - } - -#ifdef SK_DEBUG - if (!gpu->glCaps().skipErrorChecks()) { - GrGLenum status; - GR_GL_CALL_RET(interface, status, CheckFramebufferStatus(GR_GL_FRAMEBUFFER)); - if (status != GR_GL_FRAMEBUFFER_COMPLETE) { - // This can fail if the context has been asynchronously abandoned (see skbug.com/5200). - return false; - } - } -#endif - + // We defer attaching the new stencil buffer until the next time our framebuffer is bound. + fStencilAttachmentIsValid[useMultisampleFBO] = false; return true; } @@ -226,6 +181,73 @@ bool GrGLRenderTarget::ensureDynamicMSAAAttachment() { return true; } +void GrGLRenderTarget::bindInternal(GrGLenum fboTarget, bool useMultisampleFBO) { + GrGLuint fboId = useMultisampleFBO ? fMultisampleFBOID : fSingleSampleFBOID; + this->getGLGpu()->bindFramebuffer(fboTarget, fboId); + + // Make sure the stencil attachment is valid. Even though a color buffer op doesn't use stencil, + // our FBO still needs to be "framebuffer complete". + if (!fStencilAttachmentIsValid[useMultisampleFBO]) { + if (auto stencil = this->getStencilAttachment(useMultisampleFBO)) { + const GrGLAttachment* glStencil = static_cast(stencil); + GL_CALL(FramebufferRenderbuffer(fboTarget, + GR_GL_STENCIL_ATTACHMENT, + GR_GL_RENDERBUFFER, + glStencil->renderbufferID())); + if (GrGLFormatIsPackedDepthStencil(glStencil->format())) { + GL_CALL(FramebufferRenderbuffer(fboTarget, + GR_GL_DEPTH_ATTACHMENT, + GR_GL_RENDERBUFFER, + glStencil->renderbufferID())); + } else { + GL_CALL(FramebufferRenderbuffer(fboTarget, + GR_GL_DEPTH_ATTACHMENT, + GR_GL_RENDERBUFFER, + 0)); + } + } else { + GL_CALL(FramebufferRenderbuffer(fboTarget, + GR_GL_STENCIL_ATTACHMENT, + GR_GL_RENDERBUFFER, + 0)); + GL_CALL(FramebufferRenderbuffer(fboTarget, + GR_GL_DEPTH_ATTACHMENT, + GR_GL_RENDERBUFFER, + 0)); + } +#ifdef SK_DEBUG + if (!this->getGLGpu()->glCaps().skipErrorChecks()) { + GrGLenum status; + GL_CALL_RET(status, CheckFramebufferStatus(fboTarget)); + if (status != GR_GL_FRAMEBUFFER_COMPLETE) { + // This can fail if the context has been asynchronously abandoned (see + // skbug.com/5200). + SkDebugf("WARNING: failed to attach stencil.\n"); + } + } +#endif + } +} + +void GrGLRenderTarget::bindForResolve(GrGLGpu::ResolveDirection resolveDirection) { + // If the multisample FBO is nonzero, it means we always have something to resolve (even if the + // single sample buffer is FBO 0). If it's zero, then there's nothing to resolve. + SkASSERT(fMultisampleFBOID != 0); + + // In the EXT_multisampled_render_to_texture case, we shouldn't be resolving anything. + SkASSERT(!this->isMultisampledRenderToTexture()); + + if (resolveDirection == GrGLGpu::ResolveDirection::kMSAAToSingle) { + this->bindInternal(GR_GL_READ_FRAMEBUFFER, true); + this->bindInternal(GR_GL_DRAW_FRAMEBUFFER, false); + } else { + SkASSERT(resolveDirection == GrGLGpu::ResolveDirection::kSingleToMSAA); + SkASSERT(this->getGLGpu()->glCaps().canResolveSingleToMSAA()); + this->bindInternal(GR_GL_READ_FRAMEBUFFER, false); + this->bindInternal(GR_GL_DRAW_FRAMEBUFFER, true); + } +} + void GrGLRenderTarget::onRelease() { if (GrBackendObjectOwnership::kBorrowed != fRTFBOOwnership) { GrGLGpu* gpu = this->getGLGpu(); diff --git a/src/gpu/gl/GrGLRenderTarget.h b/src/gpu/gl/GrGLRenderTarget.h index c27af43b0f..57e99fcc2d 100644 --- a/src/gpu/gl/GrGLRenderTarget.h +++ b/src/gpu/gl/GrGLRenderTarget.h @@ -12,6 +12,7 @@ #include "include/core/SkScalar.h" #include "include/gpu/GrBackendSurface.h" #include "src/gpu/GrRenderTarget.h" +#include "src/gpu/gl/GrGLDefines.h" class GrGLCaps; class GrGLGpu; @@ -41,8 +42,13 @@ public: const IDs&, int stencilBits); - GrGLuint singleSampleFBOID() const { return fSingleSampleFBOID; } - GrGLuint multisampleFBOID() const { return fMultisampleFBOID; } + bool isFBO0(bool multisample) const { + return (multisample ? fMultisampleFBOID : fSingleSampleFBOID) == 0; + } + + bool isMultisampledRenderToTexture() const { + return fMultisampleFBOID != 0 && fMultisampleFBOID == fSingleSampleFBOID; + } GrBackendRenderTarget getBackendRenderTarget() const override; @@ -59,6 +65,28 @@ public: bool hasDynamicMSAAAttachment() const { return SkToBool(fDynamicMSAAAttachment); } bool ensureDynamicMSAAAttachment(); + // Binds the render target to GL_FRAMEBUFFER for rendering. + void bind(bool useMultisampleFBO) { + this->bindInternal(GR_GL_FRAMEBUFFER, useMultisampleFBO); + } + + // Binds the render target for copying, reading, or clearing pixel values. If we are an MSAA + // render target with a separate resolve texture, we bind the multisampled FBO. Otherwise we + // bind the single sample FBO. + void bindForPixelOps(GrGLenum fboTarget) { + this->bindInternal(fboTarget, + this->numSamples() > 1 && !this->isMultisampledRenderToTexture()); + } + + enum class ResolveDirection : bool { + kSingleToMSAA, // glCaps.canResolveSingleToMSAA() must be true. + kMSAAToSingle + }; + + // Binds the multisampled and single sample FBOs, one to GL_DRAW_FRAMEBUFFER and the other to + // GL_READ_FRAMEBUFFER, depending on ResolveDirection. + void bindForResolve(ResolveDirection); + protected: // Constructor for subclasses. GrGLRenderTarget(GrGLGpu*, @@ -69,6 +97,9 @@ protected: void init(GrGLFormat, const IDs&); + // Binds the render target to the given target and ensures its stencil attachment is valid. + void bindInternal(GrGLenum fboTarget, bool useMultisampleFBO); + void onAbandon() override; void onRelease() override; @@ -93,6 +124,7 @@ private: GrGLuint fSingleSampleFBOID; GrGLuint fMSColorRenderbufferID; GrGLFormat fRTFormat; + bool fStencilAttachmentIsValid[2] = {false, false}; GrBackendObjectOwnership fRTFBOOwnership;