From 2573dffde7e9ff13607ef91c66926c130777c8b0 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 22 May 2019 12:04:33 +0100 Subject: [PATCH] Remove MAP_UNSYNCHRONIZED_BIT from GL persistent maps * This is required because we add MAP_READ_BIT and the spec doesn't allow maps that are both unsynchronized and reading. * Also add a test for unsynchronised persistent maps, and update the GL_Buffer_Updates test to not require a reference image but check each quad individually for the expected colour. --- .../driver/gl/wrappers/gl_buffer_funcs.cpp | 3 + .../data/GL_Buffer_Updates/backbuffer.png | Bin 1028 -> 0 bytes util/test/demos/gl/gl_buffer_updates.cpp | 93 +++++++++++++----- util/test/tests/GL/GL_Buffer_Updates.py | 44 ++++++++- 4 files changed, 111 insertions(+), 29 deletions(-) delete mode 100644 util/test/data/GL_Buffer_Updates/backbuffer.png diff --git a/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp index e2bc8def4..664497148 100644 --- a/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp +++ b/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp @@ -2033,6 +2033,9 @@ void *WrappedOpenGL::glMapNamedBufferRangeEXT(GLuint buffer, GLintptr offset, GL // it. access &= ~(GL_MAP_INVALIDATE_BUFFER_BIT | GL_MAP_INVALIDATE_RANGE_BIT); + // also can't be unsynchronized for some reason + access &= ~GL_MAP_UNSYNCHRONIZED_BIT; + directMap = true; persistent = true; } diff --git a/util/test/data/GL_Buffer_Updates/backbuffer.png b/util/test/data/GL_Buffer_Updates/backbuffer.png deleted file mode 100644 index eb21d64ff1d044edf346f8823ca06f1eb511fbc0..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1028 zcmeAS@N?(olHy`uVBq!ia0y~yV4MKNIvi|3k+<8Q9%5i%KJDq^7*cWT?e&AahZO`| z9Xsv_iSA$*-)Z`QYnz7BbOG%{inA;W{>RMQ=5{`i;eoS$JVQeRgMtDB2L}Ta6A;mq z2et%m1C4nwPeJS>m1m!_dtP$BcYlZRY5TVf1&yFEBGn97XyVid4+^kK0=y5drd53R z85+1*8Z@`Pw*0p3ONYhg8;R|*Wj74V+kv9dEG!I+j1CSA0s;&`CXIOzOJMeac@64U zURc&JGw2C0EW5Gp`HkC`lpmc*0y!}8_#IiGXr3V0Whk29PR3#&WpeP40NV%Q%?nuk z`~xq;4hM!ANw5g+205@@wzwQ9y2BL~d`OyLA&JdEisaz_1DgWjRXFv|J8aJIVWqhR XQ-Y7qg0^O0?q=|G^>bP0l+XkKp$DQ3 diff --git a/util/test/demos/gl/gl_buffer_updates.cpp b/util/test/demos/gl/gl_buffer_updates.cpp index f3a5e1e09..e245d0264 100644 --- a/util/test/demos/gl/gl_buffer_updates.cpp +++ b/util/test/demos/gl/gl_buffer_updates.cpp @@ -74,32 +74,34 @@ void main() Vec4f green(0.0f, 1.0f, 0.0f, 1.0f); // clang-format explodes on these for some reason -#define TEST_CASES() \ - BUFFER_TEST(BufferDataImmutable) \ - BUFFER_TEST(BufferStorageImmutable) \ - BUFFER_TEST(BufferDataOrphanedOnce) \ - BUFFER_TEST(BufferDataOrphanedMany) \ - BUFFER_TEST(BufferDataOrphanedPerFrame) \ - BUFFER_TEST(BufferDataUpdatedOnce) \ - BUFFER_TEST(BufferDataUpdatedMany) \ - BUFFER_TEST(BufferDataUpdatedPerFrame) \ - BUFFER_TEST(BufferStorageUpdatedOnce) \ - BUFFER_TEST(BufferStorageUpdatedMany) \ - BUFFER_TEST(BufferStorageUpdatedPerFrame) \ - BUFFER_TEST(SingleMapBufferReadback) \ - BUFFER_TEST(SingleMapBufferRangeReadback) \ - BUFFER_TEST(CoherentMapBufferRangeReadback) \ - BUFFER_TEST(CleanBufferMapWriteInvalidate) \ - BUFFER_TEST(CleanBufferMapWriteNonInvalidate) \ - BUFFER_TEST(DirtyBufferMapWriteInvalidate) \ - BUFFER_TEST(DirtyBufferMapWriteNonInvalidate) \ - BUFFER_TEST(CleanBufferMapFlushExplicit) \ - BUFFER_TEST(DirtyBufferMapFlushExplicit) \ - BUFFER_TEST(CoherentMapWrite) \ - BUFFER_TEST(CoherentMapWriteInvalidateRange) \ - BUFFER_TEST(CoherentMapWriteInvalidateBuffer) \ - BUFFER_TEST(NonCoherentMapFlush) \ - BUFFER_TEST(OffsetMapWrite) \ +#define TEST_CASES() \ + BUFFER_TEST(BufferDataImmutable) \ + BUFFER_TEST(BufferStorageImmutable) \ + BUFFER_TEST(BufferDataOrphanedOnce) \ + BUFFER_TEST(BufferDataOrphanedMany) \ + BUFFER_TEST(BufferDataOrphanedPerFrame) \ + BUFFER_TEST(BufferDataUpdatedOnce) \ + BUFFER_TEST(BufferDataUpdatedMany) \ + BUFFER_TEST(BufferDataUpdatedPerFrame) \ + BUFFER_TEST(BufferStorageUpdatedOnce) \ + BUFFER_TEST(BufferStorageUpdatedMany) \ + BUFFER_TEST(BufferStorageUpdatedPerFrame) \ + BUFFER_TEST(SingleMapBufferReadback) \ + BUFFER_TEST(SingleMapBufferRangeReadback) \ + BUFFER_TEST(CoherentMapBufferRangeReadback) \ + BUFFER_TEST(CleanBufferMapWriteInvalidate) \ + BUFFER_TEST(CleanBufferMapWriteNonInvalidate) \ + BUFFER_TEST(DirtyBufferMapWriteInvalidate) \ + BUFFER_TEST(DirtyBufferMapWriteNonInvalidate) \ + BUFFER_TEST(CleanBufferMapFlushExplicit) \ + BUFFER_TEST(DirtyBufferMapFlushExplicit) \ + BUFFER_TEST(CoherentMapWrite) \ + BUFFER_TEST(CoherentMapWriteInvalidateRange) \ + BUFFER_TEST(CoherentMapWriteInvalidateBuffer) \ + BUFFER_TEST(CoherentMapWriteUnsynchronised) \ + BUFFER_TEST(NonCoherentMapFlush) \ + BUFFER_TEST(NonCoherentMapFlushUnsynchronised) \ + BUFFER_TEST(OffsetMapWrite) \ BUFFER_TEST(OffsetMapFlush) #undef BUFFER_TEST @@ -226,6 +228,14 @@ void main() GL_UNIFORM_BUFFER, 0, sizeof(Vec4f), GL_MAP_COHERENT_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT); + glBindBuffer(GL_UNIFORM_BUFFER, buffers[CoherentMapWriteUnsynchronised]); + glBufferStorage(GL_UNIFORM_BUFFER, sizeof(Vec4f), &red, + GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT); + + ptrs[CoherentMapWriteUnsynchronised] = (Vec4f *)glMapBufferRange( + GL_UNIFORM_BUFFER, 0, sizeof(Vec4f), + GL_MAP_COHERENT_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_WRITE_BIT | GL_MAP_UNSYNCHRONIZED_BIT); + glBindBuffer(GL_UNIFORM_BUFFER, buffers[NonCoherentMapFlush]); glBufferStorage(GL_UNIFORM_BUFFER, sizeof(Vec4f), &red, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT); @@ -233,6 +243,14 @@ void main() GL_UNIFORM_BUFFER, 0, sizeof(Vec4f), GL_MAP_PERSISTENT_BIT | GL_MAP_WRITE_BIT | GL_MAP_FLUSH_EXPLICIT_BIT); + glBindBuffer(GL_UNIFORM_BUFFER, buffers[NonCoherentMapFlushUnsynchronised]); + glBufferStorage(GL_UNIFORM_BUFFER, sizeof(Vec4f), &red, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT); + + ptrs[NonCoherentMapFlushUnsynchronised] = + (Vec4f *)glMapBufferRange(GL_UNIFORM_BUFFER, 0, sizeof(Vec4f), + GL_MAP_PERSISTENT_BIT | GL_MAP_WRITE_BIT | + GL_MAP_FLUSH_EXPLICIT_BIT | GL_MAP_UNSYNCHRONIZED_BIT); + glBindBuffer(GL_UNIFORM_BUFFER, buffers[OffsetMapWrite]); glBufferStorage(GL_UNIFORM_BUFFER, sizeof(Vec4f), &cyan, GL_MAP_WRITE_BIT | GL_DYNAMIC_STORAGE_BIT); @@ -362,12 +380,21 @@ void main() memcpy(ptrs[CoherentMapWriteInvalidateRange], &red, sizeof(Vec4f)); if(ptrs[CoherentMapWriteInvalidateBuffer]) memcpy(ptrs[CoherentMapWriteInvalidateBuffer], &red, sizeof(Vec4f)); + if(ptrs[CoherentMapWriteUnsynchronised]) + memcpy(ptrs[CoherentMapWriteUnsynchronised], &red, sizeof(Vec4f)); glBindBuffer(GL_UNIFORM_BUFFER, buffers[NonCoherentMapFlush]); if(ptrs[NonCoherentMapFlush]) memcpy(ptrs[NonCoherentMapFlush], &red, sizeof(Vec4f)); glFlushMappedBufferRange(GL_UNIFORM_BUFFER, 0, sizeof(float) * 4); + glBindBuffer(GL_UNIFORM_BUFFER, buffers[NonCoherentMapFlushUnsynchronised]); + if(ptrs[NonCoherentMapFlushUnsynchronised]) + memcpy(ptrs[NonCoherentMapFlushUnsynchronised], &red, sizeof(Vec4f)); + glFlushMappedBufferRange(GL_UNIFORM_BUFFER, 0, sizeof(float) * 4); + + glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT); + glBindBuffer(GL_UNIFORM_BUFFER, buffers[OffsetMapWrite]); glBufferSubData(GL_UNIFORM_BUFFER, 0, sizeof(Vec4f), &cyan); @@ -401,10 +428,13 @@ void main() GL_DEBUG_SEVERITY_HIGH, -1, TestNames[buf]); if(buf == CoherentMapWrite || buf == CoherentMapWriteInvalidateRange || - buf == CoherentMapWriteInvalidateBuffer) + buf == CoherentMapWriteInvalidateBuffer || buf == CoherentMapWriteUnsynchronised) { if(ptrs[buf]) memcpy(ptrs[buf], &green, sizeof(Vec4f)); + + if(buf == CoherentMapWriteUnsynchronised) + glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT); } if(buf == NonCoherentMapFlush) @@ -415,6 +445,15 @@ void main() glFlushMappedBufferRange(GL_UNIFORM_BUFFER, 0, sizeof(float) * 4); } + if(buf == NonCoherentMapFlushUnsynchronised) + { + glBindBuffer(GL_UNIFORM_BUFFER, buffers[NonCoherentMapFlushUnsynchronised]); + if(ptrs[NonCoherentMapFlushUnsynchronised]) + memcpy(ptrs[NonCoherentMapFlushUnsynchronised], &green, sizeof(Vec4f)); + glFlushMappedBufferRange(GL_UNIFORM_BUFFER, 0, sizeof(float) * 4); + glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT); + } + if(buf == SingleMapBufferReadback) { glBindBuffer(GL_COPY_READ_BUFFER, buffers[buf]); diff --git a/util/test/tests/GL/GL_Buffer_Updates.py b/util/test/tests/GL/GL_Buffer_Updates.py index 3a3f6fe2f..e0027ce1a 100644 --- a/util/test/tests/GL/GL_Buffer_Updates.py +++ b/util/test/tests/GL/GL_Buffer_Updates.py @@ -7,7 +7,46 @@ class GL_Buffer_Updates(rdtest.TestCase): return rdtest.run_and_capture("demos_x64", "GL_Buffer_Updates", 5) def check_capture(self): - self.check_final_backbuffer() + # Make an output so we can pick pixels + out: rd.ReplayOutput = self.controller.CreateOutput(rd.CreateHeadlessWindowingData(100, 100), rd.ReplayOutputType.Texture) + + self.check(out is not None) + + tex = rd.TextureDisplay() + + # At each draw, the centre pixel of the viewport should be green + draw = self.get_first_draw() + while draw is not None: + self.controller.SetFrameEvent(draw.eventId, False) + + if draw.flags & rd.DrawFlags.Drawcall: + tex.resourceId = self.controller.GetPipelineState().GetOutputTargets()[0].resourceId + out.SetTextureDisplay(tex) + + view: rd.Viewport = self.controller.GetPipelineState().GetViewport(0) + + x,y = int(view.x + view.width / 2), int(view.y + view.height / 2) + + # convert to top-left co-ordinates for use with PickPixel + y = self.get_texture(tex.resourceId).height - y + + picked: rd.PixelValue = out.PickPixel(tex.resourceId, False, x, y, 0, 0, 0) + + if not rdtest.value_compare(picked.floatValue, [0.0, 1.0, 0.0, 1.0]): + raise rdtest.TestFailureException("Picked value {} at {} doesn't match expected green".format(picked.floatValue, (x,y))) + + draw = draw.next + + rdtest.log.success("Draws are all green") + + # Now save the backbuffer to disk + ref_path = rdtest.get_tmp_path('backbuffer.png') + + save_data = rd.TextureSave() + save_data.resourceId = tex.resourceId + save_data.destType = rd.FileType.PNG + + self.controller.SaveTexture(save_data, ref_path) # Open the capture and grab the thumbnail, check that it is all green too (dirty way of verifying we didn't # break in-app updates but somehow end up with the right data) @@ -29,10 +68,11 @@ class GL_Buffer_Updates(rdtest.TestCase): f.write(thumb.data) # The original thumbnail should also be identical, since we have the uncompressed extended thumbnail. - ref_path = self.get_ref_path('backbuffer.png') if not rdtest.png_compare(tmp_path, ref_path): raise rdtest.TestFailureException("Reference backbuffer and thumbnail image differ", tmp_path, ref_path) rdtest.log.success("Thumbnail is identical to reference") + out.Shutdown() +