diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 12332409..87b4734e 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -259,7 +259,7 @@ namespace MobileGL::MG_Backend::DirectGLES { } [[noreturn]] static void RefuseFramebufferBindingSlotRead() { MG_Pipe::MGPipeInputPoisonFatalForVerb(MG_Pipe::MGPipeInputField::GetFramebufferBindingSlot, - MGB_CTX->CurrentVerb()); + MG_Pipe::gPipeInputs.CurrentVerb()); } #endif // MOBILEGL_BUILD_DISAGGREGATED #endif // MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp index 7860fff4..1988b6a5 100644 --- a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp @@ -242,7 +242,9 @@ TEST(FramebufferEmit, TheEmitterIsOneNeverDestroyedProcessSingleton) { X(FramebufferEmit, ADrawBufferTokenAboveTheWireWidthIsRefusedNotTruncated) \ X(FramebufferEmit, ALayeredCubeAttachmentDoesNotAssertAFaceItCannotKnow) \ X(FramebufferEmit, EveryNonTexturePointCarriesTheUnknownSentinelsRatherThanZero) \ - X(FramebufferEmit, ADeadFramebuffersNamedRecordLatchIsRetired) + X(FramebufferEmit, ADeadFramebuffersNamedRecordLatchIsRetired) \ + X(FramebufferEmit, TheRecordsElevenSurfacesAreTheWholePointSetTheServerReads) \ + X(FramebufferEmit, ADefaultFramebufferResizeReEmitsTheRecordWithItsNewExtent) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -579,6 +581,130 @@ TEST(FramebufferEmit, AnAttachmentPointAboveTheWireWidthIsRefusedNotTruncated) { EXPECT_GE(Framebuffers().RefusedCount(), 1u) << "the refusal was not counted"; } +// ============================ P5e (fb), CONTRACT-P5E.md §5.4 ============================ +// +// THE SERVER TWIN of AnAttachmentPointAboveTheWireWidthIsRefusedNotTruncated above. +// +// That case pins the CLIENT half: a framebuffer holding a point at or above the wire width is +// refused rather than truncated. P5e turns that refusal into a licence the server SPENDS - the +// by-handle attachment walk stopped iterating the frontend's 41 points and now walks +// Color[0..7] + Depth + Stencil and nothing else, on the argument that the record's eleven +// surfaces ARE the point set. So the statement the server relies on needs a case of its own, +// and it has two halves: +// +// 1. a framebuffer that fills every point the wire can describe round-trips ALL of them into +// the applier's record - so "eleven" is not eleven minus whatever the emitter dropped; +// 2. when a point above the width appears, the record the server is holding does not change +// at all. The refusal is not merely counted: the server keeps describing the framebuffer +// as it last legally was, rather than a truncated version of what it now is. +// +// Half 2 is the one that would go quiet on its own. A truncating emitter would still refuse +// nothing, publish a record, and leave the server's walk perfectly self-consistent over eight +// colour points while one attachment silently stopped existing. +TEST(FramebufferEmit, TheRecordsElevenSurfacesAreTheWholePointSetTheServerReads) { + FramebufferScope scope; + const auto fbo = MakeShared(21); + Vector> colors; + for (Uint i = 0; i < kMGPipeMaxColorAttachments; ++i) { + colors.push_back(MakeColorTexture(70 + i, 32)); + fbo->AttachTexture(static_cast( + static_cast(FramebufferAttachmentType::Color0) + static_cast(i)), + colors.back(), TextureUploadTarget::Texture2D); + } + const auto depth = MakeShared(21); + depth->SetInternalFormat(TextureInternalFormat::Depth24Stencil8); + depth->AllocateStorage(IntVec2{32, 32}); + fbo->AttachRenderbuffer(FramebufferAttachmentType::Depth, depth); + const auto stencil = MakeShared(22); + stencil->SetInternalFormat(TextureInternalFormat::Depth24Stencil8); + stencil->AllocateStorage(IntVec2{32, 32}); + fbo->AttachRenderbuffer(FramebufferAttachmentType::Stencil, stencil); + BindDrawAndRead(fbo, fbo); + Framebuffers().EmitFramebufferState(Ctx()); + ASSERT_EQ(Framebuffers().RefusedCount(), 0u) << "a full but legal framebuffer was refused"; + + // The wire's colour width and the server's walk width are ONE number. + static_assert(std::size(MGPFramebufferState{}.Color) == kMGPipeMaxColorAttachments, + "the server walks Color[0..kMGPipeMaxColorAttachments); the array must be that wide"); + + const MGPFramebufferState* const applied = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(applied, nullptr); + for (Uint i = 0; i < kMGPipeMaxColorAttachments; ++i) { + const MGPipeHandle expected = + MGPipeSlots().FindByLifetimeId(MGPipeKind::Texture, colors[i]->GetLifetimeId()); + EXPECT_FALSE(MGPipeHandleIsNull(expected)) << "colour point " << i << " minted no handle"; + EXPECT_TRUE(applied->Color[i].Res == expected) + << "colour point " << i << " did not reach the server's record"; + EXPECT_EQ(applied->Color[i].Kind, kMGPipeSurfaceKindTexture) << "at colour point " << i; + } + EXPECT_EQ(applied->Depth.Kind, kMGPipeSurfaceKindRenderbuffer) << "the Depth point"; + EXPECT_EQ(applied->Stencil.Kind, kMGPipeSurfaceKindRenderbuffer) << "the Stencil point"; + + // HALF 2. A point the wire cannot describe appears; the record the server holds must not + // move, because a moved one would be a description of a framebuffer that does not exist. + const Uint64 hashBefore = applied->ContentHash; + const Uint64 serialBefore = MGPipeApplier().FramebufferSerial; + const auto beyond = MakeColorTexture(90, 32); + fbo->AttachTexture(static_cast( + static_cast(FramebufferAttachmentType::Color0) + + static_cast(kMGPipeMaxColorAttachments)), + beyond, TextureUploadTarget::Texture2D); + Framebuffers().EmitFramebufferState(Ctx()); + EXPECT_GE(Framebuffers().RefusedCount(), 1u) << "the over-wide framebuffer was not refused"; + const MGPFramebufferState* const after = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(after, nullptr); + EXPECT_EQ(after->ContentHash, hashBefore) + << "a refused framebuffer still moved the record the server walks - the eleven surfaces " + "are no longer the whole point set"; + EXPECT_EQ(MGPipeApplier().FramebufferSerial, serialBefore) + << "a refused framebuffer advanced FramebufferSerial, which is the server's clean gate"; +} + +// ============================ P5e (fb), ruling 15 / ID-98's §5.4 note ==================== +// +// AN EGL SURFACE RESIZE RE-EMITS THE DEFAULT FRAMEBUFFER'S RECORD, extent and all. +// +// S3 could not settle this and flagged it: FramebufferEmit's dirty rule is +// m_anyAttachmentGeneration plus the object/slot version, and a resize that moved NEITHER would +// be suppressed - leaving the server with the old extent and no other source for it, because +// under run-ahead the frontend's is unreachable. The brief's ruling 15 verified the path +// (ClientSession's surface-changed consumer -> TextureObjectBase::PipePublishLevelDescriptor -> +// MGP_NOTE_AGGREGATE(FramebufferAttachment) -> the framebuffer shutter) and told fb to pin it. +// +// THE CASE DRIVES THE CONSUMER'S OWN CALL, not a synthetic bump: AllocateStorage on the colour +// attachment at the new extent is literally what ClientSession does when the swapchain +// publishes a new size. No rebind, no re-attach, no draw-buffer edit - which is exactly the +// shape that would have been suppressed. +TEST(FramebufferEmit, ADefaultFramebufferResizeReEmitsTheRecordWithItsNewExtent) { + FramebufferScope scope; + const auto color = MakeColorTexture(95, 64); + const auto fbo = MakeShared(22); + fbo->AttachTexture(FramebufferAttachmentType::Color0, color, TextureUploadTarget::Texture2D); + BindDrawAndRead(fbo, fbo); + Framebuffers().EmitFramebufferState(Ctx()); + + const MGPFramebufferState* applied = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(applied, nullptr); + ASSERT_EQ(applied->Width, 64u); + ASSERT_EQ(applied->Height, 64u); + const Uint64 hashBefore = applied->ContentHash; + const Uint64 emissionsBefore = Framebuffers().EmissionCount(); + + // THE RESIZE, exactly as the surface-changed consumer performs it. + color->AllocateStorage(TextureUploadTarget::Texture2D, 0, + MipmapInput{IntVec3{128, 96, 1}, static_cast(128 * 96 * 4)}); + Framebuffers().EmitFramebufferState(Ctx()); + + EXPECT_GT(Framebuffers().EmissionCount(), emissionsBefore) + << "a resize of an attached surface published no framebuffer record - under run-ahead the " + "server would keep the old extent and has no other source for it"; + applied = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(applied, nullptr); + EXPECT_NE(applied->ContentHash, hashBefore) << "the resize did not move the record's content hash"; + EXPECT_EQ(applied->Width, 128u); + EXPECT_EQ(applied->Height, 96u); +} + // ============================ D-D2 ============================ // // RED BEFORE THIS PACKAGE LANDED. RenderbufferObject's three storage setters bump no version diff --git a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp index 290dd289..71bfa31e 100644 --- a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp @@ -357,7 +357,8 @@ TEST(ImageEmit, AMakeCurrentClearsTheImageSetAndAdvancesItsSerial) { X(ImageEmit, AnInternalFormatChangeAloneStillEmitsTheSet) \ X(ImageEmit, TheApplicationsFormatAndAccessTravelUnrecast) \ X(ImageEmit, AnImageBoundTextureIsMarkedShaderImageBoundAtTheBind) \ - X(ImageEmit, TheBindFeedsTheImageUnitHighWaterMark) + X(ImageEmit, TheBindFeedsTheImageUnitHighWaterMark) \ + X(ImageEmit, TheThreeAccessConstantsArePinnedOnBothSidesOfTheWire) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -487,6 +488,87 @@ void main() { imageStore(img, ivec2(0), vec4(1.0)); } GL::UseProgram(0); } + // ============================ P5e, ruling 16 / ID-94 ============================ + // + // THE THREE CONSTANTS, PINNED ON BOTH SIDES OF THE WIRE, in one case, because the whole of + // ruling 16 is that they are ONE table and not two that happen to agree. + // + // Before P5e the client folded the three GL names into 0/1/2 in ImageEmit.h and the server + // read MGPImageBinding::Access off the FRONTEND instead of decoding the byte, under a + // comment saying the encoding "does not exist at the contract commit" - which was stale + // when it was written. c0e moved the numbers into MG_Pipe/MGPipeValueTypes.h and fb made + // the backend decode through them, so the failure this case exists to catch is the two + // halves drifting: an encode that writes 1 for GL_WRITE_ONLY against a decode that reads 1 + // as GL_READ_WRITE is a `writeonly` image the shader can read back, and no pixel test on a + // lockstep tree would see it because the frontend value was answering. + // + // 0 IS READ-ONLY AND THAT IS ASSERTED RATHER THAN ASSUMED: a zeroed MGPImageView must + // decode to the most restrictive access, so a record that forgot to set the field can only + // lose a write, never invent one. + TEST(ImageEmit, TheThreeAccessConstantsArePinnedOnBothSidesOfTheWire) { + // --- the numbers themselves ------------------------------------------------------- + EXPECT_EQ(kMGPipeImageAccessReadOnly, 0u); + EXPECT_EQ(kMGPipeImageAccessWriteOnly, 1u); + EXPECT_EQ(kMGPipeImageAccessReadWrite, 2u); + EXPECT_EQ(static_cast(MGPipeImageAccess::Count), 3u); + + // --- the CLIENT half: the encode names those enumerators, not literals ------------- + EXPECT_EQ(MGPipeEncodeImageAccess(GL_READ_ONLY), kMGPipeImageAccessReadOnly); + EXPECT_EQ(MGPipeEncodeImageAccess(GL_WRITE_ONLY), kMGPipeImageAccessWriteOnly); + EXPECT_EQ(MGPipeEncodeImageAccess(GL_READ_WRITE), kMGPipeImageAccessReadWrite); + + // --- the SERVER half: the decode is the inverse, and only those three are valid ----- + EXPECT_TRUE(MGPipeImageAccessIsValid(kMGPipeImageAccessReadOnly)); + EXPECT_TRUE(MGPipeImageAccessIsValid(kMGPipeImageAccessWriteOnly)); + EXPECT_TRUE(MGPipeImageAccessIsValid(kMGPipeImageAccessReadWrite)); + EXPECT_FALSE(MGPipeImageAccessIsValid(3)) + << "a fourth value must be Fatal{ProtocolCorruption, \"ImageView.Access\"} at the reader"; + EXPECT_EQ(MGPipeDecodeImageAccess(kMGPipeImageAccessReadOnly), MGPipeImageAccess::ReadOnly); + EXPECT_EQ(MGPipeDecodeImageAccess(kMGPipeImageAccessWriteOnly), MGPipeImageAccess::WriteOnly); + EXPECT_EQ(MGPipeDecodeImageAccess(kMGPipeImageAccessReadWrite), MGPipeImageAccess::ReadWrite); + + // The two predicates the backend's writable-image set and the barrier plan ask with. + EXPECT_TRUE(MGPipeImageAccessReads(MGPipeImageAccess::ReadOnly)); + EXPECT_FALSE(MGPipeImageAccessWrites(MGPipeImageAccess::ReadOnly)); + EXPECT_FALSE(MGPipeImageAccessReads(MGPipeImageAccess::WriteOnly)); + EXPECT_TRUE(MGPipeImageAccessWrites(MGPipeImageAccess::WriteOnly)); + EXPECT_TRUE(MGPipeImageAccessReads(MGPipeImageAccess::ReadWrite)); + EXPECT_TRUE(MGPipeImageAccessWrites(MGPipeImageAccess::ReadWrite)); + + // --- and the byte that actually crosses, for each of the three ------------------- + // + // ROUND TRIP THROUGH A REAL BIND, not through the encode alone: what the case is about + // is the value the SERVER will decode, and that is MGPImageView::Access as the emitter + // wrote it. + const struct { + GLenum gl; + Uint8 encoded; + MGPipeImageAccess decoded; + } kCases[] = { + {GL_READ_ONLY, kMGPipeImageAccessReadOnly, MGPipeImageAccess::ReadOnly}, + {GL_WRITE_ONLY, kMGPipeImageAccessWriteOnly, MGPipeImageAccess::WriteOnly}, + {GL_READ_WRITE, kMGPipeImageAccessReadWrite, MGPipeImageAccess::ReadWrite}, + }; + for (const auto& c : kCases) { + EmitterScope scope; + const GLuint program = MakeComputeProgram(kImageCompute); + GL::UseProgram(program); + const GLuint texture = MakeImageTexture(); + GL::BindImageTexture(1, texture, 0, GL_FALSE, 0, c.gl, GL_RGBA8); + Emitter().EmitShaderImages(Ctx()); + ASSERT_GE(Emitter().ImageSetCount(), 1u); + ASSERT_GE(Emitter().LastShaderImages().Count, 2u); + const MGPImageView& view = Emitter().LastImageViews()[1]; + EXPECT_EQ(view.Access, c.encoded) + << "glBindImageTexture(access=0x" << std::hex << static_cast(c.gl) << std::dec + << ") did not travel as the shared constant"; + ASSERT_TRUE(MGPipeImageAccessIsValid(view.Access)); + EXPECT_EQ(MGPipeDecodeImageAccess(view.Access), c.decoded) + << "the server's decode does not invert the client's encode"; + GL::UseProgram(0); + } + } + TEST(ImageEmit, AnAccessModeChangeAloneStillEmitsTheSet) { EmitterScope scope; const GLuint program = MakeComputeProgram(kImageCompute);