[Fix, Test] (DirectGLES): give a split buffer image its own view so the sampler still sees whole texels

This commit is contained in:
2026-08-22 06:16:52 -04:00
parent 415645ccdd
commit 7d68a17774
4 changed files with 219 additions and 11 deletions
+15 -4
View File
@@ -1490,10 +1490,17 @@ namespace MobileGL::MG_Backend::DirectGLES {
// Dim::Buffer guard there for the 32-byte GL_RG32F measurement that pinned it.
//
// What a buffer image takes instead is the SPLIT, which is the same three-layer move
// through a different door: the glTexBuffer view above and the bind below both name
// the single-channel base format, and the shader subscripts it two components per
// original texel. Same gate on both sides, so the two cannot disagree.
// through a different door: a private glTexBuffer view names the single-channel base
// format, the bind below names it too, and the shader subscripts it two components per
// original texel. Same gate on all three, so they cannot disagree.
//
// The split view is a SEPARATE texture name over the same buffer, and the bind has to
// name it rather than the application's own: the application's texture keeps the
// format it asked for so that a samplerBuffer reading the same buffer texture - which
// is NOT subscript-rewritten - still sees whole texels. See
// BackendTextureObject::m_bufferImageSplitViewId.
GLenum bindFormat = imageBinding.Format;
GLuint bindTextureId = backendTexture->GetBackendTextureId();
if (imageBinding.Texture->GetTarget() == TextureTarget::TextureBuffer) {
if (TextureImpl::GetImageBindableBufferSplitFormat(imageBinding.Texture->GetFormat()) !=
GL_UNKNOWN_MGL) {
@@ -1501,6 +1508,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
MG_Util::ConvertGLEnumToTextureInternalFormat(imageBinding.Format));
boundFormatSplit != GL_UNKNOWN_MGL) {
bindFormat = boundFormatSplit;
if (const Uint splitViewId = backendTexture->GetBufferImageSplitViewId();
splitViewId != 0) {
bindTextureId = splitViewId;
}
}
}
} else if (TextureImpl::GetImageBindableStorageWidening(imageBinding.Texture->GetFormat())) {
@@ -1510,7 +1521,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
bindFormat = boundFormatWidening.InternalFormat;
}
}
g_GLESFuncs.glBindImageTexture(unit, backendTexture->GetBackendTextureId(), imageBinding.Level,
g_GLESFuncs.glBindImageTexture(unit, bindTextureId, imageBinding.Level,
layered, layer, imageBinding.Access, bindFormat);
}
+46 -7
View File
@@ -2382,8 +2382,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
if (m_contextGeneration == g_backendContextGeneration && g_GLESFuncs.glDeleteTextures) {
g_GLESFuncs.glDeleteTextures(1, &m_backendTextureId);
if (m_bufferImageSplitViewId != 0) {
g_GLESFuncs.glDeleteTextures(1, &m_bufferImageSplitViewId);
}
}
m_backendTextureId = 0;
m_bufferImageSplitViewId = 0;
}
void BackendTextureObject::Bind(GLenum target, Uint unit) {
@@ -3828,13 +3832,17 @@ namespace MobileGL::MG_Backend::DirectGLES {
// and WidenImageFormatsPass rewrites every access to subscript it that way. Only
// for a texture that is actually image-bound: a sampled-only buffer texture keeps
// the format the application asked for (see GetImageBindableBufferSplitFormat).
if (m_imageBindableStorageRequired) {
if (const GLenum splitFormat =
TextureImpl::GetImageBindableBufferSplitFormat(textureBufferObject->GetFormat());
splitFormat != GL_UNKNOWN_MGL) {
glInternalFormat = splitFormat;
}
}
//
// The split goes on a SEPARATE name (m_bufferImageSplitViewId), not on this one.
// Re-describing the application's own texture also re-describes what a
// samplerBuffer reading it sees, and the sampler side is not subscript-rewritten -
// so texelFetch(s, i) started returning component 2i of the base view instead of
// texel i. rg32f is a legal SAMPLED buffer-texture format in ES 3.2; only the
// IMAGE binding needs the split, so only the image binding's name carries it.
const GLenum bufferImageSplitFormat =
m_imageBindableStorageRequired
? TextureImpl::GetImageBindableBufferSplitFormat(textureBufferObject->GetFormat())
: GL_UNKNOWN_MGL;
if (needsRegeneration) {
// Desktop GL has had buffer textures core since 3.1 and MobileGL advertises a
@@ -3890,6 +3898,37 @@ namespace MobileGL::MG_Backend::DirectGLES {
func, file, line, MG_Util::ConvertGLEnumToString(glInternalFormat).c_str(),
backendId, MG_Util::ConvertGLEnumToString(err).c_str());
});
// The image half of the SPLIT, on its own name over the same buffer. Minted
// lazily - only a texture that is both image-bound AND holds a format with no
// ESSL image spelling ever gets one - and re-pointed here, in the same
// regeneration gate as the view above, so the two never describe different
// buffers or different windows of one.
if (bufferImageSplitFormat != GL_UNKNOWN_MGL) {
if (m_bufferImageSplitViewId == 0) {
g_GLESFuncs.glGenTextures(1, &m_bufferImageSplitViewId);
}
if (m_bufferImageSplitViewId == 0) {
MGLOG_E_ONCE("Failed to generate the buffer-image split view for texture %u; "
"its image binding will read the unsplit view.",
stateTextureObject->GetExternalIndex());
} else {
g_GLESFuncs.glBindTexture(GL_TEXTURE_BUFFER, m_bufferImageSplitViewId);
if (rangeOffset == 0 && rangeSize == buffer->GetSize()) {
CallTexBuffer(GL_TEXTURE_BUFFER, bufferImageSplitFormat, backendId);
} else if (!CallTexBufferRange(GL_TEXTURE_BUFFER, bufferImageSplitFormat, backendId,
static_cast<GLintptr>(rangeOffset),
static_cast<GLsizeiptr>(rangeSize))) {
CallTexBuffer(GL_TEXTURE_BUFFER, bufferImageSplitFormat, backendId);
}
// The raw bind above went behind Bind()'s shadow, which tracks objects
// rather than names: leaving it claiming THIS object is bound would
// make the next Bind(GL_TEXTURE_BUFFER) a no-op and leave the split
// view bound in the application texture's place.
g_boundTexturesCache[g_activeTextureUnit][static_cast<SizeT>(
TextureTarget::TextureBuffer)] = nullptr;
}
}
}
break;
}
+23
View File
@@ -808,6 +808,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
void Bind(GLenum target, Uint unit = TempTextureUnit);
Uint GetBackendTextureId() const;
// The id to hand glBindImageTexture for a SPLIT buffer image, or 0 when this texture
// takes no split. See m_bufferImageSplitViewId.
Uint GetBufferImageSplitViewId() const { return m_bufferImageSplitViewId; }
// Aggregate first-level clean gate for the per-draw trio
// SyncTextureParamsToBackend + SyncBuiltinSamplerToBackend +
// SyncMipmapsToBackend: EXACTLY the conjunction of their own early-outs
@@ -844,6 +848,25 @@ namespace MobileGL::MG_Backend::DirectGLES {
void RecreateBackendTexture();
Uint m_backendTextureId = 0;
// A SECOND buffer-texture name over the SAME buffer object, viewed in the split's
// single-channel base format, used only as the glBindImageTexture target.
//
// The split needs the view to say r32f where the application said rg32f, but a buffer
// texture that is image-bound may ALSO be read through a samplerBuffer - and the
// sampler side is not subscript-rewritten, so re-describing the application's own
// texture broke it: texelFetch(s, i) returned component 2i of the base view instead of
// texel i's pair. That is exactly and only
// KHR-GL42/43.shader_image_load_store.advanced-sync-imageAccess, which image-stores
// into a GL_RG32F buffer texture and then reads the same texture through both an
// imageBuffer and a samplerBuffer in one shader, comparing the two.
//
// Two names over one buffer cost nothing and alias exactly: a buffer texture owns no
// storage, so both views are the application's bytes, and the split's whole premise is
// that the two describe the same memory. The application's own name therefore keeps
// the format it asked for - rg32f IS a legal SAMPLED buffer-texture format in ES 3.2,
// it is only the IMAGE binding ES cannot spell - and the private name below carries
// the split the shader was rewritten against. 0 when this texture takes no split.
Uint m_bufferImageSplitViewId = 0;
// ES context generation the id was created under; a dtor running after
// that context died must not delete a foreign (recycled) name.
Uint m_contextGeneration = 0;
@@ -1075,6 +1075,141 @@ void main()
}
}
// The SAME buffer texture read through BOTH doors at once, which is the shape the split
// originally broke. A buffer texture that is image-bound is split - the view is re-declared
// one component at a time and every image subscript is doubled to match - but the sampler
// side is NOT subscript-rewritten, so re-describing the APPLICATION's own texture made
// texelFetch(s, i) return component 2i of the base view instead of texel i's whole pair.
// The split therefore goes on a private second name over the same buffer
// (BackendTextureObject::m_bufferImageSplitViewId) and the application's name keeps the
// format it asked for: rg32f is a legal SAMPLED buffer-texture format in ES 3.2, it is only
// the IMAGE binding ES cannot spell.
//
// This is KHR-GL42/43.shader_image_load_store.advanced-sync-imageAccess reduced to one
// dispatch. That case image-stores into a GL_RG32F buffer texture and then, in one shader,
// reads the same texture through an imageBuffer AND a samplerBuffer and compares the two -
// so it went red on every pixel while its sibling -vertexArray, which never samples the
// buffer texture, passed.
//
// Like the case above this runs on every backend, and on a driver that can spell rg32f for
// an imageBuffer nothing is split at all - both doors then trivially agree, which is the
// other half of the claim: a split that fired where the driver needed none would show up
// here as the two disagreeing.
TEST_F(NonCoreImageFormatScenario, ASplitBufferImageStillSamplesWholeTexels) {
if (!Ready()) GTEST_SKIP() << "no GL context";
if (!ImagesAreUsable()) GTEST_SKIP() << "no image load/store on this driver";
GLint maxTextureBufferSize = 0;
glGetIntegerv(GL_MAX_TEXTURE_BUFFER_SIZE, &maxTextureBufferSize);
while (glGetError() != GL_NO_ERROR) {
}
if (maxTextureBufferSize <= 0) GTEST_SKIP() << "no buffer textures on this driver";
constexpr int kBufferTexels = 4;
// Both components of every texel distinct and non-zero, so a sampler that reads the
// SPLIT view cannot accidentally agree: texel i would come back as (2i-th component,
// 0, 0, 1) rather than (x, y, 0, 1), and every one of those is a value no texel holds.
std::vector<float> seed(static_cast<std::size_t>(kBufferTexels) * 2u, 0.0f);
for (int texel = 0; texel < kBufferTexels; ++texel) {
seed[static_cast<std::size_t>(texel) * 2u + 0u] = static_cast<float>(texel * 10 + 1);
seed[static_cast<std::size_t>(texel) * 2u + 1u] = static_cast<float>(texel * 10 + 2);
}
GLuint buffer = 0;
glGenBuffers(1, &buffer);
glBindBuffer(GL_TEXTURE_BUFFER, buffer);
glBufferData(GL_TEXTURE_BUFFER, static_cast<GLsizeiptr>(seed.size() * sizeof(float)), seed.data(),
GL_DYNAMIC_DRAW);
GLuint texture = 0;
glGenTextures(1, &texture);
m_textures.push_back(texture);
glBindTexture(GL_TEXTURE_BUFFER, texture);
glTexBuffer(GL_TEXTURE_BUFFER, GL_RG32F, buffer);
if (const GLenum error = FirstGLError()) {
glDeleteBuffers(1, &buffer);
GTEST_SKIP() << "glTexBuffer(GL_RG32F) errored with " << GLErrorName(error);
}
// The answer buffer is rgba32f, which IS core ESSL, so it is never split and cannot
// hide a mistake in the thing under test.
constexpr int kAnswers = kBufferTexels * 2;
const std::vector<float> answerSeed(static_cast<std::size_t>(kAnswers) * 4u, -12345.0f);
GLuint answerBuffer = 0;
glGenBuffers(1, &answerBuffer);
glBindBuffer(GL_TEXTURE_BUFFER, answerBuffer);
glBufferData(GL_TEXTURE_BUFFER, static_cast<GLsizeiptr>(answerSeed.size() * sizeof(float)),
answerSeed.data(), GL_DYNAMIC_DRAW);
GLuint answerTexture = 0;
glGenTextures(1, &answerTexture);
m_textures.push_back(answerTexture);
glBindTexture(GL_TEXTURE_BUFFER, answerTexture);
glTexBuffer(GL_TEXTURE_BUFFER, GL_RGBA32F, answerBuffer);
if (const GLenum error = FirstGLError()) {
glDeleteBuffers(1, &buffer);
glDeleteBuffers(1, &answerBuffer);
GTEST_SKIP() << "glTexBuffer(GL_RGBA32F) errored with " << GLErrorName(error);
}
const GLuint program = MakeComputeProgram(R"(#version 430 core
layout (local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
layout (rg32f, binding = 0) readonly uniform imageBuffer narrow;
layout (rgba32f, binding = 1) writeonly uniform imageBuffer answers;
uniform samplerBuffer sampled;
void main()
{
int texel = int(gl_GlobalInvocationID.x);
imageStore(answers, texel * 2 + 0, imageLoad(narrow, texel));
imageStore(answers, texel * 2 + 1, texelFetch(sampled, texel));
}
)");
if (program == 0) {
glDeleteBuffers(1, &buffer);
glDeleteBuffers(1, &answerBuffer);
return;
}
BindImage(kNarrowUnit, texture, GL_RG32F, GL_READ_ONLY);
BindImage(kWideUnit, answerTexture, GL_RGBA32F, GL_WRITE_ONLY);
glActiveTexture(GL_TEXTURE0);
glBindTexture(GL_TEXTURE_BUFFER, texture);
glUseProgram(program);
glUniform1i(glGetUniformLocation(program, "sampled"), 0);
glDispatchCompute(kBufferTexels, 1, 1);
glMemoryBarrier(GL_ALL_BARRIER_BITS);
EXPECT_EQ(FirstGLError(), 0u) << "the dispatch leaked a GL error";
glUseProgram(0);
std::vector<float> readback(answerSeed.size(), -54321.0f);
glBindBuffer(GL_TEXTURE_BUFFER, answerBuffer);
glGetBufferSubData(GL_TEXTURE_BUFFER, 0,
static_cast<GLsizeiptr>(readback.size() * sizeof(float)), readback.data());
EXPECT_EQ(FirstGLError(), 0u) << "reading the answers back errored";
for (int texel = 0; texel < kBufferTexels; ++texel) {
const float red = static_cast<float>(texel * 10 + 1);
const float green = static_cast<float>(texel * 10 + 2);
const std::size_t viaImage = static_cast<std::size_t>(texel) * 8u;
const std::size_t viaSampler = viaImage + 4u;
EXPECT_FLOAT_EQ(readback[viaImage + 0u], red) << "texel " << texel << " imageLoad red";
EXPECT_FLOAT_EQ(readback[viaImage + 1u], green) << "texel " << texel << " imageLoad green";
EXPECT_FLOAT_EQ(readback[viaSampler + 0u], red) << "texel " << texel << " texelFetch red";
EXPECT_FLOAT_EQ(readback[viaSampler + 1u], green)
<< "texel " << texel
<< " texelFetch green: a samplerBuffer must see whole texels even where the "
"image side of the same texture was split";
EXPECT_FLOAT_EQ(readback[viaSampler + 2u], 0.0f) << "texel " << texel << " texelFetch blue";
EXPECT_FLOAT_EQ(readback[viaSampler + 3u], 1.0f) << "texel " << texel << " texelFetch alpha";
}
glBindBuffer(GL_TEXTURE_BUFFER, 0);
glDeleteBuffers(1, &buffer);
glDeleteBuffers(1, &answerBuffer);
while (glGetError() != GL_NO_ERROR) {
}
}
// The other consumer of the same texture. A widened texture's ES storage really does have
// four channels, so a sampler reading it raw would see whatever the carrier holds; the
// logical format's missing channels have to keep reading 0 and 1 (which Espryt arranges