[MG_Backend] (Disaggregated): P5e ID-124 - the pull and push flavours compile again

Three sites, all pre-existing at 7c6f6886, all the same disease: something guarded by
MOBILEGL_BUILD_DISAGGREGATED used from code that is not. Nothing in the gate built either
flavour, so nobody knew.

- Managers.cpp: StagedTextureTargetForPipeTarget is a pure MGPipeResourceTarget -> TextureTarget
  switch with no wire, no record and no role in it, but it sat inside a nested
  #if MOBILEGL_BUILD_DISAGGREGATED while its only caller MGB_TEXPARAM_TARGET is guarded by
  #if MOBILEGL_PIPE_PUSH alone. Let out of the inner block without moving any text. The narrower
  fix - guarding the macro block too - is WRONG and was tried: that block also carries
  TextureDiagName and MGB_TEXTURE_RECORD_ARM_SELECTED, which the push flavour does need, and
  removing them broke it differently.
- DirectGLES.cpp:5575: the storage-block-signature clause called
  ComputeShaderStorageBlockBindingSignatureOf, declared only under MOBILEGL_PIPE_PUSH, from an
  UNGUARDED condition list. The clause is push-only in substance too - the signature it compares
  is over the override map the wire carries.
- DirectGLES.cpp BindCurrentTextures: keysMatch was declared inside
  #if MOBILEGL_BUILD_DISAGGREGATED while the ternary's else-arm and every use sat outside it.
  The legacy arm becomes a LAZY lambda, not an eager Bool: it reads currentProgram, which is
  null on the handle arm after pa, so the laziness the ternary gave for free is load-bearing
  (ID-81 / ID-110 - the arm decides, and the other arm's reads must not happen at all).

Gate: flavours rc=0 - pull 915 TUs (push define absent), push 929 TUs (push=1), both arms
asserted against compile_commands.json rather than against the flags, because the first version
of this step passed -DMOBILEGL_PIPE_PUSH=OFF beside an option that implies it and reported a
clean 982-TU "pull" build that was a second copy of the split build. The cache entry still read
OFF. Split build unaffected.
This commit is contained in:
2026-09-18 12:07:45 -04:00
parent d8b9848003
commit 3c13fa4b4a
2 changed files with 37 additions and 10 deletions
+28 -10
View File
@@ -5724,8 +5724,15 @@ namespace MobileGL::MG_Backend::DirectGLES {
twin->GetSnormFallbackClampOutputMask() != g_snormFallbackClampOutputMask ||
twin->GetUnormFallbackClampOutputMask() != g_unormFallbackClampOutputMask ||
twin->GetFragColorBroadcastCount() != g_fragColorBroadcastCount ||
#if MOBILEGL_PIPE_PUSH
// P5e (ID-124): ComputeShaderStorageBlockBindingSignatureOf is declared only
// under MOBILEGL_PIPE_PUSH (Managers.h:3122) and this clause read it from an
// UNGUARDED condition list, so the pull flavour did not compile. The clause is
// push-only in substance too: the signature it compares is over the override
// map the wire carries, and without the wire there is nothing to compare.
twin->GetShaderStorageBlockBindingSignature() !=
ComputeShaderStorageBlockBindingSignatureOf(*currentProgram) ||
#endif
// A fourth of the same shape, and the reason glBindImageTexture itself does
// nothing: GLSL ES demands a format layout qualifier on an image where desktop
// GLSL lets a writeonly declaration omit one, so a format-less declaration is
@@ -6936,16 +6943,15 @@ namespace MobileGL::MG_Backend::DirectGLES {
const SizeT shadowBytes =
static_cast<SizeT>(maxTouchedUnit + 1) * sizeof(TextureImpl::g_boundTexturesCache[0]);
const Uint64 unitBindingsEpoch = keys.unitBindingsEpoch;
#if MOBILEGL_BUILD_DISAGGREGATED
const Bool keysMatch =
byHandle ? (memo.valid && memo.byHandle && memo.maxTouchedUnit == maxTouchedUnit &&
memo.contextSerial == contextSerial && memo.samplerViewsSerial == samplerViewsSerial &&
memo.drawProgram.Slot == drawProgram.Slot && memo.drawProgram.Gen == drawProgram.Gen &&
memo.shaderCsoSerial == shaderCsoSerial && memo.bindingsSerial == bindingsSerial &&
memo.contextGeneration == g_backendContextGeneration)
:
#endif
(memo.valid && memo.glContextId == keys.contextId &&
// P5e (ID-124): the legacy key is a LAZY LAMBDA, not an eagerly-computed Bool. It was
// the else-arm of a ternary whose declaration sat inside `#if MOBILEGL_BUILD_DISAGGREGATED`
// while the arm itself and every use of `keysMatch` sat outside it, so neither
// non-disaggregated flavour compiled. Hoisting it to an eager Bool would have been
// WORSE than the build break: it reads `currentProgram`, which is null on the handle
// arm, so the laziness the ternary gave for free is load-bearing (ID-81 / ID-110 -
// the arm decides, and the other arm's reads must not happen at all).
const auto legacyKeysMatch = [&]() -> Bool {
return (memo.valid && memo.glContextId == keys.contextId &&
memo.maxTouchedUnit == maxTouchedUnit &&
memo.unitBindingsEpoch == unitBindingsEpoch &&
memo.samplingResolutionGeneration == keys.samplingGeneration &&
@@ -6955,6 +6961,18 @@ namespace MobileGL::MG_Backend::DirectGLES {
(currentProgram ? currentProgram->GetBackendStateVersion() : 0) &&
memo.programLinked == (currentProgram && currentProgram->GetLinkStatus()) &&
memo.contextGeneration == g_backendContextGeneration);
};
#if MOBILEGL_BUILD_DISAGGREGATED
const Bool keysMatch =
byHandle ? (memo.valid && memo.byHandle && memo.maxTouchedUnit == maxTouchedUnit &&
memo.contextSerial == contextSerial && memo.samplerViewsSerial == samplerViewsSerial &&
memo.drawProgram.Slot == drawProgram.Slot && memo.drawProgram.Gen == drawProgram.Gen &&
memo.shaderCsoSerial == shaderCsoSerial && memo.bindingsSerial == bindingsSerial &&
memo.contextGeneration == g_backendContextGeneration)
: legacyKeysMatch();
#else
const Bool keysMatch = legacyKeysMatch();
#endif
// Short-circuited: the shadow compare is only meaningful once the key (and with it the
// snapshotted row count) matches.
if (!keysMatch || std::memcmp(memo.boundTextures.data(), TextureImpl::g_boundTexturesCache.data(),
@@ -2671,6 +2671,14 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
}
// P5e (ID-124): OUT OF THE DISAGGREGATED BLOCK ON PURPOSE. This is a pure
// MGPipeResourceTarget -> TextureTarget switch with no wire, no record and no role in it, but
// it sat inside the nested `#if MOBILEGL_BUILD_DISAGGREGATED` below while its only caller,
// MGB_TEXPARAM_TARGET, is guarded by `#if MOBILEGL_PIPE_PUSH` alone - so a push build without
// disaggregation did not compile. The narrower fix (guarding the macro block too) is wrong:
// that block also carries TextureDiagName and MGB_TEXTURE_RECORD_ARM_SELECTED, which the push
// flavour does need. Caught by the `flavours` gate step; see ID-124.
#endif // MOBILEGL_BUILD_DISAGGREGATED
// The inverse of MG_Pipe::MGPipeResourceTargetForTextureTarget, for the sync's
// target reads (ConvertTextureTargetToBackendGLEnum and MapToBackendTextureTarget
// both want the frontend enum).
@@ -2690,6 +2698,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
default: return TextureTarget::Unknown;
}
}
#if MOBILEGL_BUILD_DISAGGREGATED
void Ops_H_TextureSubData(MG_Pipe::MGPipeHandle res, const MG_Pipe::MGPSubData& record,
const void* bytes, const MG_Pipe::MGPSubRegion* regions) {