From 9f60aadc1db58d24886539fa9d503cb399438329 Mon Sep 17 00:00:00 2001 From: rereview Date: Wed, 9 Sep 2026 00:09:30 -0400 Subject: [PATCH] [Fix, Test] (clientsp, MG_Pipe, Espryt): produce kMGPipeBindSampler and kMGPipeBindShaderImage where D-A4 places them - nothing set either bit, so ImageBindableHint was always 0, the metadata respecify had no live trigger and the remint pull the hint prevents was neither prevented nor counted (final review M-A); the sampler-view resolution notes SAMPLER, glBindImageTexture's state setter notes SHADER_IMAGE at the bind (so the hint precedes the first sync) and the image walk notes it too, both through a contract door since neither may include TextureEmit.h, and Espryt counts every re-mint of storage it already held as tex-remint-pulls (trp= on the stats line, ROADMAP open question 2's number) --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 12 ++ MobileGL/MG_Impl/Pipe/ImageEmit.h | 8 + MobileGL/MG_Impl/Pipe/PipeFill.cpp | 15 ++ MobileGL/MG_Impl/Pipe/SamplerEmit.h | 5 + MobileGL/MG_Impl/Pipe/TextureEmit.h | 7 +- .../Harness/P4aFinalFixPeek.cpp | 19 ++ .../Harness/P4aFinalFixPeek.h | 9 + .../Scenarios/P4aFinalFixScenario.cpp | 167 ++++++++++++++++++ MobileGL/MG_Pipe/PipeMutation.h | 23 +++ .../GLState/TextureState/TextureState.h | 9 + MobileGL/MG_Test/Pipe/ImageEmitTest.cpp | 41 ++++- MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp | 42 ++++- MobileGL/MG_Util/Metrics/PipeStats.cpp | 6 +- MobileGL/MG_Util/Metrics/PipeStats.h | 8 + 14 files changed, 366 insertions(+), 5 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 6d9c27b3..ca8b9ce1 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -5252,6 +5252,13 @@ namespace MobileGL::MG_Backend::DirectGLES { if (m_imageBindableStorageRequired) { return; } +#if MOBILEGL_PIPE_PUSH + // Whether this transition re-mints storage that ALREADY EXISTED on the backend: that + // is the remint PULL (the levels below are replayed from the client's shadow to fill + // the new carrier), and it is what ROADMAP open question 2 counts. A texture reaching + // here uninitialised is allocated image-bindable up front and pulls nothing. + const Bool hadBackendStorage = m_isInitialized; +#endif m_imageBindableStorageRequired = true; m_isInitialized = false; // Every level this object has ALREADY uploaded has to be replayed, because the @@ -5326,6 +5333,11 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!markedRemintPull) { MG_Pipe::MGPipeUnmigratedEmulation("texture-remint-pull"); markedRemintPull = true; + // THE COUNTER BEHIND ROADMAP OPEN QUESTION 2 (final review M-A): one per + // transition that replays a level of storage the backend already held. + if (hadBackendStorage && MG_Util::PipeStats::Enabled()) { + MG_Util::PipeStats::AddCalls(MG_Util::PipeStats::CallClass::TextureRemintPulls, 1); + } } if (!MG_Pipe::MGPipeHandleIsNull(rearmRes)) { const MG_Pipe::MGPBox wholeLevel{0, diff --git a/MobileGL/MG_Impl/Pipe/ImageEmit.h b/MobileGL/MG_Impl/Pipe/ImageEmit.h index 8c054b90..3b1b2d7b 100644 --- a/MobileGL/MG_Impl/Pipe/ImageEmit.h +++ b/MobileGL/MG_Impl/Pipe/ImageEmit.h @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -97,6 +98,13 @@ namespace MobileGL::MG_Pipe { entry.Res = binding.Texture ? MGPipeSlots().Acquire(MGPipeKind::Texture, binding.Texture->GetLifetimeId()) : kMGPipeNullHandle; + // D-A4: a texture named in an emitted MGPImageView is SHADER-IMAGE-bound from + // then on - the bit ImageBindableHint is derived from. The bind itself noted it + // first (TextureState.h, so the hint precedes the first sync); this is the + // letter of the rule and a one-compare early-out once the bit is set. + if (!MGPipeHandleIsNull(entry.Res)) { + MGPipeNoteTextureBoundAs(entry.Res, static_cast(kMGPipeBindShaderImage)); + } // THE APPLICATION's format and access, verbatim. The bind-format recast and the // buffer-texture split view are server-side and stay there; so does // SupportsLayeredImageBinding's rule, which asks the BACKEND target after diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index caa9b01d..38317380 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -1290,6 +1290,21 @@ namespace MobileGL::MG_Pipe { [&](auto& emitter) { emitter.EmitRenderbufferRespecify(renderbuffer); }); } + void MGPipeNoteTextureBoundAs(MGPipeHandle texture, Uint32 bindBit) { + // Not gated on FamilyIsLive: the mask is client state (see the declaration), and the + // emitter gates the emission it causes. + ForwardWhenWired( + MGPipeTextureEmitterInstance(), + [&](auto& emitter) { emitter.NoteTextureBoundAs(texture, static_cast(bindBit)); }); + } + + void MGPipeNoteTextureImageBound(ITextureObject& texture) { + ForwardWhenWired(MGPipeTextureEmitterInstance(), [&](auto& emitter) { + emitter.NoteTextureBoundAs(emitter.AcquireTexture(texture.GetLifetimeId(), &texture), + static_cast(kMGPipeBindShaderImage)); + }); + } + void MGPipeEmitSamplerCsoCreate(SamplerObject& sampler) { if (!FamilyIsLive(kMGPipeSubsystemSamplers, kMGPipeWiredSamplerSubsystem)) return; ForwardWhenWired( diff --git a/MobileGL/MG_Impl/Pipe/SamplerEmit.h b/MobileGL/MG_Impl/Pipe/SamplerEmit.h index 24fa07e7..3870ebc0 100644 --- a/MobileGL/MG_Impl/Pipe/SamplerEmit.h +++ b/MobileGL/MG_Impl/Pipe/SamplerEmit.h @@ -767,6 +767,11 @@ namespace MobileGL::MG_Pipe { if (MG_State::GLState::SamplesAsIncompleteTexture(texture.get(), effective)) continue; entry.Texture = MGPipeSlots().Acquire(MGPipeKind::Texture, texture->GetLifetimeId()); + // D-A4: a texture the sampler-view resolution names in an emitted MGPBoundView + // is SAMPLER-bound from then on (sticky; the texture emitter's contract door, + // since this header is included BY TextureEmit.h). One early-out per unit per + // pass once the bit is set. + MGPipeNoteTextureBoundAs(entry.Texture, static_cast(kMGPipeBindSampler)); entry.View = AcquireSamplerView(*texture, entry.Texture, bytes); } diff --git a/MobileGL/MG_Impl/Pipe/TextureEmit.h b/MobileGL/MG_Impl/Pipe/TextureEmit.h index 762dd85e..5f27e06f 100644 --- a/MobileGL/MG_Impl/Pipe/TextureEmit.h +++ b/MobileGL/MG_Impl/Pipe/TextureEmit.h @@ -519,8 +519,11 @@ namespace MobileGL::MG_Pipe { // ORed, never cleared, and emitted on BOTH resource_create and every // resource_respecify, exactly as P3a's buffer mask is. The four bits nothing set before // P4a get their producers here and in the framebuffer emitter: RENDER_TARGET and - // DEPTH_STENCIL from an attachment point, SAMPLER from a resolved sampler view and - // SHADER_IMAGE from a resolved image unit (the sampler package's two). + // DEPTH_STENCIL from an attachment point (FramebufferEmit.h), SAMPLER from a resolved + // sampler view (SamplerEmit.h) and SHADER_IMAGE from glBindImageTexture's state setter + // and the resolved image unit (TextureState.h, ImageEmit.h) - the last two through the + // contract's MGPipeNoteTextureBoundAs door, since neither may include this header + // (final review M-A: before the fix round nothing produced them and the hint was dead). // A MASK CHANGE AFTER THE ALLOCATION IS A METADATA RESPECIFY (ID-18 M4), and without it // the sticky half of D-A4 is a no-op for exactly the textures it was written for. The // mask rides resource_create and every resource_respecify - and an IMMUTABLE texture has diff --git a/MobileGL/MG_IntegrationTest/Harness/P4aFinalFixPeek.cpp b/MobileGL/MG_IntegrationTest/Harness/P4aFinalFixPeek.cpp index c4f9cdca..23c8a4ce 100644 --- a/MobileGL/MG_IntegrationTest/Harness/P4aFinalFixPeek.cpp +++ b/MobileGL/MG_IntegrationTest/Harness/P4aFinalFixPeek.cpp @@ -13,6 +13,7 @@ #if MOBILEGL_PIPE_PUSH #include #include +#include #define MGITEST_P4A_FINALFIX_PEEK_LIVE 1 #endif #endif @@ -45,8 +46,26 @@ namespace MGITest { } return false; } + + bool PeekPipeStatsTextureRemintPulls(unsigned long long* out) { + if (out == nullptr) return false; + namespace Stats = MobileGL::MG_Util::PipeStats; + if (!Stats::Enabled()) Stats::SetEnabledForTesting(true); + *out = static_cast(Stats::TotalCalls(Stats::CallClass::TextureRemintPulls)); + return true; + } + + bool PeekPipeStatsTextureUploadEmissions(unsigned long long* out) { + if (out == nullptr) return false; + namespace Stats = MobileGL::MG_Util::PipeStats; + if (!Stats::Enabled()) Stats::SetEnabledForTesting(true); + *out = static_cast(Stats::TotalCalls(Stats::CallClass::TextureUploadEmissions)); + return true; + } #else bool PeekPipeTextureResourceRecord(unsigned, PipeTextureResourceRecordPeek*) { return false; } + bool PeekPipeStatsTextureRemintPulls(unsigned long long*) { return false; } + bool PeekPipeStatsTextureUploadEmissions(unsigned long long*) { return false; } #endif } // namespace MGITest diff --git a/MobileGL/MG_IntegrationTest/Harness/P4aFinalFixPeek.h b/MobileGL/MG_IntegrationTest/Harness/P4aFinalFixPeek.h index e7cb92a4..65646d3c 100644 --- a/MobileGL/MG_IntegrationTest/Harness/P4aFinalFixPeek.h +++ b/MobileGL/MG_IntegrationTest/Harness/P4aFinalFixPeek.h @@ -29,4 +29,13 @@ namespace MGITest { }; bool PeekPipeTextureResourceRecord(unsigned glTextureName, PipeTextureResourceRecordPeek* out); + // The process-wide texture-remint pull count (PipeStats "tex-remint-pulls", `trp=` on the + // summary line; ROADMAP open question 2). Arms the PipeStats counters for this process on + // the first call, which is what lets a case read the number without a stats-enabled lane. + bool PeekPipeStatsTextureRemintPulls(unsigned long long* out); + // Espryt's count of texture uploads it actually issued (PipeStats "tex-upload-emissions"): + // what tells a CONSUMED pending upload apart from a DROPPED one, since the record's set is + // empty either way. Arms the counters the same way. + bool PeekPipeStatsTextureUploadEmissions(unsigned long long* out); + } // namespace MGITest diff --git a/MobileGL/MG_IntegrationTest/Scenarios/P4aFinalFixScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/P4aFinalFixScenario.cpp index f24eefca..b44ee6a7 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/P4aFinalFixScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/P4aFinalFixScenario.cpp @@ -487,5 +487,172 @@ void main() { oColor = texture(uTex, vUv); } glDeleteTextures(1, &cleanup); } + // ====================================================================================== + // M-A: an image bind after the allocation is a metadata respecify with the hint set + // ====================================================================================== + + // glTexStorage2D (immutable: no later respecify to ride), a red upload consumed by a draw, + // then a blue upload drained by a verb the texture is not reached by (accepted, standing + // in the applier's pending set), then glBindImageTexture. The bind must reach the record + // as a metadata update - ImageBindableHint 1, the pending upload still standing - and the + // draw after it must show the blue that upload carried through the widened carrier the + // hint schedules. + TEST_F(P4aFinalFixScenario, AnImageBindAfterAllocationReachesTheApplierAsAMetadataRespecify) { + if (!Ready()) return; + SkipUnlessEspryt("M-A's image-bindable hint"); + if (IsSkipped()) return; + GLint maxImageUnits = 0; + glGetIntegerv(GL_MAX_IMAGE_UNITS, &maxImageUnits); + while (glGetError() != GL_NO_ERROR) { + } + if (maxImageUnits < 1) { + GTEST_SKIP() << "no image units"; + return; + } + + // THE NUMBER ROADMAP OPEN QUESTION 2 ASKS FOR: a texture Espryt allocated BEFORE the + // hint reached it is re-minted image-bindable at the bind and its levels replayed + // from the client's shadow - one remint pull, counted. Arming the counter here is + // what makes it readable without a stats-enabled lane. + unsigned long long pullsBefore = 0; + const bool pullsReadable = PeekPipeStatsTextureRemintPulls(&pullsBefore); + + GLuint texture = 0; + glGenTextures(1, &texture); + glBindTexture(GL_TEXTURE_2D, texture); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 4, 4); + const std::vector red = Solid(4, 255, 0, 0); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 4, 4, GL_RGBA, GL_UNSIGNED_BYTE, red.data()); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glBindTexture(GL_TEXTURE_2D, 0); + const Image before = DrawSampled(texture); // allocated and consumed, NOT image-bindable + EXPECT_TRUE(Mostly(before, "red", "the immutable texture before the image bind")); + + PipeTextureResourceRecordPeek record{}; + const bool readable = RecordIsReadable(texture, "M-A's image-bindable hint", &record); + if (readable) { + EXPECT_EQ(record.ImageBindableHint, 0u) << "nothing has image-bound this texture yet"; + EXPECT_EQ(record.PendingUploads, 0u) << "the red upload was consumed by the draw"; + } + + // A blue upload, drained by a verb that does not reach T: accepted, unconsumed. + const std::vector blue = Solid(4, 0, 0, 255); + glBindTexture(GL_TEXTURE_2D, texture); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 4, 4, GL_RGBA, GL_UNSIGNED_BYTE, blue.data()); + glBindTexture(GL_TEXTURE_2D, 0); + const Image unrelated = DrawSampled(m_other); + EXPECT_TRUE(Mostly(unrelated, "white", "the unrelated draw")); + if (readable) { + ASSERT_TRUE(PeekPipeTextureResourceRecord(texture, &record)); + EXPECT_EQ(record.PendingUploads, 1u) << "the blue upload was not drained into the applier"; + } + const unsigned long long serialBeforeBind = record.Serial; + unsigned long long uploadsBeforeBind = 0; + const bool uploadsReadable = PeekPipeStatsTextureUploadEmissions(&uploadsBeforeBind); + + // THE TRANSITION. An immutable texture has no storage-defining respecify left, so the + // hint can only arrive as a metadata update (ID-18 M4). Espryt syncs the texture + // eagerly inside glBindImageTexture and the widening re-mints its storage, replaying + // every defined level from the shadow (the remint pull the counter below counts), so + // the standing upload is consumed by that regeneration here and the picture that + // follows is blue whatever the metadata respecify did to the record - the KEPT + // property is proved further down, on a texture no remint stands in front of. + (void)uploadsBeforeBind; + (void)uploadsReadable; + glBindImageTexture(0, texture, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + if (readable) { + ASSERT_TRUE(PeekPipeTextureResourceRecord(texture, &record)); + EXPECT_EQ(record.ImageBindableHint, 1u) + << "glBindImageTexture did not reach the applier's record as ImageBindableHint"; + EXPECT_NE(record.BindMask & (1u << 6), 0u) << "kMGPipeBindShaderImage was not produced"; + EXPECT_GT(record.Serial, serialBeforeBind) << "the metadata respecify moved no serial"; + } + + const Image image = DrawSampled(texture); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + Report("AnImageBindAfterAllocationReachesTheApplierAsAMetadataRespecify", image); + EXPECT_TRUE(Mostly(image, "blue", "the texture after the image bind that followed an unconsumed upload")); + glBindImageTexture(0, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + unsigned long long pullsAfter = 0; + if (pullsReadable && readable && PeekPipeStatsTextureRemintPulls(&pullsAfter)) { + EXPECT_EQ(pullsAfter, pullsBefore + 1) + << "the re-mint of a texture allocated before its hint was not counted as a remint pull " + "(trp= on the stats line is ROADMAP open question 2's number)"; + } + + // THE PREVENTION HALF, measured the other way round: a texture whose hint arrives at + // the bind, BEFORE its first sync, is allocated image-bindable up front and pulls + // nothing - the counter does not move. + GLuint early = 0; + glGenTextures(1, &early); + glBindTexture(GL_TEXTURE_2D, early); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 4, 4); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 4, 4, GL_RGBA, GL_UNSIGNED_BYTE, red.data()); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glBindTexture(GL_TEXTURE_2D, 0); + glBindImageTexture(0, early, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); // before any sync + const Image earlyImage = DrawSampled(early); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_TRUE(Mostly(earlyImage, "red", "a texture image-bound before its first sync")); + glBindImageTexture(0, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + unsigned long long pullsEarly = 0; + if (pullsReadable && readable && PeekPipeStatsTextureRemintPulls(&pullsEarly)) { + EXPECT_EQ(pullsEarly, pullsAfter) + << "a texture whose hint preceded its first sync was still re-minted (the prevention " + "half of the hint did not fire)"; + } + + // THE METADATA RESPECIFY KEEPS A STANDING UPLOAD, end to end and with no remint in the + // way: `early` is image-bindable already, so a NEW sticky bit reaching it - the + // RENDER_TARGET bit a DSA attachment produces at its setter (a Named record, ID-19(c)), + // with no sync of the texture in between - is a pure metadata update. The blue upload + // drained before it must still stand in the record afterwards (or, if a sync did run, + // have been uploaded rather than dropped) and reach the driver at the next draw. + glBindTexture(GL_TEXTURE_2D, early); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 4, 4, GL_RGBA, GL_UNSIGNED_BYTE, blue.data()); + glBindTexture(GL_TEXTURE_2D, 0); + const Image unrelatedAgain = DrawSampled(m_other); + EXPECT_TRUE(Mostly(unrelatedAgain, "white", "the unrelated draw")); + PipeTextureResourceRecordPeek earlyRecord{}; + const bool earlyReadable = PeekPipeTextureResourceRecord(early, &earlyRecord); + if (earlyReadable) { + EXPECT_EQ(earlyRecord.PendingUploads, 1u) << "the blue upload was not drained into the applier"; + } + const unsigned long long earlySerialBefore = earlyRecord.Serial; + unsigned long long uploadsBeforeAttach = 0; + const bool uploadsCounted = PeekPipeStatsTextureUploadEmissions(&uploadsBeforeAttach); + GLuint namedFbo = 0; + glCreateFramebuffers(1, &namedFbo); + glNamedFramebufferTexture(namedFbo, GL_COLOR_ATTACHMENT0, early, 0); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + if (earlyReadable) { + ASSERT_TRUE(PeekPipeTextureResourceRecord(early, &earlyRecord)); + EXPECT_NE(earlyRecord.BindMask & (1u << 7), 0u) + << "the DSA attachment did not produce kMGPipeBindRenderTarget"; + EXPECT_GT(earlyRecord.Serial, earlySerialBefore) << "the mask move reached the record as no respecify"; + unsigned long long uploadsAfterAttach = 0; + if (earlyRecord.PendingUploads == 0 && uploadsCounted && + PeekPipeStatsTextureUploadEmissions(&uploadsAfterAttach)) { + EXPECT_GT(uploadsAfterAttach, uploadsBeforeAttach) + << "the standing upload vanished from the record without Espryt uploading anything: " + "the metadata respecify dropped it"; + } else { + EXPECT_EQ(earlyRecord.PendingUploads, 1u) + << "the metadata respecify dropped the pending upload standing beside it"; + } + } + const Image earlyAfter = DrawSampled(early); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_TRUE(Mostly(earlyAfter, "blue", "the upload that stood across a metadata respecify")); + glDeleteFramebuffers(1, &namedFbo); + GLuint cleanup = texture; + glDeleteTextures(1, &cleanup); + GLuint cleanupEarly = early; + glDeleteTextures(1, &cleanupEarly); + } + } // namespace } // namespace MGITest diff --git a/MobileGL/MG_Pipe/PipeMutation.h b/MobileGL/MG_Pipe/PipeMutation.h index 750c8c2d..cccaebf8 100644 --- a/MobileGL/MG_Pipe/PipeMutation.h +++ b/MobileGL/MG_Pipe/PipeMutation.h @@ -361,6 +361,29 @@ namespace MobileGL::MG_Pipe { void MGPipeEmitRenderbufferResourceCreate(MG_State::GLState::RenderbufferObject& renderbuffer); void MGPipeEmitRenderbufferResourceRespecify(MG_State::GLState::RenderbufferObject& renderbuffer); + // ---- D-A4's two sticky bind-mask producers (P4a final review M-A) ---- + // + // kMGPipeBindSampler is "any texture the sampler-view resolution names in an emitted + // MGPBoundView" and kMGPipeBindShaderImage "any texture named in an emitted MGPImageView" + // - both the SAMPLER package's emitters (SamplerEmit.h, ImageEmit.h), which the texture + // emitter's header includes and which therefore cannot include it back - and, earliest of + // all, glBindImageTexture's state setter (TextureState.h, MG_State), which may include no + // emit header at all. So the note goes through this door, exactly as the birth hooks do. + // Nothing produced either bit before the fix round: ImageBindableHint was always 0, the + // metadata respecify (ID-18 M4) had no live trigger, and the remint pull the hint exists to + // prevent was neither prevented nor counted. + // + // UNCONDITIONAL IN A PUSH BUILD, like the mints: the mask is CLIENT state the framebuffer + // emitter ORs into whether or not the texture family is on, and the emission a mask move + // causes (the metadata respecify) is gated inside the emitter on the family's own pair. + void MGPipeNoteTextureBoundAs(MGPipeHandle texture, Uint32 bindBit); + // glBindImageTexture. The hint is the PREVENTION half of the texture-remint stall class - + // a texture the server knows may be image-bound is allocated image-bindable up front - so it + // has to reach the applier before the texture's first sync, i.e. at the bind itself, not at + // the validate point's image walk (which notes it as well, D-A4's letter). + void MGPipeNoteTextureImageBound(MG_State::GLState::ITextureObject& texture); + + // ---- sampler CSOs and sampler views: MG_Impl/Pipe/SamplerEmit.h, package C ---- // // Entry points MGPipeSamplerEmitter must provide, returning void: diff --git a/MobileGL/MG_State/GLState/TextureState/TextureState.h b/MobileGL/MG_State/GLState/TextureState/TextureState.h index 4186b6d3..0e416a2d 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureState.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureState.h @@ -33,6 +33,15 @@ namespace MobileGL::MG_State::GLState { Access = access; Format = format; ++Version; +#if MOBILEGL_PIPE_PUSH + // P4a D-A4 / final review M-A: the EARLIEST producer of kMGPipeBindShaderImage. The + // ImageBindableHint the bit feeds is the prevention half of the texture-remint stall + // class (a texture the server knows may be image-bound is allocated image-bindable + // up front), so it has to reach the applier before the texture's first sync - at + // the bind, not at the next validate point's image walk. Push-only through the + // contract's door, like every other hook in this directory (G1). + if (Texture) MG_Pipe::MGPipeNoteTextureImageBound(*Texture); +#endif } }; diff --git a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp index 536f6879..524b3232 100644 --- a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -347,7 +348,8 @@ TEST(ImageEmit, AMakeCurrentClearsTheImageSetAndAdvancesItsSerial) { X(ImageEmit, AZeroHighWaterMarkEmitsNothingWithoutHashing) \ X(ImageEmit, AnAccessModeChangeAloneStillEmitsTheSet) \ X(ImageEmit, AnInternalFormatChangeAloneStillEmitsTheSet) \ - X(ImageEmit, TheApplicationsFormatAndAccessTravelUnrecast) + X(ImageEmit, TheApplicationsFormatAndAccessTravelUnrecast) \ + X(ImageEmit, AnImageBoundTextureIsMarkedShaderImageBoundAtTheBind) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -516,6 +518,43 @@ void main() { imageStore(img, ivec2(0), vec4(1.0)); } EXPECT_EQ(Emitter().LastImageViews()[1].InternalFormat, static_cast(GL_RGBA8UI)); GL::UseProgram(0); } + + // FINAL REVIEW M-A: glBindImageTexture IS THE EARLIEST PRODUCER OF kMGPipeBindShaderImage - + // the bit the ImageBindableHint is derived from - and the emitted image set's walk is D-A4's + // (any texture named in an emitted MGPImageView). The hint is the PREVENTION half of the + // texture-remint stall class: a texture the server knows may be image-bound is allocated + // image-bindable up front, so it has to arrive before the first sync, i.e. at the bind. + // Nothing produced the bit before the fix round. + TEST(ImageEmit, AnImageBoundTextureIsMarkedShaderImageBoundAtTheBind) { + EmitterScope scope; + MGPipeTextureEmitterInstance().ResetForTest(); + const GLuint name = MakeImageTexture(); + const auto& texture = Ctx().GetTextureObject(name); + ASSERT_TRUE(texture); + const MGPipeHandle handle = MGPipeSlots().FindByLifetimeId(MGPipeKind::Texture, texture->GetLifetimeId()); + ASSERT_FALSE(MGPipeHandleIsNull(handle)); + EXPECT_EQ(MGPipeTextureEmitterInstance().TextureBindMask(handle) & kMGPipeBindShaderImage, 0) + << "nothing has image-bound this texture yet"; + + GL::BindImageTexture(0, name, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + EXPECT_NE(MGPipeTextureEmitterInstance().TextureBindMask(handle) & kMGPipeBindShaderImage, 0) + << "glBindImageTexture did not mark the texture image-bound"; + + static const char* kOneImage = R"(#version 430 core +layout(local_size_x = 1) in; +layout(binding = 0, rgba8) uniform image2D img; +void main() { imageStore(img, ivec2(0, 0), vec4(1.0)); } +)"; + const GLuint program = MakeComputeProgram(kOneImage); + GL::UseProgram(program); + Emitter().EmitShaderImages(Ctx()); + ASSERT_GE(Emitter().Window(), 1u); + EXPECT_TRUE(Emitter().LastImageViews()[0].Res == handle); + EXPECT_NE(MGPipeTextureEmitterInstance().TextureBindMask(handle) & kMGPipeBindShaderImage, 0) + << "the emitted image set's walk does not carry the bit either"; + GL::UseProgram(0); + GL::BindImageTexture(0, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + } } // namespace #endif // MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp b/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp index 96143616..e720af3b 100644 --- a/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp @@ -58,6 +58,7 @@ #include "Init.h" #include #include +#include #include #include #include @@ -566,7 +567,8 @@ TEST(SamplerEmit, AMakeCurrentTakesTheUnitSetsAndLeavesTheCsoAndViewRecordsStand X(SamplerEmit, ABoundSamplerStateHoldsItsCsoUntilTheUnitMoves) \ X(SamplerEmit, AReferencedCsoIsNeverTheLruVictim) \ X(SamplerEmit, AFullyPinnedCacheMintsBeyondItsCapacityAndCountsIt) \ - X(SamplerEmit, AReleaseThisCacheNeverHandedOutIsCountedRatherThanAbsorbed) + X(SamplerEmit, AReleaseThisCacheNeverHandedOutIsCountedRatherThanAbsorbed) \ + X(SamplerEmit, AResolvedSamplerViewMarksItsTextureAsSamplerBound) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -1088,6 +1090,44 @@ namespace { // turned from a compiled-out assert into a number. EXPECT_EQ(Cache().GetCounters().ReferencedEvictions, 0u); } + + // FINAL REVIEW M-A: THE SAMPLER-VIEW RESOLUTION IS D-A4's PRODUCER OF kMGPipeBindSampler. + // "Any texture the sampler-view resolution names in an emitted MGPBoundView" carries the + // sticky bit from then on; a texture bound to a unit no sampler uniform resolves does not. + // Nothing produced the bit before the fix round. + TEST(SamplerEmit, AResolvedSamplerViewMarksItsTextureAsSamplerBound) { + EmitterScope scope; + MGPipeTextureEmitterInstance().ResetForTest(); + namespace GL = MobileGL::MG_Impl::GLImpl; + const Uint program = MakeSamplerProgram(); + GLint linked = 0; + GL::GetProgramiv(program, GL_LINK_STATUS, &linked); + ASSERT_EQ(linked, GL_TRUE); + GL::UseProgram(program); + const GLint location = GL::GetUniformLocation(program, "sampled"); + ASSERT_GE(location, 0); + GL::Uniform1i(location, 3); + + GLuint sampledName = 0; + GLuint unsampledName = 0; + const SharedPtr sampled = MakeCompleteTexture(sampledName, 4); + const SharedPtr unsampled = MakeCompleteTexture(unsampledName, 4); + BindTextureToUnit(3, sampled); + BindTextureToUnit(5, unsampled); + + ASSERT_GT(Emitter().EmitSamplerViews(Ctx()), 0u); + const MGPBoundView& resolved = Emitter().LastBoundViews()[3]; + ASSERT_FALSE(MGPipeHandleIsNull(resolved.Texture)); + EXPECT_NE(MGPipeTextureEmitterInstance().TextureBindMask(resolved.Texture) & kMGPipeBindSampler, 0) + << "the texture a sampler view was resolved for does not carry kMGPipeBindSampler"; + const MGPipeHandle unsampledHandle = + MGPipeSlots().FindByLifetimeId(MGPipeKind::Texture, unsampled->GetLifetimeId()); + if (!MGPipeHandleIsNull(unsampledHandle)) { + EXPECT_EQ(MGPipeTextureEmitterInstance().TextureBindMask(unsampledHandle) & kMGPipeBindSampler, 0) + << "a texture no sampler uniform resolves to was marked sampler-bound"; + } + GL::UseProgram(0); + } } // namespace #endif // MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_Util/Metrics/PipeStats.cpp b/MobileGL/MG_Util/Metrics/PipeStats.cpp index f79ec6fb..92847bfd 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.cpp +++ b/MobileGL/MG_Util/Metrics/PipeStats.cpp @@ -179,7 +179,7 @@ namespace MobileGL::MG_Util::PipeStats { #if MOBILEGL_PIPE_PUSH "render-state-cso-mints", "render-state-cso-binds", "map-persistent-roundtrips", "framebuffer-emissions", "sampler-view-emissions", "sampler-state-emissions", - "shader-image-emissions", "client-tex-upload-emissions", + "shader-image-emissions", "client-tex-upload-emissions", "tex-remint-pulls", #endif }; const char* const kGateNames[kGateCount] = { @@ -453,6 +453,10 @@ namespace MobileGL::MG_Util::PipeStats { line += " sie=" + std::to_string(calls[static_cast(CallClass::ShaderImageEmissions)]); line += " ctu=" + std::to_string(calls[static_cast(CallClass::ClientTextureUploadEmissions)]); + // trp is the texture-remint pull count (ROADMAP open question 2): every one is a texture + // Espryt had already allocated and then had to re-mint image-bindable, replaying its + // levels from the client's shadow, because ImageBindableHint reached it too late. + line += " trp=" + std::to_string(calls[static_cast(CallClass::TextureRemintPulls)]); #endif line += "] gates["; for (Uint32 i = 0; i < kGateCount; ++i) { diff --git a/MobileGL/MG_Util/Metrics/PipeStats.h b/MobileGL/MG_Util/Metrics/PipeStats.h index 936db16c..fae4c201 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.h +++ b/MobileGL/MG_Util/Metrics/PipeStats.h @@ -156,6 +156,14 @@ namespace MobileGL::MG_Util::PipeStats { // hides is ~+6 ms/frame, so an emission-shape divergence has to be a difference of two // numbers rather than something only a GPU can see. ClientTextureUploadEmissions, + // THE TEXTURE-REMINT PULL RATE (ROADMAP open question 2; P4a final review M-A). Counted + // by Espryt once per transition in which a texture that ALREADY HAD backend storage is + // re-minted image-bindable and its defined levels are replayed from the client's shadow + // (RequireImageBindableStorage) - the reach-back a split cannot make (D-M) and the one + // ImageBindableHint exists to prevent. A texture whose hint arrived before its first + // sync is allocated image-bindable up front and never counts. `trp=` on the summary + // line; the number that decides MOBILEGL_PIPE_TEXEL_RETAIN_MB's default. + TextureRemintPulls, #endif Count };