diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index ba4ce5d6..c9b30f2f 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -1326,11 +1327,47 @@ namespace MobileGL::MG_Backend::DirectGLES { backendTarget == TextureTarget::Texture2DMultisampleArray; } + // Which image units currently hold a WRITABLE buffer texture, and how many. Kept here + // rather than recomputed per draw because the frontend tracks 192 image units and + // almost every program uses none of them: the draw path pays one integer test. + // + // Maintained by SyncImageTextureBinding, which is the single funnel for an image-unit + // change on this backend - glBindImageTextures is a frontend loop over + // glBindImageTexture, and the whole-sweep SyncImageTextureBindings goes through it too. + // Anything that clears a binding WITHOUT coming through here (a deleted texture, a + // recreated context) can only leave a bit set for a unit that no longer has one; the + // sweep re-reads the binding, finds nothing to mark, and CLEARS the bit on its way past. + // So the error is self-healing, and in the direction that costs one wasted look rather + // than one missed write. + static Array + g_writableImageBufferUnits{}; + static Uint g_writableImageBufferUnitCount = 0; + + static Bool IsWritableImageBufferTexture(const MG_State::GLState::ImageTextureBinding& binding) { + return binding.Texture != nullptr && binding.Access != GL_READ_ONLY && + binding.Texture->GetStorageType() == TextureStorageType::Buffer; + } + + static void TrackWritableImageBufferUnit(Uint unit, Bool writableBufferTexture) { + Bool& tracked = g_writableImageBufferUnits[unit]; + if (tracked == writableBufferTexture) return; + tracked = writableBufferTexture; + // Written as two guarded steps rather than one signed add: the count is unsigned, + // and a decrement that ever ran one time too many would not saturate at zero, it + // would wrap to four billion and defeat the early-out for the rest of the process. + if (writableBufferTexture) { + ++g_writableImageBufferUnitCount; + } else if (g_writableImageBufferUnitCount > 0) { + --g_writableImageBufferUnitCount; + } + } + void SyncImageTextureBinding(Uint unit) { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(unit)); + TrackWritableImageBufferUnit(unit, IsWritableImageBufferTexture(imageBinding)); if (!imageBinding.Texture) { g_GLESFuncs.glBindImageTexture(unit, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); return; @@ -1343,6 +1380,34 @@ namespace MobileGL::MG_Backend::DirectGLES { layered, imageBinding.Layer, imageBinding.Access, imageBinding.Format); } + // A buffer texture bound to a WRITABLE image unit is a buffer the shader is about to + // write, and those writes land in the ES driver's buffer object - behind the frontend's + // CPU shadow, which is what MapBuffer and GetBufferSubData read. Same flag, and for the + // same reason, as MarkShaderStorageBuffersGpuWritten does for a storage block; the + // difference is only which binding the shader reaches the buffer through. A GL_READ_ONLY + // binding is left alone: marking it would make the next map wait on - and then re-read - + // a dispatch that could not have changed a byte of it. + // + // Called from the draw and dispatch preparations rather than from the eager sync + // glBindImageTexture performs: that one runs before any shader has touched the buffer, + // and flagging there would pull the driver's copy over a shadow the application may + // still be writing into. + void MarkWritableImageBufferTexturesGpuWritten() { + if (g_writableImageBufferUnitCount == 0) return; + for (Uint unit = 0; unit < g_writableImageBufferUnits.size(); ++unit) { + if (!g_writableImageBufferUnits[unit]) continue; + const auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(unit)); + if (!IsWritableImageBufferTexture(imageBinding)) { + TrackWritableImageBufferUnit(unit, false); + continue; + } + auto* textureBuffer = + static_cast(imageBinding.Texture.get()); + const auto& bufferObject = textureBuffer->GetBufferBindingSlot().GetBoundObject(); + if (bufferObject) bufferObject->MarkGpuWritten(); + } + } + void SyncImageTextureBindings() { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); @@ -2258,6 +2323,9 @@ namespace MobileGL::MG_Backend::DirectGLES { syncBit & DrawSyncBit::IndirectBuffer); VertexArrayImpl::SyncCurrentVAO(currentVAO, vaoTwin); TextureImpl::SyncNeccessaryTextures(textureKeys); + // A draw writes through its image units too - the conformance case that found this + // stores into a buffer texture from the FRAGMENT stage, not from a dispatch. + TextureImpl::MarkWritableImageBufferTexturesGpuWritten(); FramebufferImpl::SyncCurrentFBO(); PrgramImpl::SyncCurrentProgram(currentProgram); RenderStateImpl::SyncRenderState(); @@ -3116,6 +3184,7 @@ namespace MobileGL::MG_Backend::DirectGLES { BufferImpl::SyncComputeBuffers(includeDispatchIndirectBuffer); TextureImpl::SyncNeccessaryTextures(textureKeys); TextureImpl::SyncImageTextureBindings(); + TextureImpl::MarkWritableImageBufferTexturesGpuWritten(); PrgramImpl::SyncCurrentProgram(currentProgram); if (!currentProgram || !currentProgram->GetLinkStatus() || !currentProgram->GetSpirvStatus()) { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/BufferTextureScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/BufferTextureScenario.cpp index 9079a0c8..83c62bc5 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/BufferTextureScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/BufferTextureScenario.cpp @@ -33,6 +33,7 @@ // branch on. The driver POST's "Buffer textures" row is where that verdict is stated. #include +#include #include #include @@ -73,7 +74,60 @@ out vec4 o_color; void main() { o_color = vec4(float(vFace) / 255.0, 0.0, 0.0, 1.0); } )"; - class BufferTextureScenario : public ScenarioTest {}; + // A buffer texture bound as a WRITABLE image: the shader reads one texel and writes + // another, so a single dispatch proves the read direction (which already worked) and + // the write direction (which is what this exists for) apart from each other. + constexpr const char* kImageBufferCS = R"(#version 430 core +layout(local_size_x = 1) in; +layout(binding = 0, rgba8) uniform imageBuffer uImage; +void main() { + vec4 read = imageLoad(uImage, 1); + imageStore(uImage, 0, vec4(0.0, 1.0, 0.0, 1.0)); + imageStore(uImage, 2, read); +} +)"; + + class BufferTextureScenario : public ScenarioTest { + protected: + bool ComputeImagesAreUsable() const { + GLint maxImageUnits = 0; + GLint maxComputeImageUniforms = 0; + glGetIntegerv(GL_MAX_IMAGE_UNITS, &maxImageUnits); + glGetIntegerv(GL_MAX_COMPUTE_IMAGE_UNIFORMS, &maxComputeImageUniforms); + while (glGetError() != GL_NO_ERROR) { + } + return maxImageUnits >= 1 && maxComputeImageUniforms >= 1; + } + + unsigned int MakeComputeProgram(const char* source) { + const GLuint shader = glCreateShader(GL_COMPUTE_SHADER); + glShaderSource(shader, 1, &source, nullptr); + glCompileShader(shader); + GLint compiled = GL_FALSE; + glGetShaderiv(shader, GL_COMPILE_STATUS, &compiled); + if (compiled == GL_FALSE) { + char log[4096] = {}; + glGetShaderInfoLog(shader, sizeof(log) - 1, nullptr, log); + ADD_FAILURE() << "the compute shader did not compile: " << log; + glDeleteShader(shader); + return 0; + } + const GLuint program = glCreateProgram(); + glAttachShader(program, shader); + glLinkProgram(program); + glDeleteShader(shader); + GLint linked = GL_FALSE; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + if (linked == GL_FALSE) { + char log[4096] = {}; + glGetProgramInfoLog(program, sizeof(log) - 1, nullptr, log); + ADD_FAILURE() << "the compute program did not link: " << log; + glDeleteProgram(program); + return 0; + } + return program; + } + }; // Draws the full-viewport quad and returns the red byte every fragment was painted with, // or -1 if the quad did not come out uniform (which would mean the flat varying, not the @@ -171,4 +225,78 @@ void main() { o_color = vec4(float(vFace) / 255.0, 0.0, 0.0, 1.0); } EXPECT_EQ(FirstGLError(), 0u); } + // A shader may WRITE a buffer texture too, through an image unit, and the bytes it writes + // land in the backend's buffer - not in the frontend's CPU shadow, which is what MapBuffer + // and GetBufferSubData hand back. A storage-block write is flagged for exactly this reason + // and the shadow is refreshed on the next read; a buffer reached through an image unit is + // the same write through a different binding, and Espryt used to flag only the first, so + // an imageStore into a buffer texture was invisible to every CPU read that followed it - + // silently, with the correct value sitting in the driver's buffer the whole time. + // + // The read direction is asserted in the same dispatch (texel 2 is a copy of texel 1) so a + // failure here cannot be blamed on the image binding not working at all. + TEST_F(BufferTextureScenario, AnImageStoreIntoABufferTextureIsVisibleToTheCpu) { + if (!Ready()) return; + if (!ComputeImagesAreUsable()) GTEST_SKIP() << "no compute image units on this host"; + + constexpr GLuint kRed = 0x000000ffu; // RGBA8 little-endian: r = 255 + constexpr GLuint kGreen = 0xff00ff00u; // what the shader stores: (0, 1, 0, 1) + constexpr int kTexels = 16; + + FirstGLError(); + + const unsigned int program = MakeComputeProgram(kImageBufferCS); + ASSERT_NE(program, 0u); + + const std::vector texels(kTexels, kRed); + GLuint buffer = 0; + glGenBuffers(1, &buffer); + glBindBuffer(GL_TEXTURE_BUFFER, buffer); + glBufferData(GL_TEXTURE_BUFFER, static_cast(texels.size() * sizeof(GLuint)), texels.data(), + GL_DYNAMIC_COPY); + + GLuint texture = 0; + glGenTextures(1, &texture); + glBindTexture(GL_TEXTURE_BUFFER, texture); + glTexBuffer(GL_TEXTURE_BUFFER, GL_RGBA8, buffer); + EXPECT_EQ(FirstGLError(), 0u) << "glTexBuffer(GL_RGBA8) was refused"; + + glBindImageTexture(0, texture, 0, GL_FALSE, 0, GL_READ_WRITE, GL_RGBA8); + EXPECT_EQ(FirstGLError(), 0u) << "glBindImageTexture on a buffer texture was refused"; + + glUseProgram(program); + glDispatchCompute(1, 1, 1); + glMemoryBarrier(GL_ALL_BARRIER_BITS); + + // Both CPU read paths, because they are two entry points onto the same refresh and a + // fix that reaches only one of them is not a fix. Everything below is EXPECT rather than + // ASSERT so that a failure still reaches the cleanup at the end: the harness shares one + // context across every scenario in the process, and a leaked buffer or image binding + // here would surface as a failure somewhere else entirely. + std::vector readBack(kTexels, 0u); + glBindBuffer(GL_TEXTURE_BUFFER, buffer); + glGetBufferSubData(GL_TEXTURE_BUFFER, 0, static_cast(readBack.size() * sizeof(GLuint)), + readBack.data()); + EXPECT_EQ(readBack[0], kGreen) << "glGetBufferSubData did not see the imageStore"; + EXPECT_EQ(readBack[2], kRed) << "the imageLoad side of the same dispatch read the wrong texel"; + + const void* mapped = glMapBuffer(GL_TEXTURE_BUFFER, GL_READ_ONLY); + EXPECT_NE(mapped, nullptr) << "glMapBuffer(GL_READ_ONLY) on the texture's buffer failed"; + if (mapped != nullptr) { + GLuint mappedTexel0 = 0; + std::memcpy(&mappedTexel0, mapped, sizeof(mappedTexel0)); + EXPECT_EQ(mappedTexel0, kGreen) << "glMapBuffer did not see the imageStore"; + glUnmapBuffer(GL_TEXTURE_BUFFER); + } + + glBindImageTexture(0, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + glBindBuffer(GL_TEXTURE_BUFFER, 0); + glBindTexture(GL_TEXTURE_BUFFER, 0); + glUseProgram(0); + glDeleteProgram(program); + glDeleteTextures(1, &texture); + glDeleteBuffers(1, &buffer); + EXPECT_EQ(FirstGLError(), 0u); + } + } // namespace MGITest