From 7ac31e5ca7762dad8450d6e58878dec63f3fee95 Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 1 Apr 2019 15:45:24 +0100 Subject: [PATCH] Workaround ARM driver bug with core promoted function. Closes #1322 * glRenderbufferStorageMultisample and glRenderbufferStorageMultisampleEXT should be interchangeable, with the only difference being the EXT version specifies that tile resolves may happen and MSAA data won't be preserved. * When aliasing the functions RenderDoc will call the core version if available even if the application called into the EXT, assuming them to be interchangeable (and in this case the core function is a superset of functionality). Unfortunately on ARM's driver attachments of different types are not handled properly and FBOs can fail to create. * The only available workaround for this is to copy-paste the function implementation and keep them separate, and assume that the application does the right thing and ends up with compatible attachments. The app could easily break in the same way, but at least then RenderDoc is not forcing a bug to be hit. --- renderdoc/driver/gl/gl_dispatch_table.h | 5 +- renderdoc/driver/gl/gl_dispatch_table_defs.h | 4 +- renderdoc/driver/gl/gl_driver.cpp | 5 +- renderdoc/driver/gl/gl_driver.h | 8 + .../gl/wrappers/gl_framebuffer_funcs.cpp | 164 ++++++++++++++++++ 5 files changed, 182 insertions(+), 4 deletions(-) diff --git a/renderdoc/driver/gl/gl_dispatch_table.h b/renderdoc/driver/gl/gl_dispatch_table.h index 0155dff02..1a159aad9 100644 --- a/renderdoc/driver/gl/gl_dispatch_table.h +++ b/renderdoc/driver/gl/gl_dispatch_table.h @@ -312,7 +312,7 @@ struct GLDispatchTable PFNGLDELETEFRAMEBUFFERSPROC glDeleteFramebuffers; // aliases glDeleteFramebuffersEXT PFNGLGENRENDERBUFFERSPROC glGenRenderbuffers; // aliases glGenRenderbuffersEXT PFNGLRENDERBUFFERSTORAGEPROC glRenderbufferStorage; // aliases glRenderbufferStorageEXT - PFNGLRENDERBUFFERSTORAGEMULTISAMPLEPROC glRenderbufferStorageMultisample; // aliases glRenderbufferStorageMultisampleEXT + PFNGLRENDERBUFFERSTORAGEMULTISAMPLEPROC glRenderbufferStorageMultisample; PFNGLDELETERENDERBUFFERSPROC glDeleteRenderbuffers; // aliases glDeleteRenderbuffersEXT PFNGLBINDRENDERBUFFERPROC glBindRenderbuffer; // aliases glBindRenderbufferEXT PFNGLFENCESYNCPROC glFenceSync; @@ -616,6 +616,9 @@ struct GLDispatchTable // GLES: EXT_multisampled_render_to_texture PFNGLFRAMEBUFFERTEXTURE2DMULTISAMPLEEXTPROC glFramebufferTexture2DMultisampleEXT; + // this function should be an alias of glRenderBufferStorage2DMultisample, except there are driver + // issues that prevent the functions from being treated interchangeably. + PFNGLRENDERBUFFERSTORAGEMULTISAMPLEEXTPROC glRenderbufferStorageMultisampleEXT; // GLES: EXT_discard_framebuffer PFNGLDISCARDFRAMEBUFFEREXTPROC glDiscardFramebufferEXT; diff --git a/renderdoc/driver/gl/gl_dispatch_table_defs.h b/renderdoc/driver/gl/gl_dispatch_table_defs.h index 4475101e6..3afe0a19f 100644 --- a/renderdoc/driver/gl/gl_dispatch_table_defs.h +++ b/renderdoc/driver/gl/gl_dispatch_table_defs.h @@ -463,7 +463,6 @@ FUNC(glRenderbufferStorage, glRenderbufferStorage); \ FUNC(glRenderbufferStorage, glRenderbufferStorageEXT); \ FUNC(glRenderbufferStorageMultisample, glRenderbufferStorageMultisample); \ - FUNC(glRenderbufferStorageMultisample, glRenderbufferStorageMultisampleEXT); \ FUNC(glDeleteRenderbuffers, glDeleteRenderbuffers); \ FUNC(glDeleteRenderbuffers, glDeleteRenderbuffersEXT); \ FUNC(glBindRenderbuffer, glBindRenderbuffer); \ @@ -981,6 +980,7 @@ FUNC(glPrimitiveBoundingBox, glPrimitiveBoundingBoxOES); \ FUNC(glBlendBarrier, glBlendBarrier); \ FUNC(glFramebufferTexture2DMultisampleEXT, glFramebufferTexture2DMultisampleEXT); \ + FUNC(glRenderbufferStorageMultisampleEXT, glRenderbufferStorageMultisampleEXT); \ FUNC(glDiscardFramebufferEXT, glDiscardFramebufferEXT); \ FUNC(glDepthRangeArrayfvOES, glDepthRangeArrayfvOES); \ FUNC(glDepthRangeArrayfvOES, glDepthRangeArrayfvNV); \ @@ -1723,7 +1723,6 @@ FuncWrapper4(void, glRenderbufferStorage, GLenum, target, GLenum, internalformat, GLsizei, width, GLsizei, height); \ AliasWrapper4(void, glRenderbufferStorageEXT, glRenderbufferStorage, GLenum, target, GLenum, internalformat, GLsizei, width, GLsizei, height); \ FuncWrapper5(void, glRenderbufferStorageMultisample, GLenum, target, GLsizei, samples, GLenum, internalformat, GLsizei, width, GLsizei, height); \ - AliasWrapper5(void, glRenderbufferStorageMultisampleEXT, glRenderbufferStorageMultisample, GLenum, target, GLsizei, samples, GLenum, internalformat, GLsizei, width, GLsizei, height); \ FuncWrapper2(void, glDeleteRenderbuffers, GLsizei, n, const GLuint *, renderbuffers); \ AliasWrapper2(void, glDeleteRenderbuffersEXT, glDeleteRenderbuffers, GLsizei, n, const GLuint *, renderbuffers); \ FuncWrapper2(void, glBindRenderbuffer, GLenum, target, GLuint, renderbuffer); \ @@ -2241,6 +2240,7 @@ AliasWrapper8(void, glPrimitiveBoundingBoxOES, glPrimitiveBoundingBox, GLfloat, minX, GLfloat, minY, GLfloat, minZ, GLfloat, minW, GLfloat, maxX, GLfloat, maxY, GLfloat, maxZ, GLfloat, maxW); \ FuncWrapper0(void, glBlendBarrier); \ FuncWrapper6(void, glFramebufferTexture2DMultisampleEXT, GLenum, target, GLenum, attachment, GLenum, textarget, GLuint, texture, GLint, level, GLsizei, samples); \ + FuncWrapper5(void, glRenderbufferStorageMultisampleEXT, GLenum, target, GLsizei, samples, GLenum, internalformat, GLsizei, width, GLsizei, height); \ FuncWrapper3(void, glDiscardFramebufferEXT, GLenum, target, GLsizei, numAttachments, const GLenum *, attachments); \ FuncWrapper3(void, glDepthRangeArrayfvOES, GLuint, first, GLsizei, count, const GLfloat *, v); \ AliasWrapper3(void, glDepthRangeArrayfvNV, glDepthRangeArrayfvOES, GLuint, first, GLsizei, count, const GLfloat *, v); \ diff --git a/renderdoc/driver/gl/gl_driver.cpp b/renderdoc/driver/gl/gl_driver.cpp index 81b40a54b..b7b97e9f2 100644 --- a/renderdoc/driver/gl/gl_driver.cpp +++ b/renderdoc/driver/gl/gl_driver.cpp @@ -3394,11 +3394,14 @@ bool WrappedOpenGL::ProcessChunk(ReadSerialiser &ser, GLChunk chunk) case GLChunk::glNamedRenderbufferStorageEXT: return Serialise_glNamedRenderbufferStorageEXT(ser, 0, eGL_NONE, 0, 0); case GLChunk::glRenderbufferStorageMultisample: - case GLChunk::glRenderbufferStorageMultisampleEXT: case GLChunk::glNamedRenderbufferStorageMultisample: case GLChunk::glNamedRenderbufferStorageMultisampleEXT: return Serialise_glNamedRenderbufferStorageMultisampleEXT(ser, 0, 0, eGL_NONE, 0, 0); + // needs to be separate from glRenderbufferStorageMultisample due to driver issues + case GLChunk::glRenderbufferStorageMultisampleEXT: + return Serialise_glRenderbufferStorageMultisampleEXT(ser, 0, 0, eGL_NONE, 0, 0); + case GLChunk::wglDXRegisterObjectNV: return Serialise_wglDXRegisterObjectNV(ser, GLResource(MakeNullResource), eGL_NONE, 0); case GLChunk::wglDXLockObjectsNV: diff --git a/renderdoc/driver/gl/gl_driver.h b/renderdoc/driver/gl/gl_driver.h index 82158d438..67833c1d6 100644 --- a/renderdoc/driver/gl/gl_driver.h +++ b/renderdoc/driver/gl/gl_driver.h @@ -2289,6 +2289,14 @@ public: void glFramebufferTexture2DMultisampleEXT(GLenum target, GLenum attachment, GLenum textarget, GLuint texture, GLint level, GLsizei samples); + // needs to be separate from glRenderbufferStorageMultisample due to driver issues + template + bool Serialise_glRenderbufferStorageMultisampleEXT(SerialiserType &ser, GLuint renderbufferHandle, + GLsizei samples, GLenum internalformat, + GLsizei width, GLsizei height); + void glRenderbufferStorageMultisampleEXT(GLenum target, GLsizei samples, GLenum internalformat, + GLsizei width, GLsizei height); + IMPLEMENT_FUNCTION_SERIALISED(void, glSpecializeShader, GLuint shader, const GLchar *pEntryPoint, GLuint numSpecializationConstants, const GLuint *pConstantIndex, const GLuint *pConstantValue); diff --git a/renderdoc/driver/gl/wrappers/gl_framebuffer_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_framebuffer_funcs.cpp index e9d40a0c4..f279072bf 100644 --- a/renderdoc/driver/gl/wrappers/gl_framebuffer_funcs.cpp +++ b/renderdoc/driver/gl/wrappers/gl_framebuffer_funcs.cpp @@ -2574,6 +2574,167 @@ void WrappedOpenGL::glRenderbufferStorageMultisample(GLenum target, GLsizei samp } } +/////////////////////////////////////////////////////////////////////////////////////////////////// +// +// We need a separate implementation of glRenderbufferStorageMultisampleEXT because on some drivers +// there are issues with aliasing it with the core version of the function. + +template +bool WrappedOpenGL::Serialise_glRenderbufferStorageMultisampleEXT(SerialiserType &ser, + GLuint renderbufferHandle, + GLsizei samples, + GLenum internalformat, + GLsizei width, GLsizei height) +{ + SERIALISE_ELEMENT_LOCAL(renderbuffer, RenderbufferRes(GetCtx(), renderbufferHandle)); + SERIALISE_ELEMENT(samples); + SERIALISE_ELEMENT(internalformat); + SERIALISE_ELEMENT(width); + SERIALISE_ELEMENT(height); + + SERIALISE_CHECK_READ_ERRORS(); + + if(IsReplayingAndReading()) + { + ResourceId liveId = GetResourceManager()->GetID(renderbuffer); + TextureData &texDetails = m_Textures[liveId]; + + GLenum fmt = GetBaseFormat(internalformat); + + texDetails.width = width; + texDetails.height = height; + texDetails.depth = 1; + texDetails.samples = samples; + texDetails.curType = eGL_RENDERBUFFER; + texDetails.internalFormat = internalformat; + texDetails.mipsValid = 1; + + GLuint oldRB = 0; + GL.glGetIntegerv(eGL_RENDERBUFFER_BINDING, (GLint *)&oldRB); + + GL.glBindRenderbuffer(eGL_RENDERBUFFER, renderbuffer.name); + + GL.glRenderbufferStorageMultisampleEXT(eGL_RENDERBUFFER, samples, internalformat, width, height); + + if(internalformat == eGL_DEPTH_COMPONENT || internalformat == eGL_DEPTH_STENCIL || + internalformat == eGL_STENCIL || internalformat == eGL_STENCIL_INDEX) + { + // fetch the exact sized depth-stencil formats corresponding to whatever unsized format was + // specified. + GLint depth = 0; + GLint stencil = 0; + GL.glGetNamedRenderbufferParameterivEXT(renderbuffer.name, eGL_RENDERBUFFER_DEPTH_SIZE, &depth); + GL.glGetNamedRenderbufferParameterivEXT(renderbuffer.name, eGL_RENDERBUFFER_STENCIL_SIZE, + &stencil); + + if(depth == 16 && stencil == 0) + internalformat = eGL_DEPTH_COMPONENT16; + else if(depth == 24 && stencil == 0) + internalformat = eGL_DEPTH_COMPONENT24; + else if(depth == 24 && stencil == 8) + internalformat = eGL_DEPTH24_STENCIL8; + else if(depth == 32 && stencil == 0) + internalformat = eGL_DEPTH_COMPONENT32F; + else if(depth == 32 && stencil == 8) + internalformat = eGL_DEPTH32F_STENCIL8; + else if(depth == 0 && stencil == 8) + internalformat = eGL_STENCIL_INDEX8; + } + else if(internalformat == eGL_RGBA || internalformat == eGL_RGBA_INTEGER || + internalformat == eGL_RGB || internalformat == eGL_RGB_INTEGER || + internalformat == eGL_RG || internalformat == eGL_RG_INTEGER || + internalformat == eGL_RED || internalformat == eGL_RED_INTEGER) + { + // if the color format is unsized, find the corresponding sized format + + GLint red = 0, green = 0, blue = 0, alpha = 0; + GL.glGetNamedRenderbufferParameterivEXT(renderbuffer.name, eGL_RENDERBUFFER_RED_SIZE, &red); + GL.glGetNamedRenderbufferParameterivEXT(renderbuffer.name, eGL_RENDERBUFFER_GREEN_SIZE, &green); + GL.glGetNamedRenderbufferParameterivEXT(renderbuffer.name, eGL_RENDERBUFFER_BLUE_SIZE, &blue); + GL.glGetNamedRenderbufferParameterivEXT(renderbuffer.name, eGL_RENDERBUFFER_ALPHA_SIZE, &alpha); + + // we only handle a straight regular format here + RDCASSERT(red > 0); + RDCASSERT(green == 0 || green == red); + RDCASSERT(blue == 0 || green == red); + RDCASSERT(alpha == 0 || green == red); + + // to start with, create resource format based on the unsized internalformat + ResourceFormat resfmt = MakeResourceFormat(eGL_TEXTURE_2D, internalformat); + + // then set the byte size + resfmt.compByteWidth = uint8_t(red / 8); + + internalformat = MakeGLFormat(resfmt); + } + + // create read-from texture for displaying this render buffer + GL.glGenTextures(1, &texDetails.renderbufferReadTex); + GL.glBindTexture(eGL_TEXTURE_2D_MULTISAMPLE, texDetails.renderbufferReadTex); + GL.glTextureStorage2DMultisampleEXT(texDetails.renderbufferReadTex, eGL_TEXTURE_2D_MULTISAMPLE, + samples, internalformat, width, height, true); + + GL.glGenFramebuffers(2, texDetails.renderbufferFBOs); + GL.glBindFramebuffer(eGL_FRAMEBUFFER, texDetails.renderbufferFBOs[0]); + GL.glBindFramebuffer(eGL_FRAMEBUFFER, texDetails.renderbufferFBOs[1]); + + GLenum attach = eGL_COLOR_ATTACHMENT0; + if(fmt == eGL_DEPTH_COMPONENT) + attach = eGL_DEPTH_ATTACHMENT; + if(fmt == eGL_STENCIL) + attach = eGL_STENCIL_ATTACHMENT; + if(fmt == eGL_DEPTH_STENCIL) + attach = eGL_DEPTH_STENCIL_ATTACHMENT; + GL.glNamedFramebufferRenderbufferEXT(texDetails.renderbufferFBOs[0], attach, eGL_RENDERBUFFER, + renderbuffer.name); + GL.glNamedFramebufferTexture2DEXT(texDetails.renderbufferFBOs[1], attach, + eGL_TEXTURE_2D_MULTISAMPLE, texDetails.renderbufferReadTex, 0); + + AddResourceInitChunk(renderbuffer); + + GL.glBindRenderbuffer(eGL_RENDERBUFFER, oldRB); + } + + return true; +} + +void WrappedOpenGL::glRenderbufferStorageMultisampleEXT(GLenum target, GLsizei samples, + GLenum internalformat, GLsizei width, + GLsizei height) +{ + SERIALISE_TIME_CALL( + GL.glRenderbufferStorageMultisampleEXT(target, samples, internalformat, width, height)); + + ResourceId rb = GetCtxData().m_Renderbuffer; + + if(IsCaptureMode(m_State)) + { + GLResourceRecord *record = GetResourceManager()->GetResourceRecord(rb); + RDCASSERTMSG("Couldn't identify implicit renderbuffer. Not bound?", record); + + if(record) + { + USE_SCRATCH_SERIALISER(); + SCOPED_SERIALISE_CHUNK(gl_CurChunk); + Serialise_glRenderbufferStorageMultisampleEXT(ser, record->Resource.name, samples, + internalformat, width, height); + + record->AddChunk(scope.Get()); + } + } + + { + m_Textures[rb].width = width; + m_Textures[rb].height = height; + m_Textures[rb].depth = 1; + m_Textures[rb].samples = samples; + m_Textures[rb].curType = eGL_RENDERBUFFER; + m_Textures[rb].dimension = 2; + m_Textures[rb].internalFormat = internalformat; + m_Textures[rb].mipsValid = 1; + } +} + INSTANTIATE_FUNCTION_SERIALISED(void, glGenFramebuffers, GLsizei n, GLuint *framebuffers); INSTANTIATE_FUNCTION_SERIALISED(void, glCreateFramebuffers, GLsizei n, GLuint *framebuffers); INSTANTIATE_FUNCTION_SERIALISED(void, glNamedFramebufferTextureEXT, GLuint framebufferHandle, @@ -2621,3 +2782,6 @@ INSTANTIATE_FUNCTION_SERIALISED(void, glNamedRenderbufferStorageEXT, GLuint rend INSTANTIATE_FUNCTION_SERIALISED(void, glNamedRenderbufferStorageMultisampleEXT, GLuint renderbufferHandle, GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height); +INSTANTIATE_FUNCTION_SERIALISED(void, glRenderbufferStorageMultisampleEXT, + GLuint renderbufferHandle, GLsizei samples, GLenum internalformat, + GLsizei width, GLsizei height);