diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 5a0fcc3d..e8d0e7f7 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -595,6 +595,14 @@ namespace MobileGL::MG_Config { // flag keyed on wantReply would disable the very mechanism it exists to enable // (docs/Disaggregated/notes/p12/CREATE-WINDOW-MEASURED.md sections 7-11). Uint32 CreateWindow = 10; + // MOBILEGL_IPC_PBO_READBACK_SYNC (P9 W1, MG_Remote/CONTRACT-P9.md §1): 1 = a glReadPixels + // or glGetTexImage into a bound GL_PIXEL_PACK_BUFFER takes the reply-slot form again - + // the client waits for the pixels and uploads them into the buffer itself, as before P9. + // 0 (default) sends read_pixels_to_buffer / get_texture_image_to_buffer, waits for + // nothing, and marks the buffer GPU-written. The 1 arm is the A/B control and the + // red-once of the wait-count gate (PackBufferReadbackScenario): it must show a reply + // wait per read. + Uint32 PboReadbackSync = 0; // MOBILEGL_IPC_ADOPT_TIER: 2 = emulate (client keeps the shadow and pushes), which // is the only tier P5 implements and the reason persistent-map-push can be non-zero // at all (R-6). 0 and 1 parse and are Fatal at use with "P11"; they exist now so the diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index f71c9b8a..7d3b7c42 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -421,6 +421,8 @@ namespace MobileGL::MG_ConfigLoader { // cap of 4 (see WireTables.cpp's kCreateWindowMax). WireTables.cpp's static_assert and // StreamLink's crossing log are the checks on this number. ipc.CreateWindow = QueryEnvUint32("MOBILEGL_IPC_CREATE_WINDOW", 10, 1, 12); + // P9 W1: the pack-buffer readback's A/B and red-once control (Config.h says both). + ipc.PboReadbackSync = QueryEnvUint32("MOBILEGL_IPC_PBO_READBACK_SYNC", 0, 0, 1); // The verify harness compares the pushed block against the applier per verb; a // batched queue lets the comparer read a supplied field mid-apply, which is a // torn read rather than a divergence. The batch is therefore off whenever the @@ -522,11 +524,11 @@ namespace MobileGL::MG_ConfigLoader { // changes the wall clock by seconds (the P12 sweep: 28.3 s at 1 down to 10.8 s at 6) a // log that does not name it makes every run ambiguous about its own configuration. MGLOG_I("Config: IPC ring=%uMiB stage=%uMiB wire-deferred=%uMiB spin=%uus event-wait=%ums " - "persistent-block=%uKiB create-window=%u " + "persistent-block=%uKiB create-window=%u pbo-readback-sync=%u " "adopt-tier=%u verb-barrier=%u run-ahead=%u present-credit=%u control-timeout=%ums " "cold-start=%ums strict=%d audit=%d role-split-state=%d affinity='%s' surface=%s", ipc.RingMb, ipc.StageMb, ipc.WireDeferredMb, ipc.SpinUs, ipc.EventWaitMs, - ipc.PersistentBlockKb, ipc.CreateWindow, ipc.AdoptTier, + ipc.PersistentBlockKb, ipc.CreateWindow, ipc.PboReadbackSync, ipc.AdoptTier, ipc.VerbBarrier, ipc.RunAhead, ipc.PresentCredit, ipc.ControlTimeoutMs, ipc.ColdStartMs, static_cast(ipc.StrictErrors), static_cast(ipc.Audit), static_cast(ipc.RoleSplitState), ipc.ServerAffinity.c_str(), diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index c6472853..682ce101 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -41,6 +41,18 @@ // call sites below are declarations only, exactly as the frontend's mutators are. #include +// P9 (W1, MG_Remote/CONTRACT-P9.md §1): AN UNPACK SOURCE IS READ FROM THE BUFFER'S CLIENT SHADOW, +// and under a transport a GPU write into that buffer - a read_pixels_to_buffer, which is how a +// glReadPixels into a pack buffer now crosses, or a shader store - reaches the shadow only through +// SyncGpuWrites, exactly as glMapBufferRange and glGetBufferSubData already do. Monolith keeps its +// read as it was: its ReadPixels maps the driver PBO back into the shadow inside the call, and the +// pull build compiles this to nothing (G1). +#if MOBILEGL_BUILD_DISAGGREGATED +#define MGL_SYNC_PIXEL_UNPACK_SOURCE(buffer) do { if (MG_Config::Transport != MG_Config::TransportMode::Monolith) (buffer)->SyncGpuWrites(); } while (0) +#else +#define MGL_SYNC_PIXEL_UNPACK_SOURCE(buffer) ((void)0) +#endif + namespace MobileGL::MG_Impl::GLImpl { static SharedPtr nullTextureObject; static UnorderedMap g_autoGenerateMipmapByTextureId; @@ -1773,6 +1785,7 @@ namespace MobileGL::MG_Impl::GLImpl { const auto& unpackBuffer = MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject(); if (!unpackBuffer) return data; + MGL_SYNC_PIXEL_UNPACK_SOURCE(unpackBuffer); return reinterpret_cast(unpackBuffer->MappedData()) + reinterpret_cast(data); } @@ -1819,6 +1832,7 @@ namespace MobileGL::MG_Impl::GLImpl { const auto& pixelUnpackBufferObject = MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject(); if (pixelUnpackBufferObject) { + MGL_SYNC_PIXEL_UNPACK_SOURCE(pixelUnpackBufferObject); originalPixels = reinterpret_cast(pixelUnpackBufferObject->MappedData()) + reinterpret_cast(pixels); } @@ -1941,6 +1955,7 @@ namespace MobileGL::MG_Impl::GLImpl { if (pixelUnpackBufferObject) { MGLOG_D("TexSubImage2D_State: Using Pixel Unpack Buffer Object ID: %u", pixelUnpackBufferObject->GetExternalIndex()); + MGL_SYNC_PIXEL_UNPACK_SOURCE(pixelUnpackBufferObject); originalPixels = reinterpret_cast(pixelUnpackBufferObject->MappedData()) + reinterpret_cast(pixels); } @@ -2030,6 +2045,7 @@ namespace MobileGL::MG_Impl::GLImpl { const auto& pixelUnpackBufferObject = MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject(); if (pixelUnpackBufferObject) { + MGL_SYNC_PIXEL_UNPACK_SOURCE(pixelUnpackBufferObject); originalPixels = reinterpret_cast(pixelUnpackBufferObject->MappedData()) + reinterpret_cast(pixels); } @@ -2535,6 +2551,7 @@ namespace MobileGL::MG_Impl::GLImpl { if (pixelUnpackBufferObject) { MGLOG_D("%s: Using Pixel Unpack Buffer Object ID: %u", __func__, pixelUnpackBufferObject->GetExternalIndex()); + MGL_SYNC_PIXEL_UNPACK_SOURCE(pixelUnpackBufferObject); originalPixels = reinterpret_cast(pixelUnpackBufferObject->MappedData()) + reinterpret_cast(pixels); } @@ -2682,6 +2699,7 @@ namespace MobileGL::MG_Impl::GLImpl { if (pixelUnpackBufferObject) { MGLOG_D("%s: Using Pixel Unpack Buffer Object ID: %u", __func__, pixelUnpackBufferObject->GetExternalIndex()); + MGL_SYNC_PIXEL_UNPACK_SOURCE(pixelUnpackBufferObject); originalPixels = reinterpret_cast(pixelUnpackBufferObject->MappedData()) + reinterpret_cast(pixels); } @@ -2800,6 +2818,7 @@ namespace MobileGL::MG_Impl::GLImpl { const auto& pixelUnpackBufferObject = MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject(); if (pixelUnpackBufferObject) { + MGL_SYNC_PIXEL_UNPACK_SOURCE(pixelUnpackBufferObject); originalPixels = reinterpret_cast(pixelUnpackBufferObject->MappedData()) + reinterpret_cast(pixels); } @@ -6262,6 +6281,7 @@ namespace MobileGL::MG_Impl::GLImpl { const auto& pixelUnpackBufferObject = MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject(); if (pixelUnpackBufferObject) { + MGL_SYNC_PIXEL_UNPACK_SOURCE(pixelUnpackBufferObject); originalPixels = reinterpret_cast(pixelUnpackBufferObject->MappedData()) + reinterpret_cast(pixels); } diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 5f899f9f..226b2bc5 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -84,6 +84,9 @@ add_executable(MobileGLIntegrationTest # P7 gate 5 (g5-readback): readbacks larger than one reply slot. Ordinary GL cases, so the # monolith arms run them too - that is the parity half of the split registrations below. Scenarios/BandedReadbackScenario.cpp + # P9 (W1): reads into a pixel pack buffer answered by nothing. Ordinary GL cases too, plus + # one wire case that skips where there is no wire. + Scenarios/PackBufferReadbackScenario.cpp Scenarios/PrimitiveRestartScenario.cpp Scenarios/FragCoordOriginScenario.cpp Scenarios/ClientVertexArrayScenario.cpp @@ -2447,6 +2450,13 @@ if (MOBILEGL_BUILD_DISAGGREGATED) # buffer, depth and packed depth/stencil, and glGetTexImage's windowed twin. The Magma arms of # the same cases are the `Band.` block after the Fm lane below. mgl_itest_register_split_arms("BandedReadbackScenario.*" "") + # P9 (W1, MG_Remote/CONTRACT-P9.md §1): the pack-buffer readback crosses as + # read_pixels_to_buffer / get_texture_image_to_buffer and is answered by nothing. Six parity + # cases (every pack parameter, the map reader, a byte swap, a layered glGetTexImage, an upload + # FROM the buffer the read filled, a later write over part of it) and the wire case (no + # reply-slot record, one mark per read). The Magma arms are the `Pbo.` block after the Band + # block below. + mgl_itest_register_split_arms("PackBufferReadbackScenario.*" "") # DepthStencilReadbackMatrixScenario GETS ITS OWN BLOCK, because it needs a variable the # registration macro cannot carry: MOBILEGL_ESPRYT_FORCE_DS_READBACK_EMULATION=1. @@ -3581,6 +3591,24 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ${mglItestArmProperties} ENVIRONMENT "${mglItestMagmaBandEnvironment}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/g5-band${mglItestArmLogTag}-${bandCase}.log") endforeach() + # P9 (W1): the pack-buffer readback on Magma. The server half is backend-neutral - the + # reply form's read, then the applier's buffer-write gate (WriteWireBuffer here) - so the + # same seven cases, same flavour. + foreach(pboCase ReadPixelsIntoAPackBufferMatchesClientMemoryUnderEveryPackState + MapBufferRangeAfterAPackBufferReadSeesThePixels + SwapBytesIntoAPackBufferMatchesClientMemory + GetTexImageIntoAPackBufferMatchesClientMemory + APackBufferReadFeedsAnUploadFromTheSameBuffer + ALaterBufferSubDataWinsOverThePackReadWhereTheyOverlap + APackBufferReadPostsNoReplyAndMarksTheBuffer) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectVulkan.${mglItestArm}.Pbo." + TEST_FILTER "PackBufferReadbackScenario.${pboCase}" + DISCOVERY_TIMEOUT 30 + PROPERTIES LABELS "${mglItestMagmaLabels}" TIMEOUT ${MGL_ITEST_TIMEOUT} + ${mglItestArmProperties} + ENVIRONMENT "${mglItestMagmaBandEnvironment}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p9-pbo${mglItestArmLogTag}-${pboCase}.log") + endforeach() endforeach() endif() diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp index 1bbd66da..cbc8c069 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -164,6 +165,9 @@ namespace MGITest { state.cmdWrapPads = encoder.CmdWrapPads(); state.cmdBytesWritten = encoder.CmdBytesWritten(); state.stageReclaimWaits = encoder.StageReclaimWaits(); + state.replyPostings = session->ReplyPostings(); + state.packBufferReadbackMarks = MobileGL::MG_Remote::Client::ProducerMarkCount( + MobileGL::MG_Remote::Client::GpuWriteProducer::ReadPixelsPackBuffer); } #endif return state; diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h index a7a2630b..1f305d1f 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h @@ -119,6 +119,17 @@ namespace MGITest { unsigned long long cmdWrapPads = 0; unsigned long long cmdBytesWritten = 0; unsigned long long stageReclaimWaits = 0; + + // ---- P9 (W1, CONTRACT-P9.md §1): the pack-buffer readback's two proofs ------------- + // + // replyPostings: records this client published on a row that carries a reply slot + // (ClientSession::ReplyPostings, always counted - it is the create window's clock, not a + // stats counter). A read into a pack buffer that moves it went through the reply form. + // packBufferReadbackMarks: GpuWritePending's row-4 producer count - the buffers a pack + // read marked GPU-written. The reply form marks nothing, so the pair tells the two forms + // apart from both sides. + unsigned long long replyPostings = 0; + unsigned long long packBufferReadbackMarks = 0; }; SplitRuntimeState PeekSplitRuntime(); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PackBufferReadbackScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PackBufferReadbackScenario.cpp new file mode 100644 index 00000000..692f4447 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/PackBufferReadbackScenario.cpp @@ -0,0 +1,454 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/PackBufferReadbackScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// P9 (W1, MG_Remote/CONTRACT-P9.md §1): READS INTO A PIXEL PACK BUFFER ARE ANSWERED BY NOTHING. +// +// Under a transport a glReadPixels / glGetTexImage whose destination is a bound +// GL_PIXEL_PACK_BUFFER no longer waits for its pixels: it crosses as read_pixels_to_buffer / +// get_texture_image_to_buffer, the server lands the rows in the buffer at the client's PACK +// layout, and the client marks the buffer GPU-written so that the next CPU read of it - a +// glGetBufferSubData, a glMapBufferRange, or a texture upload that uses it as the UNPACK source - +// fetches the bytes through resource_readback. +// +// THE CASES ARE ORDINARY GL AND RUN ON THE MONOLITH ARMS TOO. Each one compares the pack buffer +// against the same read into client memory under the same pack state, both destinations +// pre-filled with the same byte, so the padding the pack state skips has to survive as well: the +// monolith and every split arm must agree with themselves, and therefore with each other. The +// one exception is the last case, which is about the WIRE (the read posted no reply and marked +// the buffer) and skips where there is no wire to ask. +// +// Every image is POSITION-DEPENDENT, so a row landed at the wrong stride, a band at the wrong +// offset or a byte swapped the wrong way cannot look right. + +#include +#include +#include +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" +#include "../Harness/SplitRuntimePeek.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr GLubyte kFill = 0xAB; + + // A texel that names its own position (and its layer), so nothing misplaced can match. + std::array PatternRgba8(int x, int y, int layer) { + return {static_cast(x * 7 + layer * 31), static_cast(y * 11 + layer), + static_cast((x ^ y) + layer * 5), static_cast(0x80 | (x + y))}; + } + + // Integers below 2^24 in every component, so float storage is exact and a byte swap of + // any of them is visibly different from the value. + std::array PatternRgba32f(int x, int y) { + return {static_cast(x + 1), static_cast(y + 3), + static_cast(x * 64 + y), static_cast(1000 + x * y)}; + } + + struct PackState { + const char* name; + GLint alignment; + GLint rowLength; + GLint skipPixels; + GLint skipRows; + GLboolean swapBytes; + }; + + // Neutral, then each PACK parameter that changes the layout, then all of them at once. + constexpr PackState kPackStates[] = { + {"neutral", 4, 0, 0, 0, GL_FALSE}, + {"alignment 1", 1, 0, 0, 0, GL_FALSE}, + {"alignment 8", 8, 0, 0, 0, GL_FALSE}, + {"row length 53", 4, 53, 0, 0, GL_FALSE}, + {"skip pixels 3, skip rows 2", 4, 0, 3, 2, GL_FALSE}, + {"everything", 8, 47, 5, 3, GL_FALSE}, + }; + + class PackBufferReadbackScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + DrainErrors(); + glGenFramebuffers(1, &m_fbo); + glBindFramebuffer(GL_FRAMEBUFFER, m_fbo); + glDisable(GL_SCISSOR_TEST); + ResetPack(); + } + + void TearDown() override { + if (!Ready()) return; + ResetPack(); + glBindBuffer(GL_PIXEL_PACK_BUFFER, 0); + glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0); + for (GLuint buffer : m_buffers) glDeleteBuffers(1, &buffer); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + if (m_fbo != 0) glDeleteFramebuffers(1, &m_fbo); + for (GLuint texture : m_textures) glDeleteTextures(1, &texture); + m_buffers.clear(); + m_textures.clear(); + m_fbo = 0; + DrainErrors(); + ScenarioTest::TearDown(); + } + + static void ResetPack() { + glPixelStorei(GL_PACK_ALIGNMENT, 4); + glPixelStorei(GL_PACK_ROW_LENGTH, 0); + glPixelStorei(GL_PACK_SKIP_PIXELS, 0); + glPixelStorei(GL_PACK_SKIP_ROWS, 0); + glPixelStorei(GL_PACK_IMAGE_HEIGHT, 0); + glPixelStorei(GL_PACK_SKIP_IMAGES, 0); + glPixelStorei(GL_PACK_SWAP_BYTES, GL_FALSE); + glPixelStorei(GL_UNPACK_ALIGNMENT, 4); + glPixelStorei(GL_UNPACK_ROW_LENGTH, 0); + } + + static void ApplyPack(const PackState& pack) { + glPixelStorei(GL_PACK_ALIGNMENT, pack.alignment); + glPixelStorei(GL_PACK_ROW_LENGTH, pack.rowLength); + glPixelStorei(GL_PACK_SKIP_PIXELS, pack.skipPixels); + glPixelStorei(GL_PACK_SKIP_ROWS, pack.skipRows); + glPixelStorei(GL_PACK_SWAP_BYTES, pack.swapBytes); + } + + static void DrainErrors() { + for (int i = 0; i < 16 && glGetError() != GL_NO_ERROR; ++i) { + } + } + + // The bytes a pack state makes a width x height read occupy, from the first byte the + // read may touch (GL 4.6 §8.4.4.1 / §18.2): the skips, then the padded rows, the last + // row tight. + static std::size_t PackedExtent(const PackState& pack, int width, int height, std::size_t bpp) { + const std::size_t rowPixels = pack.rowLength > 0 ? static_cast(pack.rowLength) + : static_cast(width); + const std::size_t align = static_cast(pack.alignment); + const std::size_t stride = (rowPixels * bpp + align - 1) / align * align; + return static_cast(pack.skipRows) * stride + static_cast(pack.skipPixels) * bpp + + static_cast(height - 1) * stride + static_cast(width) * bpp; + } + + GLuint NewTexture() { + GLuint texture = 0; + glGenTextures(1, &texture); + m_textures.push_back(texture); + return texture; + } + + GLuint NewBuffer(GLenum target, std::size_t size, GLubyte fill) { + GLuint buffer = 0; + glGenBuffers(1, &buffer); + m_buffers.push_back(buffer); + glBindBuffer(target, buffer); + const std::vector initial(size, fill); + glBufferData(target, static_cast(size), initial.data(), GL_DYNAMIC_READ); + return buffer; + } + + // An RGBA8 texture holding the pattern, attached as COLOR_ATTACHMENT0 for reading. + GLuint AttachPatternRgba8(int width, int height) { + std::vector texels(static_cast(width) * height * 4); + for (int y = 0; y < height; ++y) + for (int x = 0; x < width; ++x) { + const auto t = PatternRgba8(x, y, 0); + std::memcpy(texels.data() + (static_cast(y) * width + x) * 4, t.data(), 4); + } + const GLuint texture = NewTexture(); + glBindTexture(GL_TEXTURE_2D, texture); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, width, height); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, width, height, GL_RGBA, GL_UNSIGNED_BYTE, texels.data()); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glBindFramebuffer(GL_FRAMEBUFFER, m_fbo); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0); + glReadBuffer(GL_COLOR_ATTACHMENT0); + EXPECT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), static_cast(GL_FRAMEBUFFER_COMPLETE)); + EXPECT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)) << "pattern upload"; + return texture; + } + + // glReadPixels of (x, y, width, height) under `pack` into client memory and into a + // pack buffer at `offset`, both pre-filled with kFill; the buffer's bytes are then + // read back by `readBack` and the two must be identical, byte for byte, padding + // included - and the bytes in front of `offset` must still be kFill. + template + void ExpectPackBufferMatchesClientMemory(const PackState& pack, int x, int y, int width, int height, + GLenum type, std::size_t bpp, ReadBack&& readBack) { + ApplyPack(pack); + const std::size_t extent = PackedExtent(pack, width, height, bpp); + std::vector host(extent, kFill); + glReadPixels(x, y, width, height, GL_RGBA, type, host.data()); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)) << pack.name << ": client-memory read"; + + constexpr std::size_t kOffset = 64; + NewBuffer(GL_PIXEL_PACK_BUFFER, kOffset + extent, kFill); + glReadPixels(x, y, width, height, GL_RGBA, type, reinterpret_cast(kOffset)); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)) << pack.name << ": pack-buffer read"; + std::vector actual(kOffset + extent, 0); + readBack(actual); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)) << pack.name << ": buffer read-back"; + glBindBuffer(GL_PIXEL_PACK_BUFFER, 0); + ResetPack(); + + std::size_t lead = 0; + for (std::size_t i = 0; i < kOffset; ++i) lead += actual[i] != kFill ? 1 : 0; + EXPECT_EQ(lead, 0u) << pack.name << ": " << lead << " bytes in front of the read's offset moved"; + std::size_t bad = 0, first = 0; + for (std::size_t i = 0; i < extent; ++i) { + if (actual[kOffset + i] != host[i] && bad++ == 0) first = i; + } + EXPECT_EQ(bad, 0u) << pack.name << ": " << bad << " of " << extent + << " bytes differ from the client-memory read, first at byte " << first + << " (buffer " << static_cast(actual[kOffset + first]) << ", host " + << static_cast(host[first]) << ")"; + } + + GLuint m_fbo = 0; + std::vector m_buffers; + std::vector m_textures; + }; + + TEST_F(PackBufferReadbackScenario, ReadPixelsIntoAPackBufferMatchesClientMemoryUnderEveryPackState) { + if (!Ready()) return; + AttachPatternRgba8(37, 23); + if (HasFatalFailure()) return; + for (const PackState& pack : kPackStates) { + ExpectPackBufferMatchesClientMemory(pack, 2, 1, 31, 19, GL_UNSIGNED_BYTE, 4, [](std::vector& out) { + glGetBufferSubData(GL_PIXEL_PACK_BUFFER, 0, static_cast(out.size()), out.data()); + }); + if (HasFatalFailure()) return; + } + } + + // The map is the OTHER CPU reader, and the one an application that streams readbacks + // through a PBO actually uses: glMapBufferRange(GL_MAP_READ_BIT) must see the pixels the + // server landed, not the bytes the client shadow held before the read. + TEST_F(PackBufferReadbackScenario, MapBufferRangeAfterAPackBufferReadSeesThePixels) { + if (!Ready()) return; + AttachPatternRgba8(37, 23); + if (HasFatalFailure()) return; + for (const PackState& pack : {kPackStates[0], kPackStates[5]}) { + ExpectPackBufferMatchesClientMemory(pack, 0, 0, 37, 23, GL_UNSIGNED_BYTE, 4, [](std::vector& out) { + void* mapped = glMapBufferRange(GL_PIXEL_PACK_BUFFER, 0, static_cast(out.size()), + GL_MAP_READ_BIT); + ASSERT_NE(mapped, nullptr); + std::memcpy(out.data(), mapped, out.size()); + glUnmapBuffer(GL_PIXEL_PACK_BUFFER); + }); + if (HasFatalFailure()) return; + } + } + + // GL_PACK_SWAP_BYTES reverses every component's bytes; the client carries the group + // width and the server swaps, so a wrong width or a skipped swap is a different float. + TEST_F(PackBufferReadbackScenario, SwapBytesIntoAPackBufferMatchesClientMemory) { + if (!Ready()) return; + constexpr int kWidth = 13, kHeight = 9; + std::vector texels(static_cast(kWidth) * kHeight * 4); + for (int y = 0; y < kHeight; ++y) + for (int x = 0; x < kWidth; ++x) { + const auto t = PatternRgba32f(x, y); + std::memcpy(texels.data() + (static_cast(y) * kWidth + x) * 4, t.data(), sizeof(t)); + } + const GLuint texture = NewTexture(); + glBindTexture(GL_TEXTURE_2D, texture); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA32F, kWidth, kHeight); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, kWidth, kHeight, GL_RGBA, GL_FLOAT, texels.data()); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0); + glReadBuffer(GL_COLOR_ATTACHMENT0); + ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), static_cast(GL_FRAMEBUFFER_COMPLETE)); + const PackState swapped{"swap bytes, row length 17", 4, 17, 1, 1, GL_TRUE}; + ExpectPackBufferMatchesClientMemory(swapped, 0, 0, kWidth, kHeight, GL_FLOAT, 16, [](std::vector& out) { + glGetBufferSubData(GL_PIXEL_PACK_BUFFER, 0, static_cast(out.size()), out.data()); + }); + } + + // glGetTexImage into a pack buffer, 2D and layered: the layered case is the one whose + // layout has an image stride (GL_PACK_IMAGE_HEIGHT) and a skip in the third dimension. + TEST_F(PackBufferReadbackScenario, GetTexImageIntoAPackBufferMatchesClientMemory) { + if (!Ready()) return; + constexpr int kWidth = 11, kHeight = 7, kLayers = 3; + std::vector texels(static_cast(kWidth) * kHeight * kLayers * 4); + for (int layer = 0; layer < kLayers; ++layer) + for (int y = 0; y < kHeight; ++y) + for (int x = 0; x < kWidth; ++x) { + const auto t = PatternRgba8(x, y, layer); + std::memcpy(texels.data() + ((static_cast(layer) * kHeight + y) * kWidth + x) * 4, + t.data(), 4); + } + const GLuint flat = NewTexture(); + glBindTexture(GL_TEXTURE_2D, flat); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kWidth, kHeight); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, texels.data()); + const GLuint layered = NewTexture(); + glBindTexture(GL_TEXTURE_2D_ARRAY, layered); + glTexStorage3D(GL_TEXTURE_2D_ARRAY, 1, GL_RGBA8, kWidth, kHeight, kLayers); + glTexSubImage3D(GL_TEXTURE_2D_ARRAY, 0, 0, 0, 0, kWidth, kHeight, kLayers, GL_RGBA, GL_UNSIGNED_BYTE, + texels.data()); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)) << "texture upload"; + + struct Shape { + const char* name; + GLenum target; + GLuint texture; + int depth; + GLint rowLength, alignment, imageHeight, skipImages; + }; + const Shape shapes[] = { + {"2D, row length 13, alignment 8", GL_TEXTURE_2D, flat, 1, 13, 8, 0, 0}, + {"2D array, image height 9, skip images 1", GL_TEXTURE_2D_ARRAY, layered, kLayers, 0, 4, 9, 1}, + }; + for (const Shape& shape : shapes) { + glBindTexture(shape.target, shape.texture); + glPixelStorei(GL_PACK_ROW_LENGTH, shape.rowLength); + glPixelStorei(GL_PACK_ALIGNMENT, shape.alignment); + glPixelStorei(GL_PACK_IMAGE_HEIGHT, shape.imageHeight); + glPixelStorei(GL_PACK_SKIP_IMAGES, shape.skipImages); + const std::size_t rowPixels = shape.rowLength > 0 ? static_cast(shape.rowLength) : kWidth; + const std::size_t stride = (rowPixels * 4 + shape.alignment - 1) / shape.alignment * shape.alignment; + const std::size_t imageRows = shape.imageHeight > 0 ? static_cast(shape.imageHeight) : kHeight; + const std::size_t imageStride = stride * imageRows; + const std::size_t extent = static_cast(shape.skipImages) * imageStride + + static_cast(shape.depth - 1) * imageStride + + static_cast(kHeight - 1) * stride + kWidth * 4u; + std::vector host(extent, kFill); + glGetTexImage(shape.target, 0, GL_RGBA, GL_UNSIGNED_BYTE, host.data()); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)) << shape.name << ": client-memory read"; + + constexpr std::size_t kOffset = 32; + NewBuffer(GL_PIXEL_PACK_BUFFER, kOffset + extent, kFill); + glGetTexImage(shape.target, 0, GL_RGBA, GL_UNSIGNED_BYTE, reinterpret_cast(kOffset)); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)) << shape.name << ": pack-buffer read"; + std::vector actual(kOffset + extent, 0); + glGetBufferSubData(GL_PIXEL_PACK_BUFFER, 0, static_cast(actual.size()), actual.data()); + glBindBuffer(GL_PIXEL_PACK_BUFFER, 0); + ResetPack(); + std::size_t bad = 0, first = 0; + for (std::size_t i = 0; i < kOffset; ++i) bad += actual[i] != kFill ? 1 : 0; + EXPECT_EQ(bad, 0u) << shape.name << ": bytes in front of the offset moved"; + bad = 0; + for (std::size_t i = 0; i < extent; ++i) + if (actual[kOffset + i] != host[i] && bad++ == 0) first = i; + EXPECT_EQ(bad, 0u) << shape.name << ": " << bad << " of " << extent + << " bytes differ from the client-memory read, first at byte " << first; + } + } + + // THE UNPACK HALF. A read into a buffer followed by an upload FROM that buffer is a + // GPU-to-GPU copy an application expects to work; under a transport the upload reads the + // client's shadow of the buffer, which the pack read did not touch, so this is exactly + // the case that goes wrong if the upload path forgets that the buffer was GPU-written. + TEST_F(PackBufferReadbackScenario, APackBufferReadFeedsAnUploadFromTheSameBuffer) { + if (!Ready()) return; + constexpr int kWidth = 29, kHeight = 17; + AttachPatternRgba8(kWidth, kHeight); + if (HasFatalFailure()) return; + const GLuint buffer = NewBuffer(GL_PIXEL_PACK_BUFFER, static_cast(kWidth) * kHeight * 4, kFill); + glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + glBindBuffer(GL_PIXEL_PACK_BUFFER, 0); + + const GLuint copy = NewTexture(); + glBindTexture(GL_TEXTURE_2D, copy); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kWidth, kHeight); + glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer); + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)) << "read into the buffer, then upload from it"; + + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, copy, 0); + std::vector actual(static_cast(kWidth) * kHeight * 4, 0); + glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, actual.data()); + std::size_t bad = 0; + std::string first; + for (int y = 0; y < kHeight; ++y) + for (int x = 0; x < kWidth; ++x) { + const auto expected = PatternRgba8(x, y, 0); + if (std::memcmp(actual.data() + (static_cast(y) * kWidth + x) * 4, expected.data(), 4) != 0 && + bad++ == 0) + first = "(" + std::to_string(x) + "," + std::to_string(y) + ")"; + } + EXPECT_EQ(bad, 0u) << bad << " texels of the uploaded copy differ from the pattern, first at " << first; + } + + // ORDER AGAINST LATER WRITES. A glBufferSubData into part of the buffer AFTER the read + // must win over the read's bytes there and leave the rest of the read alone - on the + // server the pixels and the later write are two queued ranges of one store, drained in + // record order. + TEST_F(PackBufferReadbackScenario, ALaterBufferSubDataWinsOverThePackReadWhereTheyOverlap) { + if (!Ready()) return; + constexpr int kWidth = 16, kHeight = 8; + AttachPatternRgba8(kWidth, kHeight); + if (HasFatalFailure()) return; + const std::size_t size = static_cast(kWidth) * kHeight * 4; + std::vector host(size, kFill); + glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, host.data()); + NewBuffer(GL_PIXEL_PACK_BUFFER, size, kFill); + glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + const std::vector overwrite(40, 0x5A); + glBufferSubData(GL_PIXEL_PACK_BUFFER, 100, static_cast(overwrite.size()), overwrite.data()); + std::vector actual(size, 0); + glGetBufferSubData(GL_PIXEL_PACK_BUFFER, 0, static_cast(size), actual.data()); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)); + std::memcpy(host.data() + 100, overwrite.data(), overwrite.size()); + std::size_t bad = 0, first = 0; + for (std::size_t i = 0; i < size; ++i) + if (actual[i] != host[i] && bad++ == 0) first = i; + EXPECT_EQ(bad, 0u) << bad << " bytes differ, first at " << first; + } + + // THE WIRE HALF, and the one case that is about the transport rather than the pixels: + // under a transport a pack-buffer read posts NO reply-slot record and marks the buffer + // GPU-written, once per read. MOBILEGL_IPC_PBO_READBACK_SYNC=1 puts the reply form back, + // and this case must then fail on both counters - that is its red-once, recorded in + // docs/Disaggregated/notes/p9/. + TEST_F(PackBufferReadbackScenario, APackBufferReadPostsNoReplyAndMarksTheBuffer) { + if (!Ready()) return; + const std::string split = SplitRuntimeSkipReason(); + if (!split.empty()) GTEST_SKIP() << split; + constexpr int kWidth = 24, kHeight = 12; + AttachPatternRgba8(kWidth, kHeight); + if (HasFatalFailure()) return; + NewBuffer(GL_PIXEL_PACK_BUFFER, static_cast(kWidth) * kHeight * 4, kFill); + // A warm-up read, so every state record the read's fill has to publish once is out + // of the way before the counted window opens. + glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)); + constexpr int kReads = 4; + const SplitRuntimeState before = PeekSplitRuntime(); + for (int i = 0; i < kReads; ++i) glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + glGetTexImage(GL_TEXTURE_2D, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + const SplitRuntimeState after = PeekSplitRuntime(); + ASSERT_EQ(FirstGLError(), static_cast(GL_NO_ERROR)); + EXPECT_EQ(after.replyPostings - before.replyPostings, 0u) + << "a read into a pack buffer published a reply-slot record: it took the reply form " + "(MOBILEGL_IPC_PBO_READBACK_SYNC=1 does this on purpose)"; + EXPECT_EQ(after.packBufferReadbackMarks - before.packBufferReadbackMarks, + static_cast(kReads + 1)) + << "every pack-buffer read marks its buffer GPU-written exactly once"; + // And the bytes are still right after all of that. + std::vector actual(static_cast(kWidth) * kHeight * 4, 0); + glGetBufferSubData(GL_PIXEL_PACK_BUFFER, 0, static_cast(actual.size()), actual.data()); + const auto corner = PatternRgba8(kWidth - 1, kHeight - 1, 0); + EXPECT_EQ(std::memcmp(actual.data() + actual.size() - 4, corner.data(), 4), 0); + } + + } // namespace +} // namespace MGITest diff --git a/MobileGL/MG_Pipe/FieldOwnership.def b/MobileGL/MG_Pipe/FieldOwnership.def index 4aaddde4..136e878c 100644 --- a/MobileGL/MG_Pipe/FieldOwnership.def +++ b/MobileGL/MG_Pipe/FieldOwnership.def @@ -163,7 +163,10 @@ X(QueryDestroy, DeleteBackendQuery) \ X(QueryCounter, QueryCounterTimestamp) \ X(QueryTimestamp, GetGpuTimestampNs) \ - X(DeleteStreamOutput, DeleteTransformFeedback) + X(DeleteStreamOutput, DeleteTransformFeedback) \ + /* P9 (CONTRACT-P9.md §1): the pack-buffer halves stamp as the verb they reproduce. */ \ + X(ReadPixelsToBuffer, ReadPixels) \ + X(GetTextureImageToBuffer, GetTextureImage) // X(Op, Why) - verb-shaped calls that are deliberately NOT stamp points. // diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 154f8a8e..73a4aafb 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -1625,6 +1625,38 @@ namespace MobileGL::MG_Pipe { }; MGP_ASSERT_POD(MGPReadbackInfo, 64); + // P9 (W1, MG_Remote/CONTRACT-P9.md §1): read_pixels_to_buffer / get_texture_image_to_buffer, + // the two readbacks whose destination is a bound GL_PIXEL_PACK_BUFFER. The destination is + // the pack buffer's RESOURCE HANDLE (CONTRACT-P5 row 23: "a PBO destination is a resource + // handle and needs no Seg field"), so nothing comes back and the client does not wait: the + // server reads exactly the tight pixels the reply-slot form would have answered and writes + // them into Dst row by row through the applier's ordinary buffer-write gate, and the client + // marks Dst GPU-written (GpuWritePending.h row 4). A later CPU read of Dst reaches the bytes + // through resource_readback, like any other buffer the GPU wrote. + // + // Src the reply form's record, unchanged: the box, format, type, target and + // level; Src.DstOffset / Src.DstSize are 0 / the tight w*h*d*bpp extent + // and are checked, not used for placement. + // Dst the pack buffer. + // DstBase byte offset in Dst of the first texel of the first row of the first image: + // the application's offset plus GL_PACK_SKIP_{PIXELS,ROWS,IMAGES} already + // applied, because the layout arithmetic is the client's (ReadbackLayout). + // RowStride bytes between two rows in Dst (GL_PACK_ROW_LENGTH and GL_PACK_ALIGNMENT + // applied). Never smaller than a tight row. + // ImageStride bytes between two images in Dst (GL_PACK_IMAGE_HEIGHT applied). Only read + // when Src.Box.D > 1. + // SwapGroup GL_PACK_SWAP_BYTES as the width of the byte group to reverse (2, 4 or 8), + // 0 for no swap - the client's ApplyReadbackByteSwap rule, precomputed there so + // the server does not re-derive the type table. + struct MGPReadbackToBuffer { + MGPReadbackInfo Src; + MGPipeHandle Dst; + Uint64 DstBase, RowStride, ImageStride; + Uint32 SwapGroup; + Uint32 Pad0; + }; + MGP_ASSERT_POD(MGPReadbackToBuffer, 104); + // --------------------------------------------------------------------------------- // Commands // --------------------------------------------------------------------------------- diff --git a/MobileGL/MG_Pipe/PipeCalls.def b/MobileGL/MG_Pipe/PipeCalls.def index a133e143..d763b1a3 100644 --- a/MobileGL/MG_Pipe/PipeCalls.def +++ b/MobileGL/MG_Pipe/PipeCalls.def @@ -140,9 +140,14 @@ // this one describes what happened to it since. It lands with a NULL ROUTE and a sink that // refuses it by name until package pg wires the emitter, exactly as set_shader_buffers has // sat catalogued-and-dead since P4a. 80 unique records. +// - delete_stream_output (opcode 81) took the XFB object's deletion across; 81 records. +// - P9 (MG_Remote/CONTRACT-P9.md §1) appended TWO more (opcodes 82..83): +// read_pixels_to_buffer and get_texture_image_to_buffer, the pack-buffer halves of the two +// readbacks, answered by nothing (ID-57's fire-and-forget). Like P5b's five they reach +// WireVerbSink only and have no monolith producer. 83 unique records. // --------------------------------------------------------------------------------------- -#define MGP_CALL_LIST_DOCUMENTED_COUNT 81 +#define MGP_CALL_LIST_DOCUMENTED_COUNT 83 // clang-format off #define MGP_CALL_LIST(X) \ @@ -339,7 +344,18 @@ /* ---- names only handles and values is exactly what rule F admits. ---- */ \ X(SetProgramBindings, MGPProgramBindings, kCtxState, kVarTail|kHostSpan, kWaitNone) \ /* Deletion carries the GL argument and its never-reused capture identity. */ \ - X(DeleteStreamOutput, MGPStreamOutputBind, kCtxVerb, kNone, kWaitNone) + X(DeleteStreamOutput, MGPStreamOutputBind, kCtxVerb, kNone, kWaitNone) \ + /* ---- APPENDED BY P9 (MG_Remote/CONTRACT-P9.md §1), opcodes 82..83: the pack-buffer ---- */ \ + /* ---- halves of read_pixels and get_texture_image. The reply-slot rows above keep every ---- */ \ + /* ---- read into client memory; a read whose destination is a bound PIXEL_PACK_BUFFER ---- */ \ + /* ---- crosses as one of these instead, names the buffer by handle, and is answered by ---- */ \ + /* ---- nothing - the server lands the rows in the buffer and the client marks it ---- */ \ + /* ---- GPU-written (ID-57's fire-and-forget). kWaitNone: the apply reads only server ---- */ \ + /* ---- state and the record, and no BARRIER_PULLED row is left for it to read. Neither ---- */ \ + /* ---- has an MGPipeApply* entry point or a monolith producer: monolith reads into its ---- */ \ + /* ---- driver PBO inside the backend's own ReadPixels, as before (G1). ---- */ \ + X(ReadPixelsToBuffer, MGPReadbackToBuffer, kCtxVerb, kNone, kWaitNone) \ + X(GetTextureImageToBuffer, MGPReadbackToBuffer, kCtxObject, kNone, kWaitNone) // clang-format on // Explicitly NOT migrated (plan 4.4.6 / appendix A "explicit deletions"): diff --git a/MobileGL/MG_Pipe/PipeFields.def b/MobileGL/MG_Pipe/PipeFields.def index 70b26d92..66484d92 100644 --- a/MobileGL/MG_Pipe/PipeFields.def +++ b/MobileGL/MG_Pipe/PipeFields.def @@ -230,6 +230,10 @@ #define MGP_FIELDS_MGPReadbackInfo(F) \ F(Res) F(Box) F(Format) F(Type) F(Target) F(Level) F(DstOffset) F(DstSize) +// P9 (CONTRACT-P9.md §1): the pack-buffer readback. Src is one field, as MGPSubData's box is. +#define MGP_FIELDS_MGPReadbackToBuffer(F) \ + F(Src) F(Dst) F(DstBase) F(RowStride) F(ImageStride) F(SwapGroup) + #define MGP_FIELDS_MGPDrawInfo(F) \ F(Mode) F(IndexSize) F(Flags) F(InstanceCount) F(StartInstance) F(RestartIndex) F(DrawIdOffset) \ F(IndexResource) F(MinIndex) F(MaxIndex) F(XfbCpuCapturedVertices) F(NumDraws) @@ -412,7 +416,8 @@ P(MGPStreamOutputTargets) P(MGPGlobalConstants) P(MGPAttribValue) P(MGPVertexAttribDefaults) \ P(MGPPixelPackState) P(MGPPatchState) P(ResidualValueBlock) P(MGPResidualValueState) \ P(MGPSubRegion) P(MGPSubData) P(MGPSubDataComplete) P(MGPFlushRange) P(MGPReadback) \ - P(MGPCopyRegion) P(MGPBlit) P(MGPClear) P(MGPMipPlan) P(MGPReadbackInfo) P(MGPDrawInfo) \ + P(MGPCopyRegion) P(MGPBlit) P(MGPClear) P(MGPMipPlan) P(MGPReadbackInfo) P(MGPReadbackToBuffer) \ + P(MGPDrawInfo) \ P(MGPDrawRange) P(MGPDrawIndirect) P(MGPGridInfo) P(MGPMemoryBarrier) P(MGPStreamOutputBegin) \ P(MGPXfbAccounting) P(MGPStreamOutputControl) P(MGPFlush) P(MGPPresent) P(MGPSwapInterval) \ P(MGPSurfaceInfo) \ diff --git a/MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc b/MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc index c1065fa5..3bade0c6 100644 --- a/MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc +++ b/MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc @@ -301,12 +301,14 @@ constexpr MGPipeVerb MGPipeVerbForWireOp(MGPWireOp op) { case MGPWireOp::QueryCounter: return MGPipeVerb::QueryCounterTimestamp; case MGPWireOp::QueryTimestamp: return MGPipeVerb::GetGpuTimestampNs; case MGPWireOp::DeleteStreamOutput: return MGPipeVerb::DeleteTransformFeedback; + case MGPWireOp::ReadPixelsToBuffer: return MGPipeVerb::ReadPixels; + case MGPWireOp::GetTextureImageToBuffer: return MGPipeVerb::GetTextureImage; default: return MGPipeVerb::kVerbCount; } } -inline constexpr SizeT kMGPipeVerbBoundaryOpCount = 32; +inline constexpr SizeT kMGPipeVerbBoundaryOpCount = 34; inline constexpr SizeT kMGPipeVerbBoundaryExemptCount = 3; // ---- THE §7 ALLOWLIST, DERIVED RATHER THAN WRITTEN (ID-116) -------------------------- diff --git a/MobileGL/MG_Pipe/generated/PipeTables.inc b/MobileGL/MG_Pipe/generated/PipeTables.inc index db243efa..b3f7be69 100644 --- a/MobileGL/MG_Pipe/generated/PipeTables.inc +++ b/MobileGL/MG_Pipe/generated/PipeTables.inc @@ -29,7 +29,7 @@ struct MGPipeScreen { void (*ApplierReset)(const MGPApplierReset* payload); }; -// context: 69 calls. A null entry means the backend does not implement this +// context: 71 calls. A null entry means the backend does not implement this // call and the frontend keeps its own path (plan B section 4.1). struct MGPipeContext { void (*QueryCreate)(const MGPQueryDesc* payload); @@ -101,11 +101,13 @@ struct MGPipeContext { void (*SetContextValues)(const MGPContextValues* payload); void (*SetProgramBindings)(const MGPProgramBindings* payload, const void* varTail, Uint32 varTailCount); void (*DeleteStreamOutput)(const MGPStreamOutputBind* payload); + void (*ReadPixelsToBuffer)(const MGPReadbackToBuffer* payload); + void (*GetTextureImageToBuffer)(const MGPReadbackToBuffer* payload); }; inline constexpr SizeT kMGPipeScreenCallCount = 12; -inline constexpr SizeT kMGPipeContextCallCount = 69; -inline constexpr SizeT kMGPipeCallCount = 81; +inline constexpr SizeT kMGPipeContextCallCount = 71; +inline constexpr SizeT kMGPipeCallCount = 83; // A table that is not exactly its call count of function pointers has grown a // member that no generator knows about. diff --git a/MobileGL/MG_Pipe/generated/PipeThunks.inc b/MobileGL/MG_Pipe/generated/PipeThunks.inc index 62ef4933..26385966 100644 --- a/MobileGL/MG_Pipe/generated/PipeThunks.inc +++ b/MobileGL/MG_Pipe/generated/PipeThunks.inc @@ -340,3 +340,11 @@ inline void MGP_SetProgramBindings(const MGPProgramBindings* payload, const void inline void MGP_DeleteStreamOutput(const MGPStreamOutputBind* payload) { gMGPipeContext.DeleteStreamOutput(payload); } + +inline void MGP_ReadPixelsToBuffer(const MGPReadbackToBuffer* payload) { + gMGPipeContext.ReadPixelsToBuffer(payload); +} + +inline void MGP_GetTextureImageToBuffer(const MGPReadbackToBuffer* payload) { + gMGPipeContext.GetTextureImageToBuffer(payload); +} diff --git a/MobileGL/MG_Pipe/generated/PipeVerify.inc b/MobileGL/MG_Pipe/generated/PipeVerify.inc index d11fbb5c..2a929f06 100644 --- a/MobileGL/MG_Pipe/generated/PipeVerify.inc +++ b/MobileGL/MG_Pipe/generated/PipeVerify.inc @@ -95,6 +95,7 @@ inline Bool MGPipeVerify(const MGPBlit& a, const MGPBlit& b, const char** outFie inline Bool MGPipeVerify(const MGPClear& a, const MGPClear& b, const char** outField); inline Bool MGPipeVerify(const MGPMipPlan& a, const MGPMipPlan& b, const char** outField); inline Bool MGPipeVerify(const MGPReadbackInfo& a, const MGPReadbackInfo& b, const char** outField); +inline Bool MGPipeVerify(const MGPReadbackToBuffer& a, const MGPReadbackToBuffer& b, const char** outField); inline Bool MGPipeVerify(const MGPDrawInfo& a, const MGPDrawInfo& b, const char** outField); inline Bool MGPipeVerify(const MGPDrawRange& a, const MGPDrawRange& b, const char** outField); inline Bool MGPipeVerify(const MGPDrawIndirect& a, const MGPDrawIndirect& b, const char** outField); @@ -230,6 +231,8 @@ struct MGPipeHasFieldVerifier : std::true_type {}; template <> struct MGPipeHasFieldVerifier : std::true_type {}; template <> +struct MGPipeHasFieldVerifier : std::true_type {}; +template <> struct MGPipeHasFieldVerifier : std::true_type {}; template <> struct MGPipeHasFieldVerifier : std::true_type {}; @@ -592,6 +595,11 @@ inline Bool MGPipeVerify(const MGPReadbackInfo& a, const MGPReadbackInfo& b, con return true; } +inline Bool MGPipeVerify(const MGPReadbackToBuffer& a, const MGPReadbackToBuffer& b, const char** outField) { + MGP_FIELDS_MGPReadbackToBuffer(MGP_VERIFY_FIELD) + return true; +} + inline Bool MGPipeVerify(const MGPDrawInfo& a, const MGPDrawInfo& b, const char** outField) { MGP_FIELDS_MGPDrawInfo(MGP_VERIFY_FIELD) return true; @@ -749,4 +757,4 @@ inline Bool MGPipeVerify(const MGPVertexBindingPointWire& a, const MGPVertexBind #undef MGP_VERIFY_FIELD -inline constexpr SizeT kMGPipeVerifiedPayloadCount = 82; +inline constexpr SizeT kMGPipeVerifiedPayloadCount = 83; diff --git a/MobileGL/MG_Pipe/generated/PipeWire.inc b/MobileGL/MG_Pipe/generated/PipeWire.inc index 8f39d32f..a25a5086 100644 --- a/MobileGL/MG_Pipe/generated/PipeWire.inc +++ b/MobileGL/MG_Pipe/generated/PipeWire.inc @@ -151,7 +151,9 @@ enum class MGPWireOp : Uint16 { SetContextValues = 79, SetProgramBindings = 80, DeleteStreamOutput = 81, - kOpCount = 82, + ReadPixelsToBuffer = 82, + GetTextureImageToBuffer = 83, + kOpCount = 84, }; // THE FLAGS, EXPORTED ONCE, INDEXED BY OPCODE (P5 R-13.4). MGPWireRecHeader::Flags is @@ -254,6 +256,8 @@ inline constexpr Uint32 kMGPipeCallFlags[static_cast(MGPWireOp::kOpCount) /* 79 SetContextValues */ static_cast(kNone), /* 80 SetProgramBindings */ static_cast(kVarTail | kHostSpan), /* 81 DeleteStreamOutput */ static_cast(kNone), + /* 82 ReadPixelsToBuffer */ static_cast(kNone), + /* 83 GetTextureImageToBuffer */ static_cast(kNone), }; static_assert(sizeof(kMGPipeCallFlags) / sizeof(kMGPipeCallFlags[0]) == static_cast(MGPWireOp::kOpCount), @@ -355,6 +359,8 @@ inline constexpr Uint8 kMGPipeCallClasses[static_cast(MGPWireOp::kOpCount /* 79 SetContextValues */ static_cast(kCtxState), /* 80 SetProgramBindings */ static_cast(kCtxState), /* 81 DeleteStreamOutput */ static_cast(kCtxVerb), + /* 82 ReadPixelsToBuffer */ static_cast(kCtxVerb), + /* 83 GetTextureImageToBuffer */ static_cast(kCtxObject), }; static_assert(sizeof(kMGPipeCallClasses) / sizeof(kMGPipeCallClasses[0]) == static_cast(MGPWireOp::kOpCount), @@ -460,6 +466,8 @@ inline constexpr Uint8 kMGPipeWaitClasses[static_cast(MGPWireOp::kOpCount /* 79 SetContextValues */ static_cast(kWaitNone), /* 80 SetProgramBindings */ static_cast(kWaitNone), /* 81 DeleteStreamOutput */ static_cast(kWaitNone), + /* 82 ReadPixelsToBuffer */ static_cast(kWaitNone), + /* 83 GetTextureImageToBuffer */ static_cast(kWaitNone), }; static_assert(sizeof(kMGPipeWaitClasses) / sizeof(kMGPipeWaitClasses[0]) == static_cast(MGPWireOp::kOpCount), @@ -1174,6 +1182,22 @@ static_assert(sizeof(MGPWireRec_DeleteStreamOutput) == ((sizeof(MGPWireRecHeader) + sizeof(MGPStreamOutputBind) + 7u) & ~SizeT(7u)), "MGPWireRec_DeleteStreamOutput gained padding; the wire format moved"); +struct alignas(8) MGPWireRec_ReadPixelsToBuffer { + MGPWireRecHeader Header; + MGPReadbackToBuffer Payload; +}; +static_assert(sizeof(MGPWireRec_ReadPixelsToBuffer) == + ((sizeof(MGPWireRecHeader) + sizeof(MGPReadbackToBuffer) + 7u) & ~SizeT(7u)), + "MGPWireRec_ReadPixelsToBuffer gained padding; the wire format moved"); + +struct alignas(8) MGPWireRec_GetTextureImageToBuffer { + MGPWireRecHeader Header; + MGPReadbackToBuffer Payload; +}; +static_assert(sizeof(MGPWireRec_GetTextureImageToBuffer) == + ((sizeof(MGPWireRecHeader) + sizeof(MGPReadbackToBuffer) + 7u) & ~SizeT(7u)), + "MGPWireRec_GetTextureImageToBuffer gained padding; the wire format moved"); + [[noreturn]] inline void MGPipeWireProtocolFatal(const char* call, Uint64 size, Uint64 remaining) { MGLOG_F("MGPipe: protocol corruption applying %s: size=%llu remaining=%llu", call, static_cast(size), static_cast(remaining)); @@ -1466,6 +1490,12 @@ inline Bool MGPipeApplyWireRecord(MGPWireOp op, const void* record, Uint64 size, case MGPWireOp::DeleteStreamOutput: MGP_WIRE_CHECK_BOUNDS(MGPWireRec_DeleteStreamOutput, "DeleteStreamOutput"); break; + case MGPWireOp::ReadPixelsToBuffer: + MGP_WIRE_CHECK_BOUNDS(MGPWireRec_ReadPixelsToBuffer, "ReadPixelsToBuffer"); + break; + case MGPWireOp::GetTextureImageToBuffer: + MGP_WIRE_CHECK_BOUNDS(MGPWireRec_GetTextureImageToBuffer, "GetTextureImageToBuffer"); + break; case MGPWireOp::kInvalid: case MGPWireOp::kOpCount: default: diff --git a/MobileGL/MG_Pipe/generated/PipeWireLayout.inc b/MobileGL/MG_Pipe/generated/PipeWireLayout.inc index 88321522..12f5c8bc 100644 --- a/MobileGL/MG_Pipe/generated/PipeWireLayout.inc +++ b/MobileGL/MG_Pipe/generated/PipeWireLayout.inc @@ -329,6 +329,13 @@ inline constexpr WireLayoutMember kMGPipeWireLayoutMembers[] = { {"MGPReadbackInfo.Level", offsetof(MGPReadbackInfo, Level), sizeof(((MGPReadbackInfo*)nullptr)->Level)}, {"MGPReadbackInfo.DstOffset", offsetof(MGPReadbackInfo, DstOffset), sizeof(((MGPReadbackInfo*)nullptr)->DstOffset)}, {"MGPReadbackInfo.DstSize", offsetof(MGPReadbackInfo, DstSize), sizeof(((MGPReadbackInfo*)nullptr)->DstSize)}, + {"MGPReadbackToBuffer", 0, sizeof(MGPReadbackToBuffer)}, + {"MGPReadbackToBuffer.Src", offsetof(MGPReadbackToBuffer, Src), sizeof(((MGPReadbackToBuffer*)nullptr)->Src)}, + {"MGPReadbackToBuffer.Dst", offsetof(MGPReadbackToBuffer, Dst), sizeof(((MGPReadbackToBuffer*)nullptr)->Dst)}, + {"MGPReadbackToBuffer.DstBase", offsetof(MGPReadbackToBuffer, DstBase), sizeof(((MGPReadbackToBuffer*)nullptr)->DstBase)}, + {"MGPReadbackToBuffer.RowStride", offsetof(MGPReadbackToBuffer, RowStride), sizeof(((MGPReadbackToBuffer*)nullptr)->RowStride)}, + {"MGPReadbackToBuffer.ImageStride", offsetof(MGPReadbackToBuffer, ImageStride), sizeof(((MGPReadbackToBuffer*)nullptr)->ImageStride)}, + {"MGPReadbackToBuffer.SwapGroup", offsetof(MGPReadbackToBuffer, SwapGroup), sizeof(((MGPReadbackToBuffer*)nullptr)->SwapGroup)}, {"MGPDrawInfo", 0, sizeof(MGPDrawInfo)}, {"MGPDrawInfo.Mode", offsetof(MGPDrawInfo, Mode), sizeof(((MGPDrawInfo*)nullptr)->Mode)}, {"MGPDrawInfo.IndexSize", offsetof(MGPDrawInfo, IndexSize), sizeof(((MGPDrawInfo*)nullptr)->IndexSize)}, @@ -753,4 +760,6 @@ inline constexpr WireCallLayout kMGPipeWireCallLayouts[] = { {"SetContextValues", "MGPContextValues", static_cast(MGPWireOp::SetContextValues), static_cast(kNone), static_cast(kWaitNone)}, {"SetProgramBindings", "MGPProgramBindings", static_cast(MGPWireOp::SetProgramBindings), static_cast(kVarTail | kHostSpan), static_cast(kWaitNone)}, {"DeleteStreamOutput", "MGPStreamOutputBind", static_cast(MGPWireOp::DeleteStreamOutput), static_cast(kNone), static_cast(kWaitNone)}, + {"ReadPixelsToBuffer", "MGPReadbackToBuffer", static_cast(MGPWireOp::ReadPixelsToBuffer), static_cast(kNone), static_cast(kWaitNone)}, + {"GetTextureImageToBuffer", "MGPReadbackToBuffer", static_cast(MGPWireOp::GetTextureImageToBuffer), static_cast(kNone), static_cast(kWaitNone)}, }; diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 75b796f0..799cff75 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -861,15 +861,23 @@ namespace MobileGL::MG_Remote::Client { ReadbackBytesPerPixel(format, type); } - void ApplyReadbackByteSwap(void* pixels, Uint64 bytes, GLenum type, - const PixelStoreParameters& pack) { - if (!pack.SwapBytes || pixels == nullptr) return; + // GL_PACK_SWAP_BYTES as the width of the byte group it reverses for `type`, 0 when there + // is nothing to reverse. One rule for both places a swap happens: the reply form swaps + // here, and the pack-buffer form (P9) carries the number to the server, which swaps there. + Uint32 ReadbackSwapGroup(GLenum type, const PixelStoreParameters& pack) { + if (!pack.SwapBytes) return 0; const auto dataType = MG_Util::ConvertGLEnumToTexturePixelDataType(type); SizeT group = MG_Util::GetSizedTexturePixelDataTypeSize(dataType); if (group == 0) group = MG_Util::GetBaseTexturePixelDataTypeSize(dataType); // This packed depth/stencil type contains two independent 32-bit words. if (type == GL_FLOAT_32_UNSIGNED_INT_24_8_REV) group = 4; - if (group <= 1) return; + return group <= 1 ? 0 : static_cast(group); + } + + void ApplyReadbackByteSwap(void* pixels, Uint64 bytes, GLenum type, + const PixelStoreParameters& pack) { + const SizeT group = ReadbackSwapGroup(type, pack); + if (group == 0 || pixels == nullptr) return; auto* data = static_cast(pixels); for (Uint64 at = 0; at + group <= bytes; at += group) { for (SizeT i = 0; i < group / 2; ++i) { @@ -963,6 +971,56 @@ namespace MobileGL::MG_Remote::Client { return; } + // One band's source record, the same for both destinations below. + const auto bandInfo = [&](const ReadbackBand& band, Uint64 bandBytes) { + MG_Pipe::MGPReadbackInfo info{}; + info.Res = MG_Pipe::kMGPipeNullHandle; // "the bound read surface answers" + info.Box = MG_Pipe::MGPBox{ReadbackBandOrigin(x, band.FirstColumn), + ReadbackBandOrigin(y, band.FirstRow), 0, + static_cast(band.Columns), + static_cast(band.Rows), 1}; + info.Format = static_cast(format); + info.Type = static_cast(type); + info.Target = 0; + info.Level = 0; + info.DstOffset = 0; + info.DstSize = bandBytes; + return info; + }; + + // P9 (W1, CONTRACT-P9.md §1): A PACK BUFFER IS ANSWERED BY NOTHING. Each band crosses + // as read_pixels_to_buffer naming the buffer and the band's place in it - the + // layout above, per band - and the client neither waits nor copies: the server lands + // the rows, and the mark (taken first, like every producer row's) is what sends the + // next CPU read of the buffer through resource_readback. The bands are the reply + // form's for the SERVER's sake, not the wire's: the server reads each one into its + // scratch before landing it, and the reply cap is the bound it already enforces on + // that scratch (PH-3). The monolith arm is untouched - it maps the driver PBO back + // inside its own ReadPixels, as it always has. + if (pbo) { + const MG_Pipe::MGPipeHandle target = PackBufferReadbackTarget(pbo, pboOffset + layout.End); + if (!MG_Pipe::MGPipeHandleIsNull(target)) { + MarkReadPixelsPackBuffer(); + const Uint32 swapGroup = ReadbackSwapGroup(type, pack); + ForEachReadbackBand(static_cast(width), static_cast(height), plan, + [&](const ReadbackBand& band) { + MG_Pipe::MGPReadbackToBuffer record{}; + record.Src = bandInfo(band, TightReadbackBytes(static_cast(band.Columns), + static_cast(band.Rows), + format, type)); + record.Dst = target; + record.DstBase = pboOffset + layout.Start + band.FirstRow * layout.RowStride + + band.FirstColumn * bytesPerPixel; + record.RowStride = layout.RowStride; + record.ImageStride = 0; + record.SwapGroup = swapGroup; + session.EmitAndWait(MG_Pipe::MGPWireOp::ReadPixelsToBuffer, &record, + sizeof(record), nullptr, 0, nullptr, 0, nullptr); + }); + return; + } + } + // THE COMMON CASE KEEPS THE ZERO-COPY. A neutral pack state means the destination // layout IS the tight layout, so each band's reply lands straight in the // application's buffer at the band's tight offset (whole-width bands and single-row @@ -983,18 +1041,7 @@ namespace MobileGL::MG_Remote::Client { const Uint64 bandBytes = TightReadbackBytes(static_cast(band.Columns), static_cast(band.Rows), format, type); - MG_Pipe::MGPReadbackInfo info{}; - info.Res = MG_Pipe::kMGPipeNullHandle; // "the bound read surface answers" - info.Box = MG_Pipe::MGPBox{ReadbackBandOrigin(x, band.FirstColumn), - ReadbackBandOrigin(y, band.FirstRow), 0, - static_cast(band.Columns), - static_cast(band.Rows), 1}; - info.Format = static_cast(format); - info.Type = static_cast(type); - info.Target = 0; - info.Level = 0; - info.DstOffset = 0; - info.DstSize = bandBytes; + MG_Pipe::MGPReadbackInfo info = bandInfo(band, bandBytes); // CHECKED BEFORE THE EMISSION, not after the answer (ID-47), through S1's // helper, for every record. The plan makes it true by construction; it stays diff --git a/MobileGL/MG_Remote/Client/GpuWritePending.cpp b/MobileGL/MG_Remote/Client/GpuWritePending.cpp index 947fe6f0..7a417646 100644 --- a/MobileGL/MG_Remote/Client/GpuWritePending.cpp +++ b/MobileGL/MG_Remote/Client/GpuWritePending.cpp @@ -253,6 +253,19 @@ namespace MobileGL::MG_Remote::Client { // UnmigratedVerbFatal), and it is what gives the hole a red spelling before the // transport arrives. if (!buffer.HasOutstandingGpuWrite()) return; + // P9 F1 (CONTRACT-P9.md §4): THE DEVICE WENT AWAY WITH THE WRITE STILL OWED. A lost device + // DECLINES every record, so the readback that was to bring the GPU's bytes back was never + // sent - and the shadow the caller is about to read holds whatever the buffer held BEFORE + // the write (for a pack-buffer read, the bytes before the pixels). Returning it would be the + // one outcome the reverse channel exists to prevent: stale bytes that look like a + // successful read. It is the reply form's ReadbackDeclined, arriving one call later. + if (ClientSession::DeviceLost()) { + SessionFail(MGFatalFamily::ReadbackDeclined, "MGPipe: Fatal{ReadbackDeclined, \"buffer-writeback\"} - buffer %u " + "still owes a GPU write (a pack-buffer read or a shader store) and the device is " + "lost, so its readback was declined; its shadow holds the bytes from before the " + "write and is not returned as the GPU's", + buffer.GetExternalIndex()); + } SessionFail(MGFatalFamily::UnimplementedWritebackWait, "MGPipe: Fatal{UnimplementedWritebackWait} - a ClientSession is active and buffer %u " "still has an outstanding GPU write after its readback was emitted. The wait is " "ClientSession::EmitAndWait's (R-3: the reply slot id IS the record seq); P5 package " diff --git a/MobileGL/MG_Remote/Client/TextureReadbackEmit.inc b/MobileGL/MG_Remote/Client/TextureReadbackEmit.inc index df954d52..05d0dab9 100644 --- a/MobileGL/MG_Remote/Client/TextureReadbackEmit.inc +++ b/MobileGL/MG_Remote/Client/TextureReadbackEmit.inc @@ -1,6 +1,9 @@ // Included in EmitTables.cpp's client-only anonymous namespace. // The server returns owned tight pixels. Only the client interprets PACK state -// and application pointers; PBO rows are ordinary buffer uploads after the reply. +// and application pointers. A pack-buffer destination (P9) is answered by nothing: the +// client sends the layout it computed and the server lands the rows in the buffer; the +// reply form's "PBO rows are ordinary buffer uploads after the reply" remains the arm for +// a buffer PackBufferReadbackTarget refuses. struct ReadbackLayout { Uint64 RowBytes = 0, RowStride = 0, ImageStride = 0, Start = 0, End = 0, Tight = 0; }; @@ -41,6 +44,30 @@ "Pixel packing exceeds the destination or its addressable range.")); } + // P9 (W1, MG_Remote/CONTRACT-P9.md §1): MAY A READ INTO THIS PACK BUFFER GO WITHOUT A + // REPLY? The answer is the buffer's handle when it may - the read then crosses as + // read_pixels_to_buffer / get_texture_image_to_buffer, lands server-side, and the buffer + // is marked GPU-written - and the null handle when the reply-slot form has to carry it. + // `end` is the last byte past the application's offset the read touches. + // + // Each refusal is a case the mark cannot keep its promise in, not a preference: + // - MOBILEGL_IPC_PBO_READBACK_SYNC=1, the A/B and red-once arm; + // - no readback route (BufferWritebackIsReachable: no size, or the resource family + // is not on the wire), so SyncGpuWrites would clear the mark with nothing fetched; + // - a MAPPED buffer. Only a persistent mapping can be live across a read, and its + // reader takes the bytes straight from the pointer through no call this side sees: + // no SyncGpuWrites would ever run for it, and the pixels would never arrive; + // - a range MGPSubData cannot name (a buffer offset stops at 2^31-1); + // - a buffer that never crossed: it has no handle for the server to write into, and + // the reply form's UploadSubData is what creates it. + MG_Pipe::MGPipeHandle PackBufferReadbackTarget(const SharedPtr& pbo, + Uint64 end) { + if (!pbo || MG_Config::Ipc.PboReadbackSync != 0) return MG_Pipe::kMGPipeNullHandle; + if (!BufferWritebackIsReachable(*pbo) || pbo->IsMapped()) return MG_Pipe::kMGPipeNullHandle; + if (end > 0x7FFFFFFFull) return MG_Pipe::kMGPipeNullHandle; + return MG_Pipe::MGPipeSlots().FindByLifetimeId(MG_Pipe::MGPipeKind::Buffer, pbo->GetLifetimeId()); + } + void EmitGetTextureImage(const SharedPtr& texture, TextureUploadTarget uploadTarget, GLint level, GLenum format, GLenum type, GLsizei bufSize, void* pixels) { @@ -80,6 +107,29 @@ info.Type = type; info.Target = wholeCube ? GL_TEXTURE_CUBE_MAP : MG_Util::ConvertTextureUploadTargetToGLEnum(uploadTarget); info.Level = static_cast(level); + // P9 (W1): into a pack buffer, ONE record and no wait. The whole level crosses at + // once - there is no reply slot to fit, and the server reads the whole level for + // every window of the reply form anyway - and the rows land in the buffer at the + // layout computed above. The mark goes first, like every producer row's: the set + // describes the work the record is about to start. + if (pbo) { + const MG_Pipe::MGPipeHandle target = PackBufferReadbackTarget(pbo, pboOffset + layout.End); + if (!MG_Pipe::MGPipeHandleIsNull(target)) { + MarkReadPixelsPackBuffer(); + MG_Pipe::MGPReadbackToBuffer record{}; + record.Src = info; + record.Src.DstOffset = 0; + record.Src.DstSize = tight; + record.Dst = target; + record.DstBase = pboOffset + layout.Start; + record.RowStride = layout.RowStride; + record.ImageStride = layout.ImageStride; + record.SwapGroup = ReadbackSwapGroup(type, pack); + session.EmitAndWait(MG_Pipe::MGPWireOp::GetTextureImageToBuffer, &record, + sizeof(record), nullptr, 0, nullptr, 0, nullptr); + return; + } + } Vector bytes(static_cast(tight)); const Uint64 capacity = session.MaxReplyBytes(); if (!capacity) Wire::WireProtocolFatal("GetTextureImage.reply", "zero reply capacity"); diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 32aba95d..52caf639 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -393,14 +393,7 @@ namespace MobileGL::MG_Remote::Server { image.DstOffset = 0; image.DstSize = tight; Vector bytes; - Bool ok = false; -#if MOBILEGL_BUILD_DISAGGREGATED - if (m_backend && m_backend->GetBackendType() == BackendType::DirectGLES) - ok = MG_Backend::DirectGLES::ReadTextureImageWire(image, bytes); - else if (m_backend && m_backend->GetBackendType() == BackendType::DirectVulkan && - MG_Backend::DirectVulkan::pVulkanRenderer) - ok = MG_Backend::DirectVulkan::pVulkanRenderer->ReadTextureImageWire(image, bytes); -#endif + const Bool ok = ReadTextureImageTight(image, bytes); if (!ok || bytes.size() != tight) { replies->PostReply(seq, Wire::ReplySink::kStatusError, nullptr, 0); return false; @@ -470,13 +463,29 @@ namespace MobileGL::MG_Remote::Server { static_cast(bpp)); } - const MG_Backend::GlobalBackendFunctionsTable* table = Table("read_pixels"); - if (table == nullptr || table->GL.ReadPixels == nullptr) { + if (!ReadBoundFramebufferTight(info, tight)) { // A well-formed call can still be unsupported by this server; return the // established decline status after the peer-controlled size has been checked. replies->PostReply(seq, Wire::ReplySink::kStatusDeclined, nullptr, 0); return false; } + + // m-7: the answer is written into the slot HERE, mid-apply, while the verb stamp is still + // up - and that is safe for exactly one reason, which is the contract's and is stated so it + // is not mistaken for luck: the client reaches a reply slot ONLY through appliedSeq + // (ReplySlot.h's ORDERING clause), never by polling the slot's own stamp, and s1's + // SessionConsumer::ApplyOne publishes appliedSeq only AFTER PipeApplier::ApplyOne has run + // LeaveApplier() and (on the joint tree) dropped the ScopedApplierEntry. So by the time the + // client is allowed to look at this slot, the apply-side gPipeInputs flag is already down. + replies->PostReply(seq, Wire::ReplySink::kStatusOk, m_readbackScratch.data(), tight); + m_readbackBytes += tight; + ++m_readbacks; + return true; + } + + Bool ServerVerbSink::ReadBoundFramebufferTight(const MG_Pipe::MGPReadbackInfo& info, Uint64 tight) { + const MG_Backend::GlobalBackendFunctionsTable* table = Table("read_pixels"); + if (table == nullptr || table->GL.ReadPixels == nullptr) return false; if (tight > m_readbackScratch.size()) { m_readbackScratch.resize(static_cast(tight)); } @@ -501,20 +510,183 @@ namespace MobileGL::MG_Remote::Server { MG_Pipe::MGPPixelPackState restorePack{}; restorePack.Pack = savedPack; MG_Pipe::MGPipeApplySetPixelPackState(restorePack); - - // m-7: the answer is written into the slot HERE, mid-apply, while the verb stamp is still - // up - and that is safe for exactly one reason, which is the contract's and is stated so it - // is not mistaken for luck: the client reaches a reply slot ONLY through appliedSeq - // (ReplySlot.h's ORDERING clause), never by polling the slot's own stamp, and s1's - // SessionConsumer::ApplyOne publishes appliedSeq only AFTER PipeApplier::ApplyOne has run - // LeaveApplier() and (on the joint tree) dropped the ScopedApplierEntry. So by the time the - // client is allowed to look at this slot, the apply-side gPipeInputs flag is already down. - replies->PostReply(seq, Wire::ReplySink::kStatusOk, m_readbackScratch.data(), tight); - m_readbackBytes += tight; - ++m_readbacks; return true; } + Bool ServerVerbSink::ReadTextureImageTight(const MG_Pipe::MGPReadbackInfo& image, Vector& bytes) { +#if MOBILEGL_BUILD_DISAGGREGATED + if (m_backend && m_backend->GetBackendType() == BackendType::DirectGLES) + return MG_Backend::DirectGLES::ReadTextureImageWire(image, bytes); + if (m_backend && m_backend->GetBackendType() == BackendType::DirectVulkan && + MG_Backend::DirectVulkan::pVulkanRenderer) + return MG_Backend::DirectVulkan::pVulkanRenderer->ReadTextureImageWire(image, bytes); +#endif + (void)image; + (void)bytes; + return false; + } + + // ---- P9 (W1, MG_Remote/CONTRACT-P9.md §1): the pack-buffer readbacks ------------------------ + // + // WHY THE ROWS GO THROUGH THE APPLIER'S BUFFER-WRITE GATE AND NOT THROUGH THE DRIVER'S OWN + // PBO READ. Monolith binds the driver buffer as GL_PIXEL_PACK_BUFFER and lets glReadPixels + // write it, then maps the whole store back into the frontend shadow (DirectGLES.cpp's + // "PBO used, mapping buffer to client memory"). A server has two stores per buffer - the + // backend's (GL buffer / VkBuffer) and R-11's staged shadow that every later drain uploads + // from - and a driver-side write reaches only the first: the next queued range drained from + // the shadow would then land OLD bytes over the pixels on Espryt, and Magma has no pack path + // in its ReadPixels at all. MGPipeApplyResourceSubData is the one write both backends already + // take from the server (Ops_H_SubData adopts into the staged shadow and queues the range; + // WriteWireBuffer orders the copy against in-flight reads), so the rows are exactly what a + // client glBufferSubData of the same bytes would have been - same serial bump, same epoch + // bump, same ordering against the records that follow. + // + // THE READ IS THE REPLY FORM'S, byte for byte (ReadBoundFramebufferTight / + // ReadTextureImageTight), so the two forms cannot disagree about what a pixel is; only the + // destination moved. The PACK layout is the client's arithmetic (ReadbackLayout) carried in + // the record, and the byte swap is applied to the tight bytes first, exactly where the client + // applied it to its reply. + // + // A FAILED READ IS LOUD. The reply form's failures reach the client as ERROR / DECLINED and + // end in a named Fatal there (RequireReadbackReplyComplete: "a blocking readback has no pixels + // to return"); this form has no reply to carry them, and a pack buffer left holding its old + // bytes is the same silently wrong answer. So the refusal latches here, under the same family + // words. + + Bool ServerVerbSink::LandReadbackInBuffer(const MG_Pipe::MGPReadbackToBuffer& record, Uint8* tight, + Uint64 bytesPerPixel, const char* call) { + const MG_Pipe::MGPBox& box = record.Src.Box; + const Uint64 rowBytes = static_cast(box.W) * bytesPerPixel; + const Uint64 rows = box.H; + const Uint64 images = box.D; + const Uint64 tightBytes = rowBytes * rows * images; // the caller proved this fits + if (record.RowStride < rowBytes || (images > 1 && record.ImageStride < record.RowStride)) { + return SessionLatch(MGFatalFamily::ProtocolCorruption, "MGPipe: Fatal{ProtocolCorruption, \"%s.layout\"} row stride %llu " + "is narrower than a tight row (%llu) or image stride %llu narrower than a row; the " + "client's ReadbackLayout never produces either", + call, static_cast(record.RowStride), + static_cast(rowBytes), + static_cast(record.ImageStride)); + } + if (record.SwapGroup != 0) { + const Uint64 group = record.SwapGroup; + if (group != 2 && group != 4 && group != 8) { + return SessionLatch(MGFatalFamily::ProtocolCorruption, "MGPipe: Fatal{ProtocolCorruption, \"%s.SwapGroup\"} %llu is " + "not a byte group GL_PACK_SWAP_BYTES can reverse (2, 4 or 8)", + call, static_cast(group)); + } + for (Uint64 at = 0; at + group <= tightBytes; at += group) { + for (Uint64 i = 0; i < group / 2; ++i) std::swap(tight[at + i], tight[at + group - 1 - i]); + } + } + + // ONE WRITE PER CONTIGUOUS RUN: the whole read when the layout is tight (the common + // case - a neutral pack state, or an image-packed read of whole images), one per image + // when only the rows are tight, one per row otherwise. The bytes between rows are the + // application's (GL_PACK_ROW_LENGTH / ALIGNMENT padding) and are never written. + const Bool rowsTight = record.RowStride == rowBytes; + const Bool imagesTight = images == 1 || record.ImageStride == rowBytes * rows; + const Uint64 runBytes = rowsTight ? (imagesTight ? tightBytes : rowBytes * rows) : rowBytes; + const Uint64 runsPerImage = rowsTight ? 1 : rows; + const Uint64 imageRuns = rowsTight && imagesTight ? 1 : images; + constexpr Uint64 kUint64Max = std::numeric_limits::max(); + for (Uint64 z = 0; z < imageRuns; ++z) { + for (Uint64 r = 0; r < runsPerImage; ++r) { + // DstBase + z * ImageStride + r * RowStride, checked: a peer can write any + // three numbers, and a wrapped offset would name bytes the range gate below + // then accepts. + if ((z != 0 && record.ImageStride > (kUint64Max - record.DstBase) / z) || + (r != 0 && record.RowStride > kUint64Max / r)) { + return Wire::WireProtocolLatch(call, "destination offset overflows"); + } + const Uint64 imageBase = record.DstBase + z * record.ImageStride; + if (r * record.RowStride > kUint64Max - imageBase) { + return Wire::WireProtocolLatch(call, "destination offset overflows"); + } + const Uint64 offset = imageBase + r * record.RowStride; + MG_Pipe::MGPSubData write{}; + write.Res = record.Dst; + write.Target = MG_Pipe::kMGPipeResourceTargetBuffer; + // HasLiveHostWrites stays 0 and that is the client's fact, not a default: the + // client takes this form only for a pack buffer it does not map (EmitTables.cpp, + // PackBufferReadbackIsAsync), so "no live host writer" is what a glBufferSubData + // of the same bytes would have said. + if (!MG_Pipe::MGPipeSetSubDataBufferRange(write, offset, runBytes)) { + return Wire::WireProtocolLatchAt(call, offset, 0x7FFFFFFFull); + } + const Uint8* bytes = tight + (z * runsPerImage + r) * runBytes; + ++m_bufferReadbackWrites; + // The acceptance is the applier's (a dead handle is a counted no-op, a range + // past the store a trip wire), so a refused write ends the landing here. + if (!MG_Pipe::MGPipeApplyResourceSubData(write, bytes, nullptr)) return false; + } + } + ++m_bufferReadbacks; + return true; + } + + Bool ServerVerbSink::OnReadPixelsToBuffer(const MG_Pipe::MGPReadbackToBuffer& record) { + const MG_Pipe::MGPReadbackInfo& info = record.Src; + const Uint64 bpp = static_cast(MG_Util::GetInputBytesPerPixel( + MG_Util::ConvertGLEnumToTextureInputFormat(static_cast(info.Format)), + MG_Util::ConvertGLEnumToTexturePixelDataType(static_cast(info.Type)))); + // The reply form's shape checks, and its PH-3 bound: the scratch is sized from the + // record, so the SERVER's own maxReplyBytes caps it exactly as it caps an answer - the + // client bands this form by the same number (PlanReadbackBands), so no correct client + // is refused by it. + if (bpp == 0 || info.Box.W == 0 || info.Box.H == 0 || info.Box.D != 1 || + info.Box.W > static_cast(std::numeric_limits::max()) || + info.Box.H > static_cast(std::numeric_limits::max())) { + return Wire::WireProtocolLatch("ReadPixelsToBuffer.extent", "invalid read box or format"); + } + constexpr Uint64 kUint64Max = std::numeric_limits::max(); + const Uint64 width = info.Box.W; + const Uint64 height = info.Box.H; + if (width > kUint64Max / height || width * height > kUint64Max / bpp) { + return Wire::WireProtocolLatch("ReadPixelsToBuffer.extent", "tight extent overflows"); + } + const Uint64 tight = width * height * bpp; + if (m_maxReplyBytes == 0 || tight > m_maxReplyBytes || + tight > static_cast(std::numeric_limits::max())) { + return Wire::WireProtocolLatchAt("ReadPixelsToBuffer.extent", tight, m_maxReplyBytes); + } + if (info.DstOffset != 0 || info.DstSize != tight) { + return Wire::WireProtocolLatchAt("ReadPixelsToBuffer.Src.DstSize", info.DstSize, tight); + } + if (!ReadBoundFramebufferTight(info, tight)) { + return SessionLatch(MGFatalFamily::ReadbackDeclined, "MGPipe: Fatal{ReadbackDeclined, \"ReadPixelsToBuffer\"} - the server " + "has no GL.ReadPixels; the pack buffer would keep bytes the application is " + "about to read as pixels, and there is no reply to carry the decline"); + } + return LandReadbackInBuffer(record, m_readbackScratch.data(), bpp, "ReadPixelsToBuffer"); + } + + Bool ServerVerbSink::OnGetTextureImageToBuffer(const MG_Pipe::MGPReadbackToBuffer& record) { + const MG_Pipe::MGPReadbackInfo& info = record.Src; + const Uint64 bpp = static_cast(MG_Util::GetInputBytesPerPixel( + MG_Util::ConvertGLEnumToTextureInputFormat(info.Format), + MG_Util::ConvertGLEnumToTexturePixelDataType(info.Type))); + if (!bpp || !info.Box.W || !info.Box.H || !info.Box.D || info.Box.X || info.Box.Y || info.Box.Z) { + return Wire::WireProtocolLatch("GetTextureImageToBuffer.extent", "invalid tight image extent"); + } + const Uint64 tight = static_cast(info.Box.W) * info.Box.H * info.Box.D * bpp; + if (tight / bpp / info.Box.W / info.Box.H != info.Box.D) { + return Wire::WireProtocolLatch("GetTextureImageToBuffer.extent", "tight extent overflows"); + } + if (info.DstOffset != 0 || info.DstSize != tight) { + return Wire::WireProtocolLatchAt("GetTextureImageToBuffer.Src.DstSize", info.DstSize, tight); + } + Vector bytes; + if (!ReadTextureImageTight(info, bytes) || bytes.size() != tight) { + return SessionLatch(MGFatalFamily::ReplyError, "MGPipe: Fatal{ReplyError, \"GetTextureImageToBuffer\"} - the level " + "could not be read (%llu of %llu bytes); the reply form answers ERROR here, and " + "this form has no reply to carry it", + static_cast(bytes.size()), + static_cast(tight)); + } + return LandReadbackInBuffer(record, bytes.data(), bpp, "GetTextureImageToBuffer"); + } + // P5b's server-side stub shape (CONTRACT-P5B.md): the same line the client's class-C table // raises (EmitTables.cpp UnmigratedVerbFatal) and the same family the census greps, so a // slot flipped on the client ahead of its server half aborts BY NAME on the apply thread diff --git a/MobileGL/MG_Remote/Server/PipeApplier.h b/MobileGL/MG_Remote/Server/PipeApplier.h index 99c57758..9fb6b81b 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.h +++ b/MobileGL/MG_Remote/Server/PipeApplier.h @@ -173,6 +173,10 @@ namespace MobileGL::MG_Remote::Server { Wire::ReplySink* replies) override; Bool OnGetTextureImage(const MG_Pipe::MGPReadbackInfo& info, Uint64 seq, Wire::ReplySink* replies) override; + // P9 (CONTRACT-P9.md §1): the pack-buffer halves. Nothing is answered; the tight pixels + // land in record.Dst through the applier's buffer-write gate. + Bool OnReadPixelsToBuffer(const MG_Pipe::MGPReadbackToBuffer& record) override; + Bool OnGetTextureImageToBuffer(const MG_Pipe::MGPReadbackToBuffer& record) override; Bool OnDrawVbo(const MG_Pipe::MGPDrawInfo& info, const MG_Pipe::MGPDrawRange* ranges, const MG_Pipe::MGHostSpan* userIndices, const MG_Pipe::MGPDrawIndirect* indirect) override; @@ -247,6 +251,10 @@ namespace MobileGL::MG_Remote::Server { Uint64 Presents() const { return m_presents; } Uint64 LastPresentSerial() const { return m_lastPresentSerial; } Uint64 ReadbackBytes() const { return m_readbackBytes; } + // P9: pack-buffer readbacks that landed in their buffer (both ops), and the buffer-write + // records they took. Readbacks() above counts only the reply-slot form. + Uint64 BufferReadbacks() const { return m_bufferReadbacks; } + Uint64 BufferReadbackWrites() const { return m_bufferReadbackWrites; } // ---- P5b package i1's tallies. R-16: a probe may not arm against a stub, and on a // split build "the scenario passed" is also what a scenario that never left the // monolith path looks like - so the lane asserts the number that only this sink can @@ -294,6 +302,18 @@ namespace MobileGL::MG_Remote::Server { private: const MG_Backend::GlobalBackendFunctionsTable* Table(const char* verb) const; + // read_pixels' read, shared by the reply form and the pack-buffer form: the bound read + // framebuffer, NEUTRAL pack state (ID-49), `tight` bytes into m_readbackScratch. False + // when this server has no GL.ReadPixels - the caller owns what that means. + Bool ReadBoundFramebufferTight(const MG_Pipe::MGPReadbackInfo& info, Uint64 tight); + // get_texture_image's read, shared the same way: the whole level into `bytes`. + Bool ReadTextureImageTight(const MG_Pipe::MGPReadbackInfo& image, Vector& bytes); + // P9: `tight` holds Src's pixels, tightly packed; land them in record.Dst at the + // record's layout through MGPipeApplyResourceSubData, one write per contiguous run. + // `call` names the op in a refusal. + Bool LandReadbackInBuffer(const MG_Pipe::MGPReadbackToBuffer& record, Uint8* tight, + Uint64 bytesPerPixel, const char* call); + struct FenceEntry { Uint32 Gen = 0; Bool Live = false; @@ -321,6 +341,8 @@ namespace MobileGL::MG_Remote::Server { Uint64 m_presents = 0; Uint64 m_lastPresentSerial = 0; Uint64 m_readbackBytes = 0; + Uint64 m_bufferReadbacks = 0; + Uint64 m_bufferReadbackWrites = 0; Uint64 m_imageBinds = 0; Uint64 m_dispatches = 0; Uint64 m_memoryBarriers = 0; diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 157e1396..45ff6250 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -261,7 +261,9 @@ namespace MobileGL::MG_Remote::Wire { X(ObjectDeath, MGPHandleOnly) \ X(SetContextValues, MGPContextValues) \ X(SetProgramBindings, MGPProgramBindings) \ - X(DeleteStreamOutput, MGPStreamOutputBind) + X(DeleteStreamOutput, MGPStreamOutputBind) \ + X(ReadPixelsToBuffer, MGPReadbackToBuffer) \ + X(GetTextureImageToBuffer, MGPReadbackToBuffer) namespace { @@ -2461,6 +2463,17 @@ namespace MobileGL::MG_Remote::Wire { case MGPWireOp::DeleteStreamOutput: return m_verbs && m_verbs->OnDeleteStreamOutput(*static_cast(payload)); + // P9 (CONTRACT-P9.md §1): the pack-buffer readbacks. No reply sink on purpose - the + // destination is a resource handle and nothing is answered - so a sink that wanted + // one could only be reading the wrong row. + case MGPWireOp::ReadPixelsToBuffer: + return m_verbs != nullptr && + m_verbs->OnReadPixelsToBuffer(*static_cast(payload)); + + case MGPWireOp::GetTextureImageToBuffer: + return m_verbs != nullptr && + m_verbs->OnGetTextureImageToBuffer(*static_cast(payload)); + case MGPWireOp::SetStorageBlockBinding: { // The block name is the ONE string on the wire (CONTRACT-P5B.md i1): a kHasBlob // record whose blob is the NUL-terminated name, Size = strlen + 1, staged whole in diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index ac5cc442..dfe2b7cf 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -543,6 +543,10 @@ namespace MobileGL::MG_Remote::Wire { virtual Bool OnGetTextureImage(const MG_Pipe::MGPReadbackInfo&, Uint64, ReplySink*) { return false; } + // P9 (CONTRACT-P9.md §1): the same two reads with a pack buffer as the destination. No + // seq and no sink - nothing is answered; the rows land in `Dst` on this side. + virtual Bool OnReadPixelsToBuffer(const MG_Pipe::MGPReadbackToBuffer&) { return false; } + virtual Bool OnGetTextureImageToBuffer(const MG_Pipe::MGPReadbackToBuffer&) { return false; } // `ranges` is info.NumDraws entries. `userIndices` is null unless the record set // kDrawHasUserIndices; `indirect` is null unless it set kDrawIsIndirect (P5b d1, // CONTRACT-P5B.md). The layout refuses a record that sets both, so at most one of the diff --git a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp index fd706946..641f4926 100644 --- a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp +++ b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp @@ -330,7 +330,10 @@ TEST_F(FieldOwnershipTest, VerbBoundaryOpsCoverEveryVerbShapedCall) { EXPECT_EQ(MGPipeVerbForWireOp(MGPWireOp::QueryCounter), MGPipeVerb::QueryCounterTimestamp); EXPECT_EQ(MGPipeVerbForWireOp(MGPWireOp::QueryTimestamp), MGPipeVerb::GetGpuTimestampNs); EXPECT_EQ(MGPipeVerbForWireOp(MGPWireOp::DeleteStreamOutput), MGPipeVerb::DeleteTransformFeedback); - EXPECT_EQ(kMGPipeVerbBoundaryOpCount, SizeT{32}); + // P9: the pack-buffer halves stamp as the verbs they reproduce. + EXPECT_EQ(MGPipeVerbForWireOp(MGPWireOp::ReadPixelsToBuffer), MGPipeVerb::ReadPixels); + EXPECT_EQ(MGPipeVerbForWireOp(MGPWireOp::GetTextureImageToBuffer), MGPipeVerb::GetTextureImage); + EXPECT_EQ(kMGPipeVerbBoundaryOpCount, SizeT{34}); EXPECT_EQ(kMGPipeVerbBoundaryExemptCount, SizeT{3}); // Present is class B (it is emitted in P5) and is STILL not a verb boundary: diff --git a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp index 3101aa94..5bde3a01 100644 --- a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp @@ -152,11 +152,12 @@ TEST(PipeCatalogue, GeneratedTablesHoldTheWholeCatalogue) { EXPECT_EQ(ClassCount(), 8u); EXPECT_EQ(ClassCount(), 13u); EXPECT_EQ(ClassCount(), 19u); - EXPECT_EQ(ClassCount(), 10u); + // + P9's get_texture_image_to_buffer (MG_Remote/CONTRACT-P9.md §1), beside get_texture_image. + EXPECT_EQ(ClassCount(), 11u); // 13 + the five P5b-appended verbs (MG_Remote/CONTRACT-P5B.md): bind_shader_image, // patch_parameter, bind_stream_output, set_storage_block_binding, - // copy_framebuffer_to_texture. - EXPECT_EQ(ClassCount(), 19u); + // copy_framebuffer_to_texture; + delete_stream_output; + P9's read_pixels_to_buffer. + EXPECT_EQ(ClassCount(), 20u); } // A row nobody has migrated is null - which is exactly what "this subsystem has not been @@ -639,7 +640,14 @@ TEST(PipeCatalogue, LateArrivalsAreAppendedWithoutRenumbering) { // exactly as set_shader_buffers has sat catalogued-and-dead since P4a. EXPECT_EQ(static_cast(MGPWireOp::SetProgramBindings), 80); EXPECT_EQ(static_cast(MGPWireOp::DeleteStreamOutput), 81); - EXPECT_EQ(static_cast(MGPWireOp::kOpCount), 82); + // P9 (MG_Remote/CONTRACT-P9.md §1) appended the two pack-buffer readbacks, by the same rule: + // opcodes 82..83, one payload, no flag at all - no reply slot, no blob, no tail. + EXPECT_EQ(static_cast(MGPWireOp::ReadPixelsToBuffer), 82); + EXPECT_EQ(static_cast(MGPWireOp::GetTextureImageToBuffer), 83); + EXPECT_EQ(static_cast(MGPWireOp::kOpCount), 84); + EXPECT_EQ(MGPipeCallFlagsFor(MGPWireOp::ReadPixelsToBuffer), static_cast(kNone)); + EXPECT_EQ(MGPipeCallFlagsFor(MGPWireOp::GetTextureImageToBuffer), static_cast(kNone)); + EXPECT_EQ(sizeof(MGPReadbackToBuffer), 104u); EXPECT_EQ(MGPipeCallFlagsFor(MGPWireOp::SetProgramBindings), static_cast(kVarTail | kHostSpan)); EXPECT_EQ(MGPipeCallFlagsFor(MGPWireOp::ApplierReset), static_cast(kNone)); @@ -792,6 +800,8 @@ TEST(PipeCatalogue, EveryRowCarriesTheWaitClassTheContractGivesIt) { MGPWireOp::SetProgramBindings, MGPWireOp::SetShaderBuffers, MGPWireOp::SetVertexBuffers, MGPWireOp::SetFramebufferState, MGPWireOp::CreateShaderState, MGPWireOp::SetDrawProgram, + // P9: the pack-buffer readbacks - the point of the two rows is that nothing waits. + MGPWireOp::ReadPixelsToBuffer, MGPWireOp::GetTextureImageToBuffer, }; for (const MGPWireOp op : noneRows) { EXPECT_EQ(MGPipeWaitClassFor(op), kWaitNone) << WireOpNameForDiag(op); @@ -1025,8 +1035,9 @@ TEST(PipeCatalogue, SixValueStructsHaveFieldLists) { // the two TAIL ELEMENT types beside it, MGPProgramSamplerUnit and MGPProgramStorageOverride // (MG_Remote/CONTRACT-P5E.md §1) - the tails are listed for the same reason MGPBufferRange // and MGPVertexAttribWire are: the comparator has to see INTO an element whose members - // include an MGHostSpan and a pad word, or it would memcmp the padding: 82. - EXPECT_EQ(kMGPipeVerifiedPayloadCount, 82u); + // include an MGHostSpan and a pad word, or it would memcmp the padding: 82. P9 appended the + // pack-buffer readbacks' one payload, MGPReadbackToBuffer (CONTRACT-P9.md §1): 83. + EXPECT_EQ(kMGPipeVerifiedPayloadCount, 83u); static_assert(MGPipeHasFieldVerifier::value); static_assert(MGPipeHasFieldVerifier::value); static_assert(MGPipeHasFieldVerifier::value); diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 8d4c9aa6..7c07b2b0 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -277,6 +277,20 @@ namespace { FramebufferCopies.push_back(copy); return true; } + // P9 (CONTRACT-P9.md §1): the pack-buffer readbacks, one recorder for both rows and + // the op each arrived through, so a swapped dispatch arm is visible. + Bool OnReadPixelsToBuffer(const MGPReadbackToBuffer& record) override { + BufferReadbacks.push_back(record); + BufferReadbackOps.push_back(MGPWireOp::ReadPixelsToBuffer); + return true; + } + Bool OnGetTextureImageToBuffer(const MGPReadbackToBuffer& record) override { + BufferReadbacks.push_back(record); + BufferReadbackOps.push_back(MGPWireOp::GetTextureImageToBuffer); + return true; + } + std::vector BufferReadbacks; + std::vector BufferReadbackOps; // ---- P5c's rows (CONTRACT-P5C.md §5): the two control records, recorded the same // way so a round trip asserts the arm and the record intact. Bool OnApplierReset(const MGPApplierReset& reset) override { @@ -1635,6 +1649,50 @@ TEST_F(PipeWireCodecTest, CopyFramebufferToTextureReachesTheSinkForBothForms) { EXPECT_EQ(wire.Sink().FramebufferCopies[1].Width, 64); } +// P9 (CONTRACT-P9.md §1): both pack-buffer rows reach their own sink arm with the record +// intact, and neither takes a reply slot - the decoder hands no seq and no sink, and the +// reply-slot readback counter the reply form moves stays at zero. +TEST_F(PipeWireCodecTest, PackBufferReadbacksReachTheirOwnArmsAndPostNoReply) { + Wire2 wire; + MGPReadbackToBuffer pixels{}; + pixels.Src.Box = MGPBox{3, 5, 0, 16, 8, 1}; + pixels.Src.Format = 0x1908; // GL_RGBA + pixels.Src.Type = 0x1401; // GL_UNSIGNED_BYTE + pixels.Src.DstSize = 16u * 8u * 4u; + pixels.Dst = MakeHandle(41); + pixels.DstBase = 12; + pixels.RowStride = 80; + pixels.SwapGroup = 0; + MGPReadbackToBuffer image = pixels; + image.Src.Res = MakeHandle(7); + image.Src.Box = MGPBox{0, 0, 0, 4, 4, 6}; + image.Src.Target = 0x8513; // GL_TEXTURE_CUBE_MAP + image.Src.DstSize = 4u * 4u * 6u * 4u; + image.ImageStride = 4u * 80u; + image.SwapGroup = 4; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::ReadPixelsToBuffer, &pixels, sizeof(pixels)), + kInvalidSeq); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::GetTextureImageToBuffer, &image, sizeof(image)), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied); + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied); + ASSERT_EQ(wire.Sink().BufferReadbacks.size(), 2u); + EXPECT_EQ(wire.Sink().BufferReadbackOps[0], MGPWireOp::ReadPixelsToBuffer); + EXPECT_EQ(wire.Sink().BufferReadbackOps[1], MGPWireOp::GetTextureImageToBuffer); + EXPECT_EQ(wire.Sink().BufferReadbacks[0].Dst.Slot, pixels.Dst.Slot); + EXPECT_EQ(wire.Sink().BufferReadbacks[0].Src.Box.X, 3); + EXPECT_EQ(wire.Sink().BufferReadbacks[0].DstBase, 12u); + EXPECT_EQ(wire.Sink().BufferReadbacks[0].RowStride, 80u); + EXPECT_EQ(wire.Sink().BufferReadbacks[1].Src.Res.Slot, image.Src.Res.Slot); + EXPECT_EQ(wire.Sink().BufferReadbacks[1].Src.Box.D, 6u); + EXPECT_EQ(wire.Sink().BufferReadbacks[1].ImageStride, 320u); + EXPECT_EQ(wire.Sink().BufferReadbacks[1].SwapGroup, 4u); + EXPECT_TRUE(wire.Sink().Readbacks.empty()) << "a pack-buffer read reached the reply-slot arm"; +} + // ===================================================================================== // CreateShaderState's seven blobs // ===================================================================================== diff --git a/MobileGL/MG_Test/Wire/RemoteClientControls.inc b/MobileGL/MG_Test/Wire/RemoteClientControls.inc index c2d8f9e7..982a0d35 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientControls.inc +++ b/MobileGL/MG_Test/Wire/RemoteClientControls.inc @@ -869,6 +869,65 @@ TEST(RemoteClientControls, DeviceLostRespecifyDoesNotDieOfTheUncarriedBytesCheck ExpectChildSuccess(child); } +// ===================================================================================== +// P9 F1 (CONTRACT-P9.md §4): the device is lost with a pack-buffer read still owed +// ===================================================================================== +// +// A glReadPixels into a pack buffer is answered by nothing (read_pixels_to_buffer) and marks the +// buffer GPU-written; the bytes arrive only when a CPU read fetches them. If the server dies in +// between, that fetch is DECLINED - and the client shadow still holds what the buffer held +// before the read. The peer below plays a server that took the read and died before landing +// it: it counts the record and writes nothing. The map that follows must end BY NAME, never +// hand the old bytes back as pixels. +// +// THE REDS, RUN ONCE: with MOBILEGL_IPC_PBO_READBACK_SYNC=1 the read takes the reply form, the +// peer sees no pack-buffer record and the child exits 151; without AwaitBufferWriteback's +// device-lost arm the child still aborts, but as Fatal{UnimplementedWritebackWait}, which names +// a missing wait rather than a lost device. +struct PackReadPeer : Codec::WireVerbSink { + std::atomic packReads{0}; + Bool OnReadPixelsToBuffer(const MGPReadbackToBuffer&) override { + packReads.fetch_add(1); + return true; + } + void Install() { + auto rc = Srv::ServerLoopInstance().RunProbeOnApplyThreadForTesting([](void* self) { + auto& decoder = Srv::ServerSessionInstance().Applier().*PeerMember(DecoderTag{}); + decoder.SetVerbSink(static_cast(self)); + return MOBILEGL_OK; + }, this); + if (rc != MOBILEGL_OK) ::_exit(150); + } +}; + +TEST(RemoteClientControls, APackReadOwedWhenTheDeviceIsLostNeverReadsBackAsTheOldBytes) { + const auto child = RunInChild([] { + StartControlSession(); + PackReadPeer peer; + peer.Install(); + MG_Config::Features.PipePush |= kMGPipeSubsystemResources; + MG_State::pGLContext = MakeUnique(); + constexpr GLenum kPixelPackBuffer = 0x88EB; + GLuint buffer = 0; + MG_Impl::GLImpl::GenBuffers(1, &buffer); + MG_Impl::GLImpl::BindBuffer(kPixelPackBuffer, buffer); + Uint8 before[64]; + std::memset(before, 0x5a, sizeof(before)); + MG_Impl::GLImpl::BufferData(kPixelPackBuffer, sizeof(before), before, 0x88EA /* STREAM_READ */); + // 4x4 RGBA8 = the whole buffer, read to offset 0. + RemoteEmitTable().GL.ReadPixels(0, 0, 4, 4, 0x1908, 0x1401, nullptr); + ClientSessionInstance().WaitForApplyToCatchUp("F1 pack read"); + if (peer.packReads.load() != 1) ::_exit(151); + ClientSessionInstance().LatchDeviceLost("RemoteClientControls: the server died with the pack read owed"); + Uint8 after[64]; + std::memset(after, 0, sizeof(after)); + MG_Impl::GLImpl::GetBufferSubData(kPixelPackBuffer, 0, sizeof(after), after); + // Reaching here means the read returned: the bytes are the ones from BEFORE the pixels. + ::_exit(152); + }); + ExpectNamedAbort(child, "Fatal{ReadbackDeclined, \"buffer-writeback\"}"); +} + // ===================================================================================== // P12 item B: the texture half stops owning a reply on the wire arm // =====================================================================================