From 815217cc07aa77e901fd05619f53e5c6bb9d2274 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 2 Oct 2026 20:57:18 -0400 Subject: [PATCH] [Fix] (Espryt): turn the scissor off around backend copies by what the driver has, not another session's render-state shadow --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 64 ++++++++++++--- MobileGL/MG_Backend/DirectGLES/DirectGLES.h | 15 ++++ .../Backend/DirectGLES/ContextEpochTest.cpp | 81 +++++++++++++++++++ 3 files changed, 148 insertions(+), 12 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index fdf1d2f6..f374b4e0 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -5239,19 +5239,49 @@ namespace MobileGL::MG_Backend::DirectGLES { // SyncRenderState runs on the GL thread only. static Array g_dualSourceDeclinedBlendStates; +#if MOBILEGL_BUILD_DISAGGREGATED + // The context epoch the shadow was last synced in (SyncRenderState). + static ContextEpoch g_syncedRenderStateEpoch{}; +#endif void InvalidateSyncedRenderState() { g_forceFullRenderStateResync = true; g_hasSyncedRenderState = false; g_syncedBackendViewport = IntVec4(-1, -1, -1, -1); g_syncedBackendScissorBox = IntVec4(-1, -1, -1, -1); } + + // Whether the shadow's bits are the current native context's. An invalidation (a context + // switch, the backend's turn moving to another session's thread) only raises flags - the + // bits stay, and they are then another context's. + static Bool SyncedRenderStateDescribesCurrentContext() { + if (!g_hasSyncedRenderState || g_forceFullRenderStateResync) return false; +#if MOBILEGL_BUILD_DISAGGREGATED + if (g_syncedRenderStateEpoch != CurrentContextEpoch()) return false; +#endif + return true; + } + + Bool DriverScissorTestEnabled() { + if (SyncedRenderStateDescribesCurrentContext()) + return (g_syncedRenderStateParameters.ScissorTestEnabledMask & 1u) != 0; + return g_GLESFuncs.glIsEnabled != nullptr && g_GLESFuncs.glIsEnabled(GL_SCISSOR_TEST) == GL_TRUE; + } + + void SetSyncedScissorTestForTesting(Bool enabled) { + g_syncedRenderStateParameters.ScissorTestEnabledMask = enabled ? 1u : 0u; + g_hasSyncedRenderState = true; + g_forceFullRenderStateResync = false; +#if MOBILEGL_BUILD_DISAGGREGATED + g_syncedRenderStateEpoch = CurrentContextEpoch(); +#endif + } + void SyncRenderState(Bool forColorClear) { #if MOBILEGL_BUILD_DISAGGREGATED - static ContextEpoch syncedEpoch{}; const auto epoch = CurrentContextEpoch(); - if (syncedEpoch != epoch) { + if (g_syncedRenderStateEpoch != epoch) { InvalidateSyncedRenderState(); - syncedEpoch = epoch; + g_syncedRenderStateEpoch = epoch; } #endif #ifdef TRACY_ENABLE @@ -10252,8 +10282,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // error. Disable for the staging blit only; the caller-visible blit below // keeps the blit's native scissor semantics. Tracked via the render-state // shadow, exactly like ScopedScissorDisable. - const Bool scissorWasEnabled = - (RenderStateImpl::g_syncedRenderStateParameters.ScissorTestEnabledMask & 1u) != 0; + const Bool scissorWasEnabled = RenderStateImpl::DriverScissorTestEnabled(); if (scissorWasEnabled) g_GLESFuncs.glDisable(GL_SCISSOR_TEST); g_GLESFuncs.glBlitFramebuffer(left, bottom, right, top, 0, 0, width, height, GL_COLOR_BUFFER_BIT, GL_NEAREST); @@ -10987,8 +11016,7 @@ namespace MobileGL::MG_Backend::DirectGLES { const Bool oneToOne = width > 0 && height > 0 && (dstX1 - dstX0) == width && (dstY1 - dstY0) == height; // The scissor clips a blit and does not clip a copy, so an enabled scissor makes the two // different operations no matter how the rectangles line up. - const Bool scissorEnabled = - (RenderStateImpl::g_syncedRenderStateParameters.ScissorTestEnabledMask & 1u) != 0; + const Bool scissorEnabled = RenderStateImpl::DriverScissorTestEnabled(); using MobileGL::FramebufferAttachmentType; struct AspectPlan { @@ -11169,8 +11197,7 @@ namespace MobileGL::MG_Backend::DirectGLES { const Bool oneToOne = width > 0 && height > 0 && (dstX1 - dstX0) == width && (dstY1 - dstY0) == height; // The scissor clips a blit and does not clip a copy, so an enabled scissor makes the two // different operations no matter how the rectangles line up. - const Bool scissorEnabled = - (RenderStateImpl::g_syncedRenderStateParameters.ScissorTestEnabledMask & 1u) != 0; + const Bool scissorEnabled = RenderStateImpl::DriverScissorTestEnabled(); using MobileGL::FramebufferAttachmentType; struct AspectPlan { @@ -11605,11 +11632,17 @@ namespace MobileGL::MG_Backend::DirectGLES { // Force-disables GL_SCISSOR_TEST for the scope (emulation blits and clears are // scissored; readback copies must not be clipped by app scissor state) and - // restores the app state on exit, tracked via the render-state shadow. + // restores the app state on exit. + // + // WHAT THE DRIVER HAS, NOT WHAT THE SHADOW LAST SAID. The render-state shadow is one per + // process and is synced at draws; a session that presents or reads back without drawing + // since another session's context synced (a frame that redraws nothing, a client whose + // turn came after the compositor's) found the other context's bit there. Its own scissor, + // still on from its last draw, then clipped the shared-image copy to a box - flipped into + // the wrong rows, too - and the image kept the frame it held before. class ScopedScissorDisable { public: - ScopedScissorDisable() - : m_wasEnabled((RenderStateImpl::g_syncedRenderStateParameters.ScissorTestEnabledMask & 1u) != 0) { + ScopedScissorDisable() : m_wasEnabled(RenderStateImpl::DriverScissorTestEnabled()) { if (m_wasEnabled) g_GLESFuncs.glDisable(GL_SCISSOR_TEST); } ~ScopedScissorDisable() { @@ -11622,6 +11655,13 @@ namespace MobileGL::MG_Backend::DirectGLES { const Bool m_wasEnabled; }; + namespace RenderStateImpl { + void RunScissorDisabledForTesting(void (*inside)(void*), void* data) { + ScopedScissorDisable scissorOff; + inside(data); + } + } // namespace RenderStateImpl + // Binds the shared scratch FBO at READ (isRead) or DRAW for one temp operation, // restoring the previous binding on exit. Attachments are managed through the // ScratchFBOImpl attachment shadow by the caller (see Framebuffer()). diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h index e78b14a9..d955bf39 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h @@ -495,6 +495,21 @@ namespace MobileGL::MG_Backend::DirectGLES { // answers from the frontend state, which this function only reads. void SyncRenderState(Bool forColorClear = false); void InvalidateSyncedRenderState(); + + // GL_SCISSOR_TEST as the driver has it in the native context current on this thread - + // what a guard that turns the test off around its own blit, and puts it back, needs to + // know. The shadow answers only while it describes this context (synced since the last + // invalidation, in this context epoch); otherwise the driver is asked. The shadow is one + // per process: once another session's context synced into it, or a context switch + // invalidated it, its bits are some other context's. + Bool DriverScissorTestEnabled(); + + // TESTING: the shadow as SyncRenderState leaves it in the current context epoch, with + // GL_SCISSOR_TEST `enabled`. + void SetSyncedScissorTestForTesting(Bool enabled); + // TESTING: `inside` runs within the guard the shared-image present and the readbacks + // copy under (ScopedScissorDisable). + void RunScissorDisabledForTesting(void (*inside)(void*), void* data); } // namespace RenderStateImpl #if MOBILEGL_BUILD_DISAGGREGATED diff --git a/MobileGL/MG_Test/Backend/DirectGLES/ContextEpochTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/ContextEpochTest.cpp index 5450ebe5..1343654d 100644 --- a/MobileGL/MG_Test/Backend/DirectGLES/ContextEpochTest.cpp +++ b/MobileGL/MG_Test/Backend/DirectGLES/ContextEpochTest.cpp @@ -250,3 +250,84 @@ TEST(ContextEpochTest, DualBlockApplierDiagnosticIsRoleLocalWhileSharedControlRe GTEST_SKIP() << "role-local observer requires the disaggregated build"; #endif } + +// THE SCISSOR GUARD AROUND A COPY THE BACKEND MAKES ON ITS OWN (the shared-image present, the +// readbacks). The render-state shadow is one per process and is only synced at draws, so after a +// context switch or another session's turn its bits are another context's. A client that presents +// without drawing since then (a frame that redraws nothing) kept its own scissor on through the +// copy - the image was clipped to the client's last box and kept the frame it held before. +#if MOBILEGL_BUILD_DISAGGREGATED +namespace { + Bool fakeScissor = false; + Uint isEnabledCalls = 0; + void GL_APIENTRY FakeEnable(GLenum cap) { if (cap == GL_SCISSOR_TEST) fakeScissor = true; } + void GL_APIENTRY FakeDisable(GLenum cap) { if (cap == GL_SCISSOR_TEST) fakeScissor = false; } + GLboolean GL_APIENTRY FakeIsEnabled(GLenum cap) { + ++isEnabledCalls; + return cap == GL_SCISSOR_TEST && fakeScissor ? GL_TRUE : GL_FALSE; + } + struct ScissorDriver : DriverScope { + ScissorDriver() { + g_GLESFuncs.glEnable = &FakeEnable; + g_GLESFuncs.glDisable = &FakeDisable; + g_GLESFuncs.glIsEnabled = &FakeIsEnabled; + isEnabledCalls = 0; + } + ~ScissorDriver() { RenderStateImpl::InvalidateSyncedRenderState(); } + }; + // The driver's scissor as the copy inside the guard sees it. + Bool ScissorDuringGuard() { + Bool seen = true; + RenderStateImpl::RunScissorDisabledForTesting([](void* out) { *static_cast(out) = fakeScissor; }, + &seen); + return seen; + } +} // namespace +#endif + +TEST(ContextEpochTest, ScissorGuardTurnsOffTheScissorTheShadowNoLongerDescribes) { +#if MOBILEGL_BUILD_DISAGGREGATED + ScissorDriver scope; + // Another context last synced the shadow with the test off; then the switch to this one. + RenderStateImpl::SetSyncedScissorTestForTesting(false); + RenderStateImpl::InvalidateSyncedRenderState(); + fakeScissor = true; // this context's application left its scissor on + EXPECT_FALSE(ScissorDuringGuard()) << "the copy ran clipped by the application's scissor box"; + EXPECT_TRUE(fakeScissor) << "the guard did not put the application's scissor back"; + + // The same through a context epoch moving without an explicit invalidation. + RenderStateImpl::SetSyncedScissorTestForTesting(false); + ++g_backendContextGeneration; + fakeScissor = true; + EXPECT_FALSE(ScissorDuringGuard()) << "a shadow synced in another context epoch was trusted"; + EXPECT_TRUE(fakeScissor); +#else + GTEST_SKIP() << "the per-session render-state shadow hazard is a multi-session server's"; +#endif +} + +TEST(ContextEpochTest, ScissorGuardLeavesAnOffScissorOffWhateverAStaleShadowSays) { +#if MOBILEGL_BUILD_DISAGGREGATED + ScissorDriver scope; + RenderStateImpl::SetSyncedScissorTestForTesting(true); + RenderStateImpl::InvalidateSyncedRenderState(); + fakeScissor = false; + EXPECT_FALSE(ScissorDuringGuard()); + EXPECT_FALSE(fakeScissor) << "the guard turned on a scissor this context never had"; +#else + GTEST_SKIP() << "the per-session render-state shadow hazard is a multi-session server's"; +#endif +} + +TEST(ContextEpochTest, ScissorGuardTakesACurrentShadowsWordWithoutAskingTheDriver) { +#if MOBILEGL_BUILD_DISAGGREGATED + ScissorDriver scope; + RenderStateImpl::SetSyncedScissorTestForTesting(true); + fakeScissor = true; + EXPECT_FALSE(ScissorDuringGuard()); + EXPECT_TRUE(fakeScissor); + EXPECT_EQ(isEnabledCalls, 0u) << "a shadow synced in this context should answer without a driver query"; +#else + GTEST_SKIP() << "the per-session render-state shadow hazard is a multi-session server's"; +#endif +}