swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-09 20:58:31 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix, Test] (MG_Backend/DirectGLES, MG_IntegrationTest): a shader writing a buffer texture through an image unit left the CPU shadow stale, so every map and readback after it saw the old bytes

Browse Source
This commit is contained in:
swung0x48
2026-08-12 17:50:31 -04:00
parent a51c68bb2c
commit 49cb1be0fd
2 changed files with 198 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp
+69
View File
@@ -17,6 +17,7 @@
#include <MG_Util/Metrics/TextureMetrics.h>
#include <MG_State/GLState/Core.h>
#include <MG_State/GLState/ErrorState/Error.h>
#include <MG_State/GLState/TextureState/TextureObjectBuffer.h>
#include <MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h>
#include <MG_Util/BackendLoaders/OpenGL/Loader.h>
#include <MG_Util/Converters/GLToStr/GLEnumConverter.h>
@@ -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<Bool, MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS>
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<Int>(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<Int>(unit));
if (!IsWritableImageBufferTexture(imageBinding)) {
TrackWritableImageBufferUnit(unit, false);
continue;
}
auto* textureBuffer =
static_cast<MG_State::GLState::TextureObjectBuffer*>(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()) {
MobileGL/MG_IntegrationTest/Scenarios/BufferTextureScenario.cpp
+129 -1
View File
@@ -33,6 +33,7 @@
// branch on. The driver POST's "Buffer textures" row is where that verdict is stated.
#include <cstdint>
#include <cstring>
#include <string>
#include <vector>
@@ -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<GLuint> texels(kTexels, kRed);
GLuint buffer = 0;
glGenBuffers(1, &buffer);
glBindBuffer(GL_TEXTURE_BUFFER, buffer);
glBufferData(GL_TEXTURE_BUFFER, static_cast<GLsizeiptr>(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<GLuint> readBack(kTexels, 0u);
glBindBuffer(GL_TEXTURE_BUFFER, buffer);
glGetBufferSubData(GL_TEXTURE_BUFFER, 0, static_cast<GLsizeiptr>(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