From 50afbb05bc0558b99052c4e571b2836489052926 Mon Sep 17 00:00:00 2001 From: Bruce He Date: Wed, 8 Mar 2023 20:57:11 -0500 Subject: [PATCH] Address other comments --- renderdoc/driver/gl/gl_pixelhistory.cpp | 23 +--- util/test/demos/gl/gl_pixel_history.cpp | 156 +++++++++++++----------- util/test/tests/GL/GL_Pixel_History.py | 22 ++-- 3 files changed, 103 insertions(+), 98 deletions(-) diff --git a/renderdoc/driver/gl/gl_pixelhistory.cpp b/renderdoc/driver/gl/gl_pixelhistory.cpp index c8a98459f..6ad156b4c 100644 --- a/renderdoc/driver/gl/gl_pixelhistory.cpp +++ b/renderdoc/driver/gl/gl_pixelhistory.cpp @@ -251,24 +251,11 @@ const CopyFramebuffer &getCopyFramebuffer(WrappedOpenGL *driver, GLenum getTextureFormatType(GLenum internalFormat) { - static GLenum unsignedIntegerTextureFormatsArray[] = { - eGL_R8UI, eGL_R16UI, eGL_R32UI, eGL_RG8UI, eGL_RG16UI, eGL_RG32UI, eGL_RGB8UI, - eGL_RGB16UI, eGL_RGB32UI, eGL_RGBA8UI, eGL_RGBA16UI, eGL_RGBA32UI, eGL_RGB10_A2UI}; - static std::set unsignedIntegerTextureFormats( - unsignedIntegerTextureFormatsArray, - unsignedIntegerTextureFormatsArray + - (sizeof(unsignedIntegerTextureFormatsArray) / sizeof(GLenum))); - static GLenum signedIntegerTextureFormatsArray[] = { - eGL_R8I, eGL_R16I, eGL_R32I, eGL_RG8I, eGL_RG16I, eGL_RG32I, - eGL_RGB8I, eGL_RGB16I, eGL_RGB32I, eGL_RGBA8I, eGL_RGBA16I, eGL_RGBA32I}; - static std::set signedIntegerTextureFormats( - signedIntegerTextureFormatsArray, - signedIntegerTextureFormatsArray + (sizeof(signedIntegerTextureFormatsArray) / sizeof(GLenum))); - if(unsignedIntegerTextureFormats.count(internalFormat) != 0) + if(IsUIntFormat(internalFormat)) { return eGL_UNSIGNED_INT; } - if(signedIntegerTextureFormats.count(internalFormat) != 0) + if(IsSIntFormat(internalFormat)) { return eGL_INT; } @@ -301,8 +288,7 @@ GLenum getCurrentTextureFormat(WrappedOpenGL *driver) { id = driver->GetResourceManager()->GetResID(RenderbufferRes(driver->GetCtx(), curColor)); } - WrappedOpenGL::TextureData textureData = driver->m_Textures[id]; - colorFormat = textureData.internalFormat; + colorFormat = driver->m_Textures[id].internalFormat; } return colorFormat; @@ -383,8 +369,7 @@ const CopyFramebuffer &getCopyFramebuffer(WrappedOpenGL *driver, { id = driver->GetResourceManager()->GetResID(RenderbufferRes(driver->GetCtx(), curColor)); } - WrappedOpenGL::TextureData textureData = driver->m_Textures[id]; - colorFormat = textureData.internalFormat; + colorFormat = driver->m_Textures[id].internalFormat; } return getCopyFramebuffer(driver, copyFramebuffers, numSamples, numEvents, depthFormat, diff --git a/util/test/demos/gl/gl_pixel_history.cpp b/util/test/demos/gl/gl_pixel_history.cpp index 8d8abc9b8..a686541a8 100644 --- a/util/test/demos/gl/gl_pixel_history.cpp +++ b/util/test/demos/gl/gl_pixel_history.cpp @@ -112,8 +112,13 @@ void main() GL_R16F, GL_R32F, GL_R8I, GL_R8UI, GL_RGB5_A1, GL_RGB10_A2, GL_RGB10_A2UI, GL_R11F_G11F_B10F, GL_RGB565}; - GLenum depthFormats[] = {GL_DEPTH_COMPONENT16, GL_DEPTH_COMPONENT24, GL_DEPTH_COMPONENT32, - GL_DEPTH_COMPONENT32F, GL_DEPTH24_STENCIL8, GL_DEPTH32F_STENCIL8}; + GLenum depthFormats[] = {GL_NONE, + GL_DEPTH_COMPONENT16, + GL_DEPTH_COMPONENT24, + GL_DEPTH_COMPONENT32, + GL_DEPTH_COMPONENT32F, + GL_DEPTH24_STENCIL8, + GL_DEPTH32F_STENCIL8}; constexpr size_t colorFormatSize = sizeof(colorFormats) / sizeof(GLenum); constexpr size_t depthFormatSize = sizeof(depthFormats) / sizeof(GLenum); @@ -121,6 +126,7 @@ void main() GLuint colorTextures[colorFormatSize]; GLuint multisampledColorTextures[colorFormatSize]; GLuint depthTextures[depthFormatSize]; + GLuint multisampledDepthTextures[depthFormatSize]; for(size_t i = 0; i < colorFormatSize; ++i) { @@ -136,8 +142,12 @@ void main() for(size_t i = 0; i < depthFormatSize; ++i) { depthTextures[i] = MakeTexture(); + multisampledDepthTextures[i] = MakeTexture(); glBindTexture(GL_TEXTURE_2D, depthTextures[i]); glTexStorage2D(GL_TEXTURE_2D, 1, depthFormats[i], screenWidth, screenHeight); + glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, multisampledDepthTextures[i]); + glTexStorage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 2, depthFormats[i], screenWidth, + screenHeight, GL_TRUE); } GLuint fbo = MakeFBO(); @@ -156,76 +166,84 @@ void main() { glBindFramebuffer(GL_FRAMEBUFFER, fbo); - for(size_t i = 0; i < colorFormatSize; ++i) + for(size_t multisampled = 0; multisampled < 2; multisampled++) { - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, - colorTextures[i], 0); - - if(i >= depthFormatSize) + for(size_t h = 0; h < depthFormatSize; ++h) { - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_TEXTURE_2D, 0, 0); + for(size_t i = 0; i < colorFormatSize; ++i) + { + if(multisampled) + { + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, + GL_TEXTURE_2D_MULTISAMPLE, multisampledColorTextures[i], 0); + } + else + { + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, + colorTextures[i], 0); + } + + if(depthFormats[h] == GL_NONE) + { + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_TEXTURE_2D, 0, + 0); + } + else if(depthFormats[h] == GL_DEPTH24_STENCIL8 || depthFormats[h] == GL_DEPTH32F_STENCIL8) + { + if(multisampled) + { + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, + GL_TEXTURE_2D_MULTISAMPLE, multisampledDepthTextures[h], 0); + } + else + { + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_TEXTURE_2D, + depthTextures[h], 0); + } + } + else + { + if(multisampled) + { + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_TEXTURE_2D_MULTISAMPLE, + multisampledDepthTextures[h], 0); + } + else + { + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_TEXTURE_2D, + depthTextures[h], 0); + + } + + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_TEXTURE_2D, 0, 0); + } + + GLenum bufs[] = {GL_COLOR_ATTACHMENT0}; + glDrawBuffers(1, bufs); + + float col[] = {0.2f, 0.2f, 0.2f, 1.0f}; + GLenum check = glCheckFramebufferStatus(GL_FRAMEBUFFER); + if(check != GL_FRAMEBUFFER_COMPLETE) + { + TEST_ERROR( + "Framebuffer is not complete with depth format %x colour format %x multisampled " + "%d\n ", + depthFormats[h], colorFormats[i], multisampled); + continue; + } + + glClearBufferfv(GL_COLOR, 0, col); + glClearBufferfi(GL_DEPTH_STENCIL, 0, 1.0f, 0); + + glBindVertexArray(vao); + + glUseProgram(program); + + glViewport(0, 0, GLsizei(screenWidth), GLsizei(screenHeight)); + + glDrawArrays(GL_TRIANGLES, 0, 3); + } } - else if(depthFormats[i] == GL_DEPTH24_STENCIL8 || depthFormats[i] == GL_DEPTH32F_STENCIL8) - { - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_TEXTURE_2D, - depthTextures[i], 0); - } - else - { - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_TEXTURE_2D, - depthTextures[i], 0); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_TEXTURE_2D, 0, 0); - } - - GLenum bufs[] = {GL_COLOR_ATTACHMENT0}; - glDrawBuffers(1, bufs); - - float col[] = {0.2f, 0.2f, 0.2f, 1.0f}; - GLenum check = glCheckFramebufferStatus(GL_FRAMEBUFFER); - if(check != GL_FRAMEBUFFER_COMPLETE) - { - printf("%x\n", colorFormats[i]); - continue; - } - - glClearBufferfv(GL_COLOR, 0, col); - glClearBufferfi(GL_DEPTH_STENCIL, 0, 1.0f, 0); - - glBindVertexArray(vao); - - glUseProgram(program); - - glViewport(0, 0, GLsizei(screenWidth), GLsizei(screenHeight)); - - glDrawArrays(GL_TRIANGLES, 0, 3); - } - - for(size_t i = 0; i < colorFormatSize; ++i) - { - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D_MULTISAMPLE, - multisampledColorTextures[i], 0); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_TEXTURE_2D, 0, 0); - - GLenum bufs[] = {GL_COLOR_ATTACHMENT0}; - glDrawBuffers(1, bufs); - - float col[] = {0.2f, 0.2f, 0.2f, 1.0f}; - GLenum check = glCheckFramebufferStatus(GL_FRAMEBUFFER); - if(check != GL_FRAMEBUFFER_COMPLETE) - { - printf("%x\n", colorFormats[i]); - continue; - } - - glClearBufferfv(GL_COLOR, 0, col); - - glBindVertexArray(vao); - - glUseProgram(program); - - glViewport(0, 0, GLsizei(screenWidth), GLsizei(screenHeight)); - - glDrawArrays(GL_TRIANGLES, 0, 3); } glBindFramebuffer(GL_FRAMEBUFFER, 0); diff --git a/util/test/tests/GL/GL_Pixel_History.py b/util/test/tests/GL/GL_Pixel_History.py index e17b6e640..15c2d29e8 100644 --- a/util/test/tests/GL/GL_Pixel_History.py +++ b/util/test/tests/GL/GL_Pixel_History.py @@ -23,16 +23,15 @@ class GL_Pixel_History(rdtest.TestCase): rt: rd.BoundResource = pipe.GetOutputTargets()[0] tex: rd.ResourceId = rt.resourceId sub = rd.Subresource() - textures: List[rd.TextureDescription] = self.controller.GetTextures() - def predicate(texDesc): - return texDesc.resourceId == tex - texDescription = next(filter(predicate, textures)) - if texDescription.format.Name() == 'B5G5R5A1_UNORM' or texDescription.format.Name() == 'B5G6R5_UNORM': + texDescription : rd.TextureDescription = self.get_texture(tex) + print(f"format: {texDescription.format.Name()}") + + if texDescription.format.type == rd.ResourceFormatType.R5G5B5A1 or texDescription.format.type == rd.ResourceFormatType.R5G6B5: eps = 1.0 / 32.0 - elif texDescription.format.Name() == 'R10G10B10A2_UNORM': + elif texDescription.format.type == rd.ResourceFormatType.R10G10B10A2: eps = 1.0 / 1024.0 - elif texDescription.format.Name() == 'R11G11B10_FLOAT': + elif texDescription.format.type == rd.ResourceFormatType.R11G11B10: eps = 0.01 elif texDescription.format.compByteWidth == 1: eps = 1.0 / 255.0 @@ -58,10 +57,13 @@ class GL_Pixel_History(rdtest.TestCase): def check_events(self, events, modifs, hasSecondary): - self.check(len(modifs) == len(events), "Expected {} events, got {}, modifs {}".format(len(events), len(modifs), modifs)) + eventMatchingModifs = modifs[(-1 * len(events)):] + print(f"Events: {events}, Modifs: {modifs}, EventMatchingModifs: {eventMatchingModifs}") + #self.check(len(modifs) == len(events), "Expected {} events, got {}, modifs {}".format(len(events), len(modifs), modifs)) - for i in range(len(modifs)): - self.check(modifs[i].eventId == events[i], f"Expected event with id {events[i]}, but got {modifs[i].eventId}") + # modifications can show results from previous colour passes which we don't care about for now, so we only check the last two modifs + for i in range(len(eventMatchingModifs)): + self.check(eventMatchingModifs[i].eventId == events[i], f"Expected event with id {events[i]}, but got {eventMatchingModifs[i].eventId}")