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

[Fix, Test] (DirectGLES): name a repaired image uniform after its repair, not after its stage

Browse Source
This commit is contained in:
swung0x48
2026-08-21 06:51:12 -04:00
parent 8587b83be3
commit 37a656dedc
4 changed files with 249 additions and 155 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/Managers.cpp
+5 -4
View File
@@ -6437,11 +6437,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
// declaration and preserves its binding - an image unit cannot be set from
// the API in ES, so the qualifier is the only binding mechanism there is,
// and both halves of the pair have to still be carrying theirs when it runs.
// * and it needs the STAGE, because the qualifier it adds is a per-stage decision
// and the rename that keeps two stages from declaring one image uniform
// differently is keyed on it.
// Takes no stage: the qualifier it adds is a decision about THIS text's accesses
// and the rename that keeps two stages from declaring one image uniform
// differently is keyed on that same decision, so two stages that agree still
// share one uniform (see the location-budget note on the pass).
Uint splitImageUniformCount = 0;
source = SplitReadWriteImageUniforms(source, glShaderType, &splitImageUniformCount);
source = SplitReadWriteImageUniforms(source, &splitImageUniformCount);
if (splitImageUniformCount != 0) {
splitImageUniformStages.push_back({shader->GetShaderStage(), splitImageUniformCount});
}
MobileGL/MG_Backend/DirectGLES/Utils.cpp
+47 -48
View File
@@ -933,7 +933,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
struct ImageUniformDecl {
String name;
String stageName; // the stage-tagged name the rewritten declaration takes; empty
String aliasName; // the repair-tagged name the rewritten declaration takes; empty
// for a declaration this pass leaves alone
String writeName; // the writeonly half's name, when split
String layout; // raw contents of layout(...)
@@ -1029,21 +1029,6 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
} // namespace
String ImageStageAliasPrefix(GLenum shaderType) {
switch (shaderType) {
case GL_VERTEX_SHADER: return String(IMAGE_STAGE_ALIAS_PREFIX) + "Vs_";
case GL_FRAGMENT_SHADER: return String(IMAGE_STAGE_ALIAS_PREFIX) + "Fs_";
case GL_COMPUTE_SHADER: return String(IMAGE_STAGE_ALIAS_PREFIX) + "Cs_";
case GL_GEOMETRY_SHADER: return String(IMAGE_STAGE_ALIAS_PREFIX) + "Gs_";
case GL_TESS_CONTROL_SHADER: return String(IMAGE_STAGE_ALIAS_PREFIX) + "Tcs_";
case GL_TESS_EVALUATION_SHADER: return String(IMAGE_STAGE_ALIAS_PREFIX) + "Tes_";
// A stage this build does not know. Still a tag of its own rather than the bare
// name, so the anti-collision property below never depends on the switch being
// exhaustive - it only stops being able to say WHICH stage a name came from.
default: return String(IMAGE_STAGE_ALIAS_PREFIX) + "Xs_";
}
}
namespace {
// The digits of an array extent or of an element subscript, or -1 for "not a plain
// decimal literal".
@@ -1260,7 +1245,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
return result;
}
String SplitReadWriteImageUniforms(const String& glslCode, GLenum shaderType, Uint* outSplitCount) {
String SplitReadWriteImageUniforms(const String& glslCode, Uint* outSplitCount) {
#ifdef TRACY_ENABLE
ZoneScopedC(TRACY_ZONECOLOR_BACKEND);
#endif
@@ -1405,38 +1390,52 @@ namespace MobileGL::MG_Backend::DirectGLES {
Vector<ImageSourceEdit> edits;
Vector<String> takenNames;
const String stagePrefix = ImageStageAliasPrefix(shaderType);
for (auto& decl : decls) {
if (decl.unknownUse) continue; // leave it exactly as it was; no guessing
// EVERY declaration this pass rewrites is also RENAMED, per stage - the
// qualifier it is about to add is a per-stage decision, and GLSL requires a
// uniform declared in two stages to be declared IDENTICALLY (GLSL 4.3 4.3.9 /
// GLSL ES 3.20 4.3.9). A shader that stores to an image in the vertex stage and
// loads it in the fragment stage gets `writeonly` on one and `readonly` on the
// other, and on Adreno the linker merges the two same-named declarations and
// SILENTLY DISCARDS the vertex-stage stores: no GL error, no link log,
// LINK_STATUS = 1, and the image still holding its initial contents afterwards
// EVERY declaration this pass rewrites is also RENAMED, under the prefix of the
// repair it is about to receive - the qualifier below is a decision about ONE
// STAGE's accesses, and GLSL requires a uniform declared in two stages to be
// declared IDENTICALLY (GLSL 4.3 4.3.9 / GLSL ES 3.20 4.3.9). A shader that
// stores to an image in the vertex stage and loads it in the fragment stage gets
// `writeonly` on one and `readonly` on the other, and on Adreno the linker merges
// the two same-named declarations and SILENTLY DISCARDS the vertex-stage stores:
// no GL error, no link log, LINK_STATUS = 1, and the image still holding its
// initial contents afterwards
// (KHR-GL4x.shader_image_load_store.advanced-memory-dependentInvocation, and any
// shader pack that writes an image in one stage to read it in another). A
// per-stage name means there is no cross-stage variable left to merge.
// shader pack that writes an image in one stage to read it in another).
//
// Unconditional, rather than only when another stage is known to declare the same
// name, because this pass sees ONE stage at a time and a conditional rename would
// have to guess. Nothing downstream reads these names: the two passes that key on
// the GL uniform name (RebindImageUniformsToFrontendUnits, BakeImageFormatQualifiers)
// both run BEFORE this one, RemoveLayoutBinding recognises an image declaration by
// its TYPE token, and CacheResourceLocations skips image uniforms outright because
// ES image units come only from layout(binding=N). The declarations this pass
// LEAVES ALONE - already readonly/writeonly in the source, or r32f/r32i/r32ui,
// which need no qualifier - keep their names, and they are exactly the ones that
// already match across stages.
decl.stageName = MakeImageAliasName(stagePrefix, decl.name, glslCode, takenNames);
takenNames.push_back(decl.stageName);
// Keyed on the REPAIR and not on the stage, which is what makes the rename
// exactly as wide as the problem. Two stages that use the image the same way
// reach the same prefix and emit byte-identical declarations, so they keep ONE
// shared uniform and there is nothing mismatched to merge; two that use it
// differently reach different prefixes and cannot be merged at all. Tagging by
// stage instead also broke the merge - but it broke it for the agreeing stages
// too, turning one image uniform into one PER STAGE that names it, and Adreno
// allocates image locations per distinct uniform: the five stages of
// KHR-GL43.shading_language_420pack.binding_images_texture_type_* went from 6
// image uniforms to 30 and the link failed outright with "Error: Image Image
// location or component exceeds max allowed." on an Adreno 830, where Mali and
// Mesa both accept the same text.
//
// Nothing downstream reads these names: the two passes that key on the GL uniform
// name (RebindImageUniformsToFrontendUnits, BakeImageFormatQualifiers) both run
// BEFORE this one, RemoveLayoutBinding recognises an image declaration by its TYPE
// token, and CacheResourceLocations skips image uniforms outright because ES image
// units come only from layout(binding=N). The declarations this pass LEAVES ALONE -
// already readonly/writeonly in the source, or r32f/r32i/r32ui, which need no
// qualifier - keep their names, and they are exactly the ones that already match
// across stages.
const char* aliasPrefix = decl.loaded && decl.stored ? IMAGE_SPLIT_READ_ALIAS_PREFIX
: decl.stored ? IMAGE_WRITEONLY_ALIAS_PREFIX
: IMAGE_READONLY_ALIAS_PREFIX;
decl.aliasName = MakeImageAliasName(aliasPrefix, decl.name, glslCode, takenNames);
takenNames.push_back(decl.aliasName);
if (decl.loaded && decl.stored) {
// Minted from the ALREADY stage-tagged name, so two stages that both split
// the same image do not collide on the write half either.
// Minted from the ALREADY access-tagged name, so the write half of a split
// can never collide with the single declaration another stage's repair mints
// for the same image.
decl.writeName =
MakeImageAliasName(IMAGE_WRITE_ALIAS_PREFIX, decl.stageName, glslCode, takenNames);
MakeImageAliasName(IMAGE_WRITE_ALIAS_PREFIX, decl.aliasName, glslCode, takenNames);
takenNames.push_back(decl.writeName);
decl.split = true;
if (outSplitCount != nullptr) ++*outSplitCount;
@@ -1445,28 +1444,28 @@ namespace MobileGL::MG_Backend::DirectGLES {
// there is no visibility to restore and no reason to pay for the cache
// behaviour.
edits.push_back({decl.declStart, decl.declLength,
BuildImageDeclaration(decl, "readonly", decl.stageName,
BuildImageDeclaration(decl, "readonly", decl.aliasName,
/*forceCoherent=*/true) +
"\n" +
BuildImageDeclaration(decl, "writeonly", decl.writeName,
/*forceCoherent=*/true)});
} else if (decl.stored) {
edits.push_back({decl.declStart, decl.declLength,
BuildImageDeclaration(decl, "writeonly", decl.stageName)});
BuildImageDeclaration(decl, "writeonly", decl.aliasName)});
} else {
// Loaded only, or only ever handed to imageSize (or unused): readonly is
// the qualifier that keeps every one of those legal.
edits.push_back({decl.declStart, decl.declLength,
BuildImageDeclaration(decl, "readonly", decl.stageName)});
BuildImageDeclaration(decl, "readonly", decl.aliasName)});
}
}
for (const ImageUseSite& site : useSites) {
const ImageUniformDecl& decl = decls[site.declIndex];
// Empty exactly when the declaration was poisoned above and left untouched; its
// uses must keep naming the variable that is still called that.
if (decl.stageName.empty()) continue;
if (decl.aliasName.empty()) continue;
edits.push_back(
{site.start, site.length, decl.split && site.stores ? decl.writeName : decl.stageName});
{site.start, site.length, decl.split && site.stores ? decl.writeName : decl.aliasName});
if (!decl.split || !site.stores) continue;
// ...and an explicit barrier behind it. `coherent` on both halves is what makes
// the store VISIBLE to a load through the other variable, but it says nothing
MobileGL/MG_Backend/DirectGLES/Utils.h
+44 -29
View File
@@ -332,15 +332,16 @@ namespace MobileGL::MG_Backend::DirectGLES {
const String& inPerVertexMembers,
const String& outPerVertexMembers);
// Prefix of the writeonly half a read+write image uniform is split into (see
// SplitReadWriteImageUniforms); the suffix is the image's own (already stage-tagged) name.
// SplitReadWriteImageUniforms); the suffix is the image's own (already access-tagged) name.
constexpr const char* IMAGE_WRITE_ALIAS_PREFIX = "mg_imageWrite_";
// Stem of the per-stage name every image declaration SplitReadWriteImageUniforms rewrites
// is renamed under; ImageStageAliasPrefix appends the stage tag and the separator.
constexpr const char* IMAGE_STAGE_ALIAS_PREFIX = "mg_image";
// "mg_imageVs_", "mg_imageFs_", "mg_imageCs_", ... - the prefix SplitReadWriteImageUniforms
// renames a rewritten image declaration under, so that no two stages can end up declaring
// the same image uniform name with different memory qualifiers. Exposed for the tests.
String ImageStageAliasPrefix(GLenum shaderType);
// The three names SplitReadWriteImageUniforms renames a rewritten image declaration
// under, one per REPAIR it can apply. Which one a stage picks is decided by that stage's
// own accesses, so two stages that use an image the same way arrive at the SAME name and
// two that use it differently arrive at different ones - which is exactly the property
// the rename exists for, at no cost to the stages that agree. Exposed for the tests.
constexpr const char* IMAGE_READONLY_ALIAS_PREFIX = "mg_imageRo_";
constexpr const char* IMAGE_WRITEONLY_ALIAS_PREFIX = "mg_imageWo_";
constexpr const char* IMAGE_SPLIT_READ_ALIAS_PREFIX = "mg_imageRw_";
// ESSL refuses an image variable that carries a format qualifier other than r32f /
// r32i / r32ui unless it also carries `readonly` or `writeonly` (GLSL ES 3.10 4.9 /
// 3.20 4.10; glslang enforces it verbatim in ParseHelper.cpp's layoutObjectCheck).
@@ -352,31 +353,47 @@ namespace MobileGL::MG_Backend::DirectGLES {
// bare declaration, so the frontend raises no error and the illegal ESSL only shows
// up as a device compile failure - and then as a silently no-op draw.
//
// Restores a legal declaration, and RENAMES it per stage while doing so:
// * loaded only -> add `readonly`
// * stored only -> add `writeonly`
// Restores a legal declaration, and RENAMES it after the repair it applied while doing so:
// * loaded only -> add `readonly`, rename under IMAGE_READONLY_ALIAS_PREFIX
// * stored only -> add `writeonly`, rename under IMAGE_WRITEONLY_ALIAS_PREFIX
// * both -> emit TWO declarations on the same binding and of the
// same type, `coherent readonly <name>` and `coherent
// writeonly <IMAGE_WRITE_ALIAS_PREFIX><name>`, point
// same type, `coherent readonly
// <IMAGE_SPLIT_READ_ALIAS_PREFIX><name>` and `coherent
// writeonly <IMAGE_WRITE_ALIAS_PREFIX><that name>`, point
// every imageStore at the second one, and follow each of
// those stores with `memoryBarrierImage();`. Several image
// variables may share an image unit as long as they have
// the same type and format, which is exactly what the pair
// is.
//
// The rename (ImageStageAliasPrefix) is the other half of the repair and applies to all
// three cases. The qualifier chosen above is a decision about ONE STAGE's accesses, and
// GLSL requires a uniform declared in two stages to be declared identically - so a shader
// that stores an image from the vertex stage and loads it from the fragment stage came out
// of here `writeonly` in one and `readonly` in the other. Adreno merges the two same-named
// declarations and silently drops the vertex-stage STORES: no GL error, no link log,
// LINK_STATUS = 1, and the image still reads back its initial contents
// The rename is the other half of the repair and applies to all three cases. The qualifier
// chosen above is a decision about ONE STAGE's accesses, and GLSL requires a uniform
// declared in two stages to be declared identically - so a shader that stores an image from
// the vertex stage and loads it from the fragment stage came out of here `writeonly` in one
// and `readonly` in the other. Adreno merges the two same-named declarations and silently
// drops the vertex-stage STORES: no GL error, no link log, LINK_STATUS = 1, and the image
// still reads back its initial contents
// (KHR-GL4x.shader_image_load_store.advanced-memory-dependentInvocation; a raw-ES probe
// isolated the trigger to the same-name/mismatched-qualifier pair, and only when both
// carry `coherent`). A per-stage name leaves no cross-stage variable to merge. Every
// rewritten declaration is renamed, including the readonly half of a split pair; the
// declarations this pass leaves untouched keep their names, and those are exactly the ones
// that already agree across stages.
// carry `coherent`). Renaming leaves no cross-stage variable to merge.
//
// The name is keyed on the REPAIR, not on the stage, and that distinction is the whole
// point: two stages that use an image the same way emit byte-identical declarations, so
// letting them keep one shared name costs nothing and merging them is correct, while two
// stages that use it differently land on different prefixes and cannot be merged at all.
// A per-STAGE tag also satisfied the first requirement but violated the second: it made
// the SAME image a distinct uniform in every stage that named it, and Adreno allocates
// image LOCATIONS per distinct uniform. KHR-GL43.shading_language_420pack.
// binding_images_texture_type_* declares three read+write images in each of its five
// stages; merged that is 6 image uniforms, per-stage-tagged it is 30, and the Adreno 830
// linker answered "Error: Image Image location or component exceeds max allowed. Error:
// Linking failed." - which, the frontend having already published LINK_STATUS = TRUE from
// glslang's link, surfaced only as every draw silently doing nothing and the images
// reading back zero. Mali and Mesa link the same text, so nothing but a device gate
// catches this.
//
// The declarations this pass leaves untouched keep their names, and those are exactly the
// ones that already agree across stages.
//
// The `coherent` on both halves of the pair is load-bearing, not decoration: GLSL only
// guarantees a write through one image variable is visible to a read through a DIFFERENT
@@ -401,16 +418,14 @@ namespace MobileGL::MG_Backend::DirectGLES {
//
// Runs on the transpiled ESSL, so it must see the bindings the frontend units were
// already rewritten to and must run before those bindings are stripped - see the call
// site in Managers.cpp. It is downstream of the L2 shader-translation memo (which stores
// what SPIRV-Cross emitted, before any of these text passes), so `shaderType` steering the
// names it mints needs no entry in BuildEsslTranslationKey.
// site in Managers.cpp. Its output is a function of the emitted text alone - it needs no
// stage and no per-program state - so it adds nothing to BuildEsslTranslationKey either.
//
// `outSplitCount`, when given, receives the number of declarations that were actually
// doubled - i.e. exactly how many image uniforms this stage gained over what the
// application declared. Zero for every shader but a handful, and the only number the
// budget note above can be reported with.
String SplitReadWriteImageUniforms(const String& glslCode, GLenum shaderType,
Uint* outSplitCount = nullptr);
String SplitReadWriteImageUniforms(const String& glslCode, Uint* outSplitCount = nullptr);
// Prefix of the per-sampler float uniform that carries GL_TEXTURE_LOD_BIAS into
// the shader (see EmulateTextureLodBias); the suffix is the sampler's own name.
constexpr const char* LOD_BIAS_UNIFORM_PREFIX = "mg_lodBias_";
MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp
+153 -74
View File
@@ -20,11 +20,12 @@ using MobileGL::MG_Backend::DirectGLES::PrgramImpl::BakeImageFormatQualifiers;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::BuildPassthroughTessControlEssl;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::ExtractPerVertexBlockMembers;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::ForceFlatIntegerVaryings;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_STAGE_ALIAS_PREFIX;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_ARRAY_ELEMENT_PREFIX;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_READONLY_ALIAS_PREFIX;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_SPLIT_READ_ALIAS_PREFIX;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_WRITE_ALIAS_PREFIX;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_WRITEONLY_ALIAS_PREFIX;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::ImageArrayUnitPlan;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::ImageStageAliasPrefix;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RemapImageArrayElementUnits;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RemoveLayoutBinding;
using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RequestExtendedImageFormats;
@@ -44,14 +45,14 @@ namespace {
return count;
}
// Every fixture below is a fragment shader unless it says otherwise. The pass tags the name
// of every declaration it rewrites with the stage it ran on, so the expectations have to
// spell the same tag.
constexpr GLenum kStage = GL_FRAGMENT_SHADER;
String StageAlias(const String& name) { return ImageStageAliasPrefix(kStage) + name; }
// The writeonly half is minted from the ALREADY stage-tagged name, so it carries both.
// The pass tags the name of every declaration it rewrites with the REPAIR it applied, so the
// expectations have to spell the tag that matches how the fixture uses the image.
String RoAlias(const String& name) { return String(IMAGE_READONLY_ALIAS_PREFIX) + name; }
String WoAlias(const String& name) { return String(IMAGE_WRITEONLY_ALIAS_PREFIX) + name; }
String RwAlias(const String& name) { return String(IMAGE_SPLIT_READ_ALIAS_PREFIX) + name; }
// The writeonly half is minted from the ALREADY access-tagged name, so it carries both.
String WriteAlias(const String& name) { return String(IMAGE_WRITE_ALIAS_PREFIX) + name; }
String SplitWriteAlias(const String& name) { return WriteAlias(StageAlias(name)); }
String SplitWriteAlias(const String& name) { return WriteAlias(RwAlias(name)); }
// The scalar RemapImageArrayElementUnits declares for one element of a split image array.
String Elem(const String& name, Int element) {
return String(IMAGE_ARRAY_ELEMENT_PREFIX) + name + "_" + std::to_string(element);
@@ -74,13 +75,13 @@ void main()
mg_FragColor = loaded;
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
const String out = SplitReadWriteImageUniforms(source);
// Both halves: same binding, same format, same type - which is what makes two image
// variables on one image unit legal - and both `coherent`, which is what makes the store
// through one of them visible to the load through the other.
EXPECT_TRUE(Contains(out, "layout(binding = 2, rgba8) uniform coherent readonly highp image2D " +
StageAlias("goku") + ";"))
RwAlias("goku") + ";"))
<< out;
EXPECT_TRUE(Contains(out, "layout(binding = 2, rgba8) uniform coherent writeonly highp image2D " +
SplitWriteAlias("goku") + ";"))
@@ -88,7 +89,7 @@ void main()
// The load goes to the readonly half, the store to the writeonly one, and neither is called
// what the application called it any more.
EXPECT_TRUE(Contains(out, "imageLoad(" + StageAlias("goku") + ","));
EXPECT_TRUE(Contains(out, "imageLoad(" + RwAlias("goku") + ","));
EXPECT_TRUE(Contains(out, "imageStore(" + SplitWriteAlias("goku") + ","));
EXPECT_FALSE(Contains(out, "imageStore(goku,"));
EXPECT_FALSE(Contains(out, "imageLoad(goku,"));
@@ -105,7 +106,7 @@ void main()
imageStore(goku, ivec2(0), imageLoad(goku, ivec2(0)));
}
)";
const String out = RemoveLayoutBinding(SplitReadWriteImageUniforms(source, kStage));
const String out = RemoveLayoutBinding(SplitReadWriteImageUniforms(source));
EXPECT_EQ(CountOf(out, "binding = 5"), 2u);
}
@@ -120,11 +121,11 @@ void main()
mg_FragColor = imageLoad(trunks, ivec3(0));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(Contains(out, "layout(binding = 1, rgba16f) uniform readonly highp image2DArray " +
StageAlias("trunks") + ";"))
RoAlias("trunks") + ";"))
<< out;
EXPECT_TRUE(Contains(out, "imageLoad(" + StageAlias("trunks") + ","));
EXPECT_TRUE(Contains(out, "imageLoad(" + RoAlias("trunks") + ","));
EXPECT_FALSE(Contains(out, "writeonly"));
EXPECT_FALSE(Contains(out, IMAGE_WRITE_ALIAS_PREFIX));
EXPECT_EQ(CountOf(out, "image2DArray"), 1u);
@@ -138,11 +139,11 @@ void main()
imageStore(gohan, ivec2(0), vec4(1.0));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(
Contains(out, "layout(binding = 3, rgba8) uniform writeonly highp image2D " + StageAlias("gohan") + ";"))
Contains(out, "layout(binding = 3, rgba8) uniform writeonly highp image2D " + WoAlias("gohan") + ";"))
<< out;
EXPECT_TRUE(Contains(out, "imageStore(" + StageAlias("gohan") + ","));
EXPECT_TRUE(Contains(out, "imageStore(" + WoAlias("gohan") + ","));
EXPECT_FALSE(Contains(out, "readonly"));
EXPECT_FALSE(Contains(out, IMAGE_WRITE_ALIAS_PREFIX));
}
@@ -155,7 +156,7 @@ TEST(SplitReadWriteImageUniformsTest, ExemptFormatsAreLeftCompletelyAlone) {
const String source = "#version 320 es\nlayout(binding = 4, " + String(format) + ") uniform highp " + type +
" vegeta;\nvoid main()\n{\n imageStore(vegeta, ivec2(0), imageLoad(vegeta, "
"ivec2(0)));\n}\n";
EXPECT_EQ(SplitReadWriteImageUniforms(source, kStage), source) << "format " << format;
EXPECT_EQ(SplitReadWriteImageUniforms(source), source) << "format " << format;
}
}
@@ -172,7 +173,7 @@ void main()
// Untouched means UNRENAMED too: a declaration that already carries its qualifier in the
// source carries the SAME one in every stage, so there is no cross-stage mismatch to break up
// and renaming it would only churn the text.
EXPECT_EQ(SplitReadWriteImageUniforms(source, kStage), source);
EXPECT_EQ(SplitReadWriteImageUniforms(source), source);
}
// The binding of an image array is the array's base; splitting must keep the array on both
@@ -185,15 +186,15 @@ void main()
imageStore(gohan[1], ivec2(0), imageLoad(gohan[2], ivec2(0)));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(Contains(out, "layout(binding = 6, rgba8) uniform coherent readonly highp image2D " +
StageAlias("gohan") + "[3];"))
RwAlias("gohan") + "[3];"))
<< out;
EXPECT_TRUE(Contains(out, "layout(binding = 6, rgba8) uniform coherent writeonly highp image2D " +
SplitWriteAlias("gohan") + "[3];"))
<< out;
EXPECT_TRUE(Contains(out, "imageStore(" + SplitWriteAlias("gohan") + "[1],"));
EXPECT_TRUE(Contains(out, "imageLoad(" + StageAlias("gohan") + "[2],"));
EXPECT_TRUE(Contains(out, "imageLoad(" + RwAlias("gohan") + "[2],"));
}
// The rewrite is by identifier, not by substring: "goku" must not reach into "goku_hd", and
@@ -209,20 +210,20 @@ void main()
imageStore(goku_hd, ivec2(0), loaded);
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
const String out = SplitReadWriteImageUniforms(source);
// goku is read+write -> split (and coherent with it); goku_hd is write-only -> qualified in
// place, not split, and left non-coherent because nothing aliases it. Both are renamed.
EXPECT_TRUE(Contains(out, "layout(binding = 1, rgba8) uniform coherent readonly highp image2D " +
StageAlias("goku") + ";"))
RwAlias("goku") + ";"))
<< out;
EXPECT_TRUE(Contains(out, "layout(binding = 1, rgba8) uniform coherent writeonly highp image2D " +
SplitWriteAlias("goku") + ";"))
<< out;
EXPECT_TRUE(Contains(out, "layout(binding = 2, rgba8) uniform writeonly highp image2D " +
StageAlias("goku_hd") + ";"))
WoAlias("goku_hd") + ";"))
<< out;
EXPECT_TRUE(Contains(out, "imageStore(" + StageAlias("goku_hd") + ","));
EXPECT_TRUE(Contains(out, "imageStore(" + WoAlias("goku_hd") + ","));
EXPECT_FALSE(Contains(out, SplitWriteAlias("goku") + "_hd"));
EXPECT_FALSE(Contains(out, SplitWriteAlias("goku_hd")));
}
@@ -237,8 +238,8 @@ void main()
imageStore(goku, ivec2(0), imageLoad(goku, ivec2(0)));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
EXPECT_TRUE(Contains(out, "uniform readonly coherent restrict highp image2D " + StageAlias("goku") + ";"))
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(Contains(out, "uniform readonly coherent restrict highp image2D " + RwAlias("goku") + ";"))
<< out;
EXPECT_TRUE(
Contains(out, "uniform writeonly coherent restrict highp image2D " + SplitWriteAlias("goku") + ";"))
@@ -266,14 +267,14 @@ void main()
imageStore(storeOnly, ivec2(0), vec4(2.0));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
EXPECT_TRUE(Contains(out, "uniform coherent readonly highp image2D " + StageAlias("goku") + ";")) << out;
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(Contains(out, "uniform coherent readonly highp image2D " + RwAlias("goku") + ";")) << out;
EXPECT_TRUE(Contains(out, "uniform coherent writeonly highp image2D " + SplitWriteAlias("goku") + ";"))
<< out;
// Exactly the two halves of the pair, and nothing else: the store-only image is repaired in
// place, has no alias to stay visible to, and must not pay for uncached access.
EXPECT_EQ(CountOf(out, "coherent"), 2u);
EXPECT_TRUE(Contains(out, "uniform writeonly highp image2D " + StageAlias("storeOnly") + ";")) << out;
EXPECT_TRUE(Contains(out, "uniform writeonly highp image2D " + WoAlias("storeOnly") + ";")) << out;
}
// The ORDERING half of the split, which `coherent` alone does not buy. Coherent makes the store
@@ -294,7 +295,7 @@ void main()
mg_FragColor = first + imageLoad(goku, ivec2(0));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(
Contains(out, "imageStore(" + SplitWriteAlias("goku") + ", ivec2(0), vec4(1.0)); memoryBarrierImage();"))
@@ -317,8 +318,8 @@ void main()
imageStore(storeOnly, ivec2(0), vec4(1.0));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
EXPECT_TRUE(Contains(out, "uniform writeonly highp image2D " + StageAlias("storeOnly") + ";")) << out;
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(Contains(out, "uniform writeonly highp image2D " + WoAlias("storeOnly") + ";")) << out;
EXPECT_FALSE(Contains(out, "memoryBarrierImage")) << out;
}
@@ -333,8 +334,8 @@ void main()
imageStore(gohan[1], ivec2(0), max(imageLoad(gohan[2], ivec2(0)), vec4(0.5)));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
EXPECT_TRUE(Contains(out, "max(imageLoad(" + StageAlias("gohan") +
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(Contains(out, "max(imageLoad(" + RwAlias("gohan") +
"[2], ivec2(0)), vec4(0.5))); memoryBarrierImage();"))
<< out;
EXPECT_EQ(CountOf(out, "memoryBarrierImage();"), 1u) << out;
@@ -359,7 +360,7 @@ void main()
}
)";
Uint splitCount = 99u;
SplitReadWriteImageUniforms(twoSplits, kStage, &splitCount);
SplitReadWriteImageUniforms(twoSplits, &splitCount);
EXPECT_EQ(splitCount, 2u) << "only the read+write pair counts; the store-only repair adds no uniform";
// Every early return has to write the count too, or a caller reads whatever was there before.
@@ -371,7 +372,7 @@ void main()
}
)";
splitCount = 99u;
SplitReadWriteImageUniforms(noImages, kStage, &splitCount);
SplitReadWriteImageUniforms(noImages, &splitCount);
EXPECT_EQ(splitCount, 0u);
}
@@ -386,20 +387,20 @@ void main()
mg_FragColor = vec4(float(imageSize(sizeOnly).x));
}
)";
const String out = SplitReadWriteImageUniforms(source, kStage);
const String out = SplitReadWriteImageUniforms(source);
EXPECT_TRUE(Contains(out, "layout(binding = 8, rgba8ui) uniform readonly highp uimage2D " +
StageAlias("sizeOnly") + ";"))
RoAlias("sizeOnly") + ";"))
<< out;
// The rename has to reach imageSize too, or the declaration and its only use stop agreeing.
EXPECT_TRUE(Contains(out, "imageSize(" + StageAlias("sizeOnly") + ")")) << out;
EXPECT_TRUE(Contains(out, "imageSize(" + RoAlias("sizeOnly") + ")")) << out;
EXPECT_FALSE(Contains(out, IMAGE_WRITE_ALIAS_PREFIX));
}
// Neither minted name may land on an identifier the shader already uses - and there are two of
// them now, the stage-tagged name of the repaired declaration and the writeonly half built on
// them now, the access-tagged name of the repaired declaration and the writeonly half built on
// top of it. Both collisions are exercised at once.
TEST(SplitReadWriteImageUniformsTest, AliasNamesAvoidExistingIdentifiers) {
const String stageCollision = StageAlias("taken");
const String stageCollision = RwAlias("taken");
const String writeCollision = SplitWriteAlias("taken");
const String source = "#version 320 es\n"
"layout(binding = 6, rgba8) uniform highp image2D taken;\n"
@@ -408,7 +409,7 @@ TEST(SplitReadWriteImageUniformsTest, AliasNamesAvoidExistingIdentifiers) {
";\nvoid main()\n{\n"
" imageStore(taken, ivec2(0), imageLoad(taken, ivec2(0)) + " +
stageCollision + " + " + writeCollision + ");\n}\n";
const String out = SplitReadWriteImageUniforms(source, kStage);
const String out = SplitReadWriteImageUniforms(source);
EXPECT_FALSE(Contains(out, "image2D " + stageCollision + ";")) << out;
EXPECT_FALSE(Contains(out, "image2D " + writeCollision + ";")) << out;
@@ -435,7 +436,7 @@ void main()
// Declining means declining EVERYTHING: no qualifier, and no rename either. A rename that
// moved the declaration but not the use inside helper() would be a compile error rather than
// the wrong-but-compiling shader this pass refuses to guess at.
EXPECT_EQ(SplitReadWriteImageUniforms(source, kStage), source);
EXPECT_EQ(SplitReadWriteImageUniforms(source), source);
}
TEST(SplitReadWriteImageUniformsTest, ShaderWithoutImagesIsReturnedUnchanged) {
@@ -447,7 +448,7 @@ void main()
mg_FragColor = texture(goku, vec2(0.5));
}
)";
EXPECT_EQ(SplitReadWriteImageUniforms(source, kStage), source);
EXPECT_EQ(SplitReadWriteImageUniforms(source), source);
}
// The defect the rename exists for. The pass sees ONE stage at a time and picks the memory
@@ -455,9 +456,9 @@ void main()
// shader that only loads the same image came out `writeonly g_image` and `readonly g_image` -
// two declarations of one uniform name that GLSL requires to be identical. Adreno merges them
// and silently discards the vertex-stage stores (advanced-memory-dependentInvocation reads back
// the untouched zeros, with LINK_STATUS = 1 and an empty driver log). Stage-tagged names leave
// nothing to merge.
TEST(SplitReadWriteImageUniformsTest, TheSameImageGetsADifferentNameInEachStage) {
// the untouched zeros, with LINK_STATUS = 1 and an empty driver log). Tagging by the repair
// leaves nothing to merge.
TEST(SplitReadWriteImageUniformsTest, StagesThatUseAnImageDifferentlyGetDifferentNames) {
const String vertexSource = R"(#version 320 es
layout(binding = 0, rgba32f) uniform coherent highp image2D g_image;
void main()
@@ -474,11 +475,11 @@ void main()
mg_FragColor = imageLoad(g_image, ivec2(0));
}
)";
const String vsOut = SplitReadWriteImageUniforms(vertexSource, GL_VERTEX_SHADER);
const String fsOut = SplitReadWriteImageUniforms(fragmentSource, GL_FRAGMENT_SHADER);
const String vsOut = SplitReadWriteImageUniforms(vertexSource);
const String fsOut = SplitReadWriteImageUniforms(fragmentSource);
const String vsName = ImageStageAliasPrefix(GL_VERTEX_SHADER) + "g_image";
const String fsName = ImageStageAliasPrefix(GL_FRAGMENT_SHADER) + "g_image";
const String vsName = WoAlias("g_image");
const String fsName = RoAlias("g_image");
EXPECT_NE(vsName, fsName);
EXPECT_TRUE(Contains(vsOut, "uniform writeonly coherent highp image2D " + vsName + ";")) << vsOut;
EXPECT_TRUE(Contains(fsOut, "uniform readonly coherent highp image2D " + fsName + ";")) << fsOut;
@@ -493,21 +494,62 @@ void main()
EXPECT_TRUE(Contains(fsOut, "binding = 0"));
}
// Every stage gets a tag of its own, including the ones a fragment/vertex pair never exercises.
TEST(SplitReadWriteImageUniformsTest, EveryStageTagIsDistinct) {
const GLenum stages[] = {GL_VERTEX_SHADER, GL_FRAGMENT_SHADER, GL_COMPUTE_SHADER,
GL_GEOMETRY_SHADER, GL_TESS_CONTROL_SHADER, GL_TESS_EVALUATION_SHADER};
Vector<String> prefixes;
for (const GLenum stage : stages) {
const String prefix = ImageStageAliasPrefix(stage);
EXPECT_EQ(prefix.rfind(IMAGE_STAGE_ALIAS_PREFIX, 0), 0u) << prefix;
// The other side of that coin, and the one a per-STAGE tag got wrong. Two stages that use the
// image the same way emit byte-identical declarations, so they must arrive at ONE shared name:
// Adreno allocates an image LOCATION per distinct uniform, and giving each stage its own name
// multiplied a program's image-uniform count by the number of stages that mention it - which is
// how the five stages of KHR-GL43.shading_language_420pack.binding_images_texture_type_* went
// from 6 image uniforms to 30 and drew "Error: Image Image location or component exceeds max
// allowed." out of the Adreno 830 linker, with LINK_STATUS = TRUE already published by the
// frontend and every draw silently doing nothing.
TEST(SplitReadWriteImageUniformsTest, StagesThatUseAnImageAlikeShareOneName) {
const String vertexSource = R"(#version 320 es
layout(binding = 1, rgba8) uniform highp image2D goku;
void main()
{
imageStore(goku, ivec2(0), imageLoad(goku, ivec2(0)));
gl_Position = vec4(0.0);
}
)";
const String fragmentSource = R"(#version 320 es
layout(binding = 1, rgba8) uniform highp image2D goku;
layout(location = 0) out highp vec4 mg_FragColor;
void main()
{
imageStore(goku, ivec2(0), imageLoad(goku, ivec2(0)));
mg_FragColor = vec4(0.0);
}
)";
const String vsOut = SplitReadWriteImageUniforms(vertexSource);
const String fsOut = SplitReadWriteImageUniforms(fragmentSource);
// One name, arrived at independently by two different stages, so the linker merges them
// back into the single image uniform the application declared.
for (const String& out : {vsOut, fsOut}) {
EXPECT_TRUE(Contains(out, "uniform coherent readonly highp image2D " + RwAlias("goku") + ";")) << out;
EXPECT_TRUE(Contains(out, "uniform coherent writeonly highp image2D " + SplitWriteAlias("goku") + ";"))
<< out;
EXPECT_TRUE(Contains(out, "imageLoad(" + RwAlias("goku") + ",")) << out;
EXPECT_TRUE(Contains(out, "imageStore(" + SplitWriteAlias("goku") + ",")) << out;
}
}
// One tag per repair, all three distinct, and each a legal identifier stem.
TEST(SplitReadWriteImageUniformsTest, EveryAccessTagIsDistinct) {
const String prefixes[] = {String(IMAGE_READONLY_ALIAS_PREFIX), String(IMAGE_WRITEONLY_ALIAS_PREFIX),
String(IMAGE_SPLIT_READ_ALIAS_PREFIX), String(IMAGE_WRITE_ALIAS_PREFIX)};
Vector<String> seenPrefixes;
for (const String& prefix : prefixes) {
// A GLSL identifier may not contain "__" (GLSL ES 3.20 3.7), and the prefix is glued
// straight onto a name that may itself start with '_'.
EXPECT_EQ(prefix.find("__"), String::npos) << prefix;
for (const String& seen : prefixes) {
for (const String& seen : seenPrefixes) {
EXPECT_NE(seen, prefix) << prefix;
// Nor may one be a prefix of another: the write half is minted on top of an
// already-tagged name, so a shared stem would let two repairs collide.
EXPECT_NE(prefix.rfind(seen, 0), 0u) << prefix << " vs " << seen;
}
prefixes.push_back(prefix);
seenPrefixes.push_back(prefix);
}
}
@@ -690,17 +732,17 @@ void main()
}
)";
String out = RemapImageArrayElementUnits(source, {Plan("g_image", {4, 6})});
out = SplitReadWriteImageUniforms(out, kStage);
out = SplitReadWriteImageUniforms(out);
out = RemoveLayoutBinding(out);
// Element 0 is only ever loaded and element 1 only ever stored, so neither is split into a
// pair - but each keeps the unit the application gave it, which the array could not express.
EXPECT_TRUE(Contains(out, "binding = 4")) << out;
EXPECT_TRUE(Contains(out, "binding = 6")) << out;
EXPECT_TRUE(Contains(out, "readonly highp image2D " + StageAlias(Elem("g_image", 0)) + ";")) << out;
EXPECT_TRUE(Contains(out, "writeonly highp image2D " + StageAlias(Elem("g_image", 1)) + ";")) << out;
EXPECT_TRUE(Contains(out, "imageStore(" + StageAlias(Elem("g_image", 1)) + ", ivec2(0), imageLoad(" +
StageAlias(Elem("g_image", 0)) + ", ivec2(0)))"))
EXPECT_TRUE(Contains(out, "readonly highp image2D " + RoAlias(Elem("g_image", 0)) + ";")) << out;
EXPECT_TRUE(Contains(out, "writeonly highp image2D " + WoAlias(Elem("g_image", 1)) + ";")) << out;
EXPECT_TRUE(Contains(out, "imageStore(" + WoAlias(Elem("g_image", 1)) + ", ivec2(0), imageLoad(" +
RoAlias(Elem("g_image", 0)) + ", ivec2(0)))"))
<< out;
// Nothing is left addressing the array.
EXPECT_FALSE(Contains(out, "g_image[")) << out;
@@ -718,17 +760,54 @@ void main()
}
)";
String out = RemapImageArrayElementUnits(source, {Plan("g_image", {4, 9})});
out = SplitReadWriteImageUniforms(out, kStage);
out = SplitReadWriteImageUniforms(out);
out = RemoveLayoutBinding(out);
// Element 1 sits on unit 9, and both halves of its split pair say so.
EXPECT_EQ(CountOf(out, "binding = 9"), 2u) << out;
EXPECT_TRUE(Contains(out, "readonly highp image2D " + StageAlias(Elem("g_image", 1)) + ";")) << out;
EXPECT_TRUE(Contains(out, "writeonly highp image2D " + WriteAlias(StageAlias(Elem("g_image", 1))) + ";"))
EXPECT_TRUE(Contains(out, "readonly highp image2D " + RwAlias(Elem("g_image", 1)) + ";")) << out;
EXPECT_TRUE(Contains(out, "writeonly highp image2D " + WriteAlias(RwAlias(Elem("g_image", 1))) + ";"))
<< out;
EXPECT_EQ(CountOf(out, "binding = 4"), 1u) << out;
}
// The gap that let a per-STAGE image rename reach production: every fixture above declares an
// image ARRAY, and the regression it caused was in the SCALAR images sitting next to one. A
// scalar with an explicit binding has to come out of the whole chain still on ITS OWN unit,
// still spelled once, and named the same thing every stage would name it - it is the array that
// needs repairing, not its neighbour.
TEST(RemapImageArrayElementUnitsTest, AScalarImageWithItsOwnBindingIsUntouchedByTheArrayRepair) {
const String source = R"(#version 320 es
layout(rgba8, binding = 7) uniform highp image2D goku;
layout(rgba32f, binding = 4) uniform highp image2D g_image[2];
void main()
{
imageStore(g_image[1], ivec2(0), imageLoad(g_image[0], ivec2(0)));
imageStore(goku, ivec2(0), imageLoad(goku, ivec2(0)));
}
)";
Vector<String> declined;
String out = RemapImageArrayElementUnits(source, {Plan("g_image", {4, 9})}, &declined);
EXPECT_TRUE(declined.empty());
// The array pass may only ever touch the arrays it was handed a plan for.
EXPECT_TRUE(Contains(out, "layout(rgba8, binding = 7) uniform highp image2D goku;")) << out;
out = SplitReadWriteImageUniforms(out);
out = RemoveLayoutBinding(out);
// Unit 7 exactly twice - the two halves of the scalar's own split pair - and nothing has
// moved it onto one of the array's units.
EXPECT_EQ(CountOf(out, "binding = 7"), 2u) << out;
EXPECT_TRUE(Contains(out, "readonly highp image2D " + RwAlias("goku") + ";")) << out;
EXPECT_TRUE(Contains(out, "writeonly highp image2D " + SplitWriteAlias("goku") + ";")) << out;
// ...and no per-stage tag anywhere: the name a scalar gets is a function of how this text
// uses it, so every stage that uses it the same way keeps ONE shared uniform (Adreno spends
// an image location per distinct one).
EXPECT_FALSE(Contains(out, "mg_imageVs_")) << out;
EXPECT_FALSE(Contains(out, "mg_imageFs_")) << out;
EXPECT_FALSE(Contains(out, "mg_imageCs_")) << out;
}
// ---------------------------------------------------------------------------------------
// RetargetTextureBufferExtension
//
@@ -971,7 +1050,7 @@ layout(binding = 3) uniform writeonly highp uimage2D uni_image;
void main() { imageStore(uni_image, ivec2(0), uvec4(1u)); }
)";
String out = BakeImageFormatQualifiers(source, {{"uni_image", "r8ui"}});
out = SplitReadWriteImageUniforms(out, kStage);
out = SplitReadWriteImageUniforms(out);
out = RemoveLayoutBinding(out);
EXPECT_TRUE(Contains(out, "r8ui")) << out;
EXPECT_TRUE(Contains(out, "binding = 3")) << out;