diff --git a/.github/scripts/validate-plugin-apks.sh b/.github/scripts/validate-plugin-apks.sh index 0f62e691..1dcbf08b 100644 --- a/.github/scripts/validate-plugin-apks.sh +++ b/.github/scripts/validate-plugin-apks.sh @@ -48,6 +48,7 @@ require 'key:MOBILEGL_DISABLE_SUBGROUP' "$plugin_resource_text" 'V2 Vulkan subgr require 'key:MOBILEGL_MAGMA_R11G11B10F_FALLBACK' "$plugin_resource_text" 'V2 Magma format fallback toggle' require 'key:MOBILEGL_MAGMA_FRAMESINFLIGHT' "$plugin_resource_text" 'V2 Magma frames-in-flight setting' require 'key:MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER' "$plugin_resource_text" 'V2 sampler workaround toggle' +require 'key:MOBILEGL_COHERENT_AS_FLUSH' "$plugin_resource_text" 'V2 coherent-as-flush toggle' require 'key:MOBILEGL_USE_ANGLE' "$plugin_resource_text" 'V2 ANGLE toggle' if [[ $(grep -Fc 'fclPlugin_V2' <<<"$plugin_manifest") -ne 1 ]]; then diff --git a/.github/workflows/apk.yml b/.github/workflows/apk.yml index e6909d99..bc27f575 100644 --- a/.github/workflows/apk.yml +++ b/.github/workflows/apk.yml @@ -375,6 +375,9 @@ jobs: if [ "${{ matrix.backend.name }}" = "DirectGLES" ] && [ "${{ matrix.case.name }}" = "minecraft-1.21.4-fabric-iris-bliss-in-world" ]; then extra_retrace_args+=(--avoid-angle-llvmpipe-sampler-mipmap-min-filter) fi + if [ "${{ matrix.case.coherent_as_flush || false }}" = "true" ]; then + extra_retrace_args+=(--coherent-as-flush) + fi timeout "$(( ${{ matrix.case.timeout_seconds }} + 300 ))" sh android-plugin/trace-replay-ci.sh \ --apk-file "${apk_file}" \ --package top.mobilegl.plugin.trace \ diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 923a9ef8..51ba0ddd 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -48,6 +48,13 @@ namespace MobileGL::MG_Config { // MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER: avoid mipmap min filters in samplers, // resolves certain rendering bugs on ANGLE + llvmpipe. Bool AvoidSamplerMipmapMinFilter = false; + // MOBILEGL_COHERENT_AS_FLUSH: app-compat for engines (e.g. Flywheel) that write + // GPU-read data through persistent GL_MAP_FLUSH_EXPLICIT_BIT maps they never + // flush. Persistent FLUSH_EXPLICIT map requests are rewritten to coherent + // semantics: writes reach the backend without glFlushMappedBufferRange, and + // flush calls on rewritten maps become error-free no-ops. Non-persistent maps + // keep spec FLUSH_EXPLICIT behavior. + Bool CoherentAsFlush = false; // MOBILEGL_TRACE_SKIP_AUTODESTROY: skip teardown in the ELF destructor (Init.cpp). Bool TraceSkipAutodestroy = false; }; diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index 6c8f2bc1..1639361f 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -119,6 +119,7 @@ namespace MobileGL::MG_ConfigLoader { features.MagmaFramesInFlight = QueryEnvUint32("MOBILEGL_MAGMA_FRAMESINFLIGHT", 3, 1, 64); features.AvoidSamplerMipmapMinFilter = QueryEnvFlag("MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER"); + features.CoherentAsFlush = QueryEnvFlag("MOBILEGL_COHERENT_AS_FLUSH"); features.TraceSkipAutodestroy = QueryEnvFlag("MOBILEGL_TRACE_SKIP_AUTODESTROY"); } diff --git a/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp b/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp index a52e011c..b27a0665 100644 --- a/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp +++ b/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp @@ -8,6 +8,7 @@ #include "GL_Buffer.h" #include "Validators.h" +#include #include #include #include @@ -208,6 +209,26 @@ namespace MobileGL::MG_Impl::GLImpl { return bufferObject; } + // MOBILEGL_COHERENT_AS_FLUSH: rewrite a validated persistent FLUSH_EXPLICIT mapping + // request to coherent semantics: the map becomes coherent-persistent (eligible for + // the zero-copy backend map, otherwise synced wholesale at draw time), so its writes + // reach the GPU without glFlushMappedBufferRange; flush calls on rewritten maps are + // tolerated as no-ops by the FlushMappedBufferRange entry points. Non-persistent + // maps keep spec FLUSH_EXPLICIT behavior on purpose: the GPU cannot read them while + // mapped, so they gain nothing from the rewrite, and honoring only the app's flushed + // subranges avoids clobbering GPU-written bytes elsewhere in the mapped range. + // Runs after validation so the app's original access combination is what gets + // validated (Coherent is injected without requiring GL_MAP_COHERENT_BIT storage). + Flags ApplyCoherentAsFlush(Flags accessBits) { + if (!MG_Config::Features.CoherentAsFlush) return accessBits; + if (!(accessBits & BufferMappingAccessBit::FlushExplicit)) return accessBits; + if (!(accessBits & BufferMappingAccessBit::Persistent)) return accessBits; + accessBits = Flags( + accessBits.GetRaw() & ~static_cast(BufferMappingAccessBit::FlushExplicit)); + accessBits |= BufferMappingAccessBit::Coherent; + return accessBits; + } + Bool ValidateStorageFlags(GLbitfield flags, BufferOp op) { constexpr GLbitfield validFlags = GL_MAP_READ_BIT | GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT | GL_DYNAMIC_STORAGE_BIT | GL_CLIENT_STORAGE_BIT; @@ -465,6 +486,15 @@ namespace MobileGL::MG_Impl::GLImpl { auto mappingAccess = bufferObject->GetMappingAccess(); if (!(mappingAccess & BufferMappingAccessBit::FlushExplicit)) { + // MOBILEGL_COHERENT_AS_FLUSH strips FLUSH_EXPLICIT from persistent maps at map + // time (leaving Persistent|Coherent), so honor the app's flush on such a map as + // a no-op: its writes reach the backend without explicit flushes. Other maps + // keep the spec error. + if (MG_Config::Features.CoherentAsFlush && + (mappingAccess & BufferMappingAccessBit::Persistent) && + (mappingAccess & BufferMappingAccessBit::Coherent)) { + return; + } MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeUnique( @@ -605,7 +635,7 @@ namespace MobileGL::MG_Impl::GLImpl { } void* result = bufferObject->AcquireMemoryRange( - {static_cast(offset), static_cast(offset + length)}, accessBits); + {static_cast(offset), static_cast(offset + length)}, ApplyCoherentAsFlush(accessBits)); if (!result) { MG_State::pGLContext->RecordError( ErrorCode::OutOfMemory, @@ -1197,7 +1227,7 @@ namespace MobileGL::MG_Impl::GLImpl { } return bufferObject->AcquireMemoryRange({static_cast(offset), static_cast(offset + length)}, - accessBits); + ApplyCoherentAsFlush(accessBits)); } GLboolean UnmapNamedBuffer_State(GLuint buffer) { @@ -1239,7 +1269,15 @@ namespace MobileGL::MG_Impl::GLImpl { "Offset and length exceed mapped range.")); return; } - if (!(bufferObject->GetMappingAccess() & BufferMappingAccessBit::FlushExplicit)) { + const auto namedMappingAccess = bufferObject->GetMappingAccess(); + if (!(namedMappingAccess & BufferMappingAccessBit::FlushExplicit)) { + // See FlushMappedBufferRange_State: rewritten coherent-as-flush persistent maps + // tolerate app flushes as no-ops. + if (MG_Config::Features.CoherentAsFlush && + (namedMappingAccess & BufferMappingAccessBit::Persistent) && + (namedMappingAccess & BufferMappingAccessBit::Coherent)) { + return; + } MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeUnique("MG_Impl/GLImpl", "FlushMappedNamedBufferRange_State", diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 6a4aa3be..af5492cb 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -246,26 +246,6 @@ namespace MobileGL::MG_Impl::GLImpl { MG_Backend::gBackendFunctionsTable.GL.DrawArraysIndirect(mode, indirect); } - // Flywheel-style engines write GPU-copy descriptors into a FLUSH_EXPLICIT - // persistent map and glBindBufferRange that span as an SSBO for a compute - // dispatch WITHOUT ever flushing it (undefined per spec, but real drivers' - // persistent maps alias GPU-visible memory, so it works there). MobileGL's - // persistent maps alias the CPU shadow, so those bytes would never reach the - // GPU: push every explicitly-ranged SSBO binding of such maps down right - // before each dispatch. Whole-buffer (BindBufferBase) bindings are excluded - // on purpose — ranges the app DID flush already arrived, and re-uploading a - // 16MB staging ring per dispatch would be prohibitive. - static void SyncUnflushedMappedSsboRangesForDispatch() { - const auto pointCount = MG_State::pGLContext->GetBufferBindingPointCount(BufferTarget::ShaderStorage); - for (SizeT i = 0; i < pointCount; ++i) { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, i); - if (!point.HasExplicitRange()) continue; - const auto& bufferObject = point.GetBoundObject(); - if (!bufferObject) continue; - bufferObject->SyncMappedRangeForGpuRead(point.GetRange()); - } - } - /* @INSERTION_POINT:FUNCTION_IMPLEMENTATION@ */ void DispatchCompute(GLuint numGroupsX, GLuint numGroupsY, GLuint numGroupsZ) { auto dispatchCompute = MG_Backend::gBackendFunctionsTable.GL.DispatchCompute; @@ -276,7 +256,6 @@ namespace MobileGL::MG_Impl::GLImpl { return; } if (!ValidateCurrentProgramForCompute(__func__)) return; - SyncUnflushedMappedSsboRangesForDispatch(); dispatchCompute(numGroupsX, numGroupsY, numGroupsZ); } @@ -290,7 +269,6 @@ namespace MobileGL::MG_Impl::GLImpl { return; } if (!ValidateCurrentProgramForCompute(__func__)) return; - SyncUnflushedMappedSsboRangesForDispatch(); dispatchComputeIndirect(indirect); } diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp index 7729cf02..3aff2d5e 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp +++ b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp @@ -7,7 +7,6 @@ // End of Source File Header #include "BufferObject.h" -#include namespace MobileGL::MG_State::GLState { namespace { @@ -167,23 +166,6 @@ namespace MobileGL::MG_State::GLState { NotifySubData(m_mappedRange.start, m_mappedRange.end - m_mappedRange.start); } - void BufferObject::SyncMappedRangeForGpuRead(Range1D range) { - if (!m_isMapped) return; - // GPU-resident maps already alias GPU-visible memory; nothing to push. - if (m_resource.IsGpuResident()) return; - if (!(m_mappingAccess & BufferMappingAccessBit::Persistent)) return; - if (!(m_mappingAccess & BufferMappingAccessBit::Write)) return; - // Non-FLUSH_EXPLICIT persistent maps are already covered wholesale by - // SyncPersistentMappedRange. - if (!(m_mappingAccess & BufferMappingAccessBit::FlushExplicit)) return; - - const SizeT start = std::max(range.start, m_mappedRange.start); - const SizeT end = std::min({range.end, m_mappedRange.end, m_size}); - if (start >= end) return; - - NotifySubData(start, end - start); - } - void BufferObject::WritebackFromBackend(DataPtr data, SizeT atOffset) { MOBILEGL_ASSERT(atOffset + data.size <= m_size, "WritebackFromBackend out of bounds: atOffset (%zu) + data.size (%zu) > m_size (%zu)", atOffset, diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.h b/MobileGL/MG_State/GLState/BufferState/BufferObject.h index cf9ab1b3..a397721e 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.h +++ b/MobileGL/MG_State/GLState/BufferState/BufferObject.h @@ -139,12 +139,6 @@ namespace MobileGL { // Pushes the persistently-mapped write range to the backend; called by // backends at draw time (persistent maps mutate the shadow without API calls). void SyncPersistentMappedRange(); - // App-compat for FLUSH_EXPLICIT persistent maps: engines like Flywheel - // write copy descriptors into the mapping and bind that range as an SSBO - // without ever flushing it. Undefined per spec, but works on drivers whose - // persistent maps alias GPU-visible memory. Ours alias the CPU shadow, so - // callers push the GPU-read range down right before it is consumed. - void SyncMappedRangeForGpuRead(Range1D range); // Shadow-only write used when the backend copies GPU results (e.g. ReadPixels // into a pixel-pack buffer) back into the frontend mirror. Does not issue a // backend op: the backend storage already holds these bytes. diff --git a/MobileGL/MG_Test/Buffer/BufferTest.cpp b/MobileGL/MG_Test/Buffer/BufferTest.cpp index 77d0bb95..2220a19d 100644 --- a/MobileGL/MG_Test/Buffer/BufferTest.cpp +++ b/MobileGL/MG_Test/Buffer/BufferTest.cpp @@ -10,6 +10,7 @@ #include "Includes.h" #include "Init.h" +#include #include #include @@ -1009,3 +1010,173 @@ TEST_F(GeneralBufferTest, General_PersistentCoherentFallbackSyncsPerDrawWhenBack EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER)); g_zeroCopyMock = nullptr; } + +// --------------------------------------------------------------------------- +// MOBILEGL_COHERENT_AS_FLUSH: persistent FLUSH_EXPLICIT mapping requests are +// rewritten to coherent semantics, so apps that bind mapped ranges for GPU reads +// without ever calling glFlushMappedBufferRange (e.g. Flywheel's copy descriptors) +// still get their writes, and their flush calls stay error-free no-ops. +// Non-persistent maps keep spec FLUSH_EXPLICIT behavior. +namespace { + struct ScopedCoherentAsFlush { + ScopedCoherentAsFlush() { MG_Config::Features.CoherentAsFlush = true; } + ~ScopedCoherentAsFlush() { MG_Config::Features.CoherentAsFlush = false; } + }; +} // namespace + +TEST_F(GeneralBufferTest, General_CoherentAsFlush_NonPersistentMapKeepsExplicitFlushSemantics) { + ScopedCoherentAsFlush scopedFeature; + + GLuint buffer = CreateBoundBuffer(GL_ARRAY_BUFFER, 64, GL_STATIC_DRAW); + const char initial[16] = "0123456789ABCDE"; + BufferSubData(GL_ARRAY_BUFFER, 20, sizeof(initial), initial); + + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + + auto* mapped = + static_cast(MapBufferRange(GL_ARRAY_BUFFER, 20, 16, GL_MAP_WRITE_BIT | GL_MAP_FLUSH_EXPLICIT_BIT)); + ASSERT_NE(mapped, nullptr); + // Non-persistent maps are not rewritten: the FLUSH_EXPLICIT contract stays. + EXPECT_TRUE(bufferObject->GetMappingAccess() & BufferMappingAccessBit::FlushExplicit); + + memcpy(mapped, "PARTIAL", 8); + memcpy(mapped + 8, "WRITTEN", 8); + FlushMappedBufferRange(GL_ARRAY_BUFFER, 0, 8); // flush only the first half + EXPECT_EQ(GetError(), GL_NO_ERROR); + EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER)); + + // Only the flushed subrange reaches the shadow; the un-flushed half keeps its + // previous contents (spec behavior, unchanged by the feature). + char readBack[16] = {}; + GetBufferSubData(GL_ARRAY_BUFFER, 20, sizeof(readBack), readBack); + EXPECT_EQ(memcmp(readBack, "PARTIAL", 8), 0); + EXPECT_EQ(memcmp(readBack + 8, "89ABCDE", 8), 0); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +TEST_F(GeneralBufferTest, General_FlushWithoutExplicitBitStillErrorsWhenFeatureOff) { + GLuint buffer = CreateBoundBuffer(GL_ARRAY_BUFFER, 64, GL_STATIC_DRAW); + (void)buffer; + void* mapped = MapBufferRange(GL_ARRAY_BUFFER, 0, 16, GL_MAP_WRITE_BIT); + ASSERT_NE(mapped, nullptr); + FlushMappedBufferRange(GL_ARRAY_BUFFER, 0, 8); + EXPECT_EQ(GetError(), GL_INVALID_OPERATION); + EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER)); +} + +TEST_F(GeneralBufferTest, General_CoherentAsFlush_PersistentMapSyncsWithoutExplicitFlush) { + ScopedCoherentAsFlush scopedFeature; + + GLuint buffer = 0; + GenBuffers(1, &buffer); + BindBuffer(GL_ARRAY_BUFFER, buffer); + + GLint initial[] = {10, 20, 30, 40}; + BufferStorage(GL_ARRAY_BUFFER, sizeof(initial), initial, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + const Uint64 baseSerial = bufferObject->GetChangeSerial(); + + auto* mapped = static_cast(MapBufferRange( + GL_ARRAY_BUFFER, 0, sizeof(initial), GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_FLUSH_EXPLICIT_BIT)); + ASSERT_NE(mapped, nullptr); + const auto access = bufferObject->GetMappingAccess(); + EXPECT_FALSE(access & BufferMappingAccessBit::FlushExplicit); + EXPECT_TRUE(access & BufferMappingAccessBit::Coherent); + + mapped[1] = 200; + // Un-flushed writes are picked up by the draw-time persistent sync - the coverage + // the removed FLUSH_EXPLICIT dispatch hack (SyncMappedRangeForGpuRead) used to add. + bufferObject->SyncPersistentMappedRange(); + EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial); + EXPECT_EQ(reinterpret_cast(bufferObject->MappedData())[1], 200); + + FlushMappedBufferRange(GL_ARRAY_BUFFER, 0, sizeof(GLint)); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER)); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +TEST_F(GeneralBufferTest, General_CoherentAsFlush_DsaPersistentMapRewritesAndToleratesFlush) { + ScopedCoherentAsFlush scopedFeature; + + GLuint buffer = 0; + GenBuffers(1, &buffer); + + GLint initial[] = {1, 2, 3, 4}; + NamedBufferStorage(buffer, sizeof(initial), initial, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + const Uint64 baseSerial = bufferObject->GetChangeSerial(); + + // The DSA map entry point applies the same rewrite as the bound-target one. + auto* mapped = static_cast(MapNamedBufferRange( + buffer, 0, sizeof(initial), GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_FLUSH_EXPLICIT_BIT)); + ASSERT_NE(mapped, nullptr); + const auto access = bufferObject->GetMappingAccess(); + EXPECT_FALSE(access & BufferMappingAccessBit::FlushExplicit); + EXPECT_TRUE(access & BufferMappingAccessBit::Coherent); + + mapped[2] = 300; + bufferObject->SyncPersistentMappedRange(); + EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial); + EXPECT_EQ(reinterpret_cast(bufferObject->MappedData())[2], 300); + + // The DSA flush entry point tolerates the app's flush as a no-op too. + FlushMappedNamedBufferRange(buffer, 0, sizeof(GLint)); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + EXPECT_TRUE(UnmapNamedBuffer(buffer)); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +TEST_F(GeneralBufferTest, General_CoherentAsFlush_PersistentMapAdoptsZeroCopyBackendStorage) { + ScopedCoherentAsFlush scopedFeature; + ZeroCopyMockBackend mock; + g_zeroCopyMock = &mock; + ScopedBackendOps scopedOps(&kZeroCopyMockOps); + + GLuint buffer = 0; + GenBuffers(1, &buffer); + BindBuffer(GL_ARRAY_BUFFER, buffer); + + constexpr SizeT kCount = 256; + Vector initial(kCount, 0); + BufferStorage(GL_ARRAY_BUFFER, static_cast(kCount * sizeof(GLint)), initial.data(), + GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + // Flywheel-style map: FLUSH_EXPLICIT and never flushed. Under the feature it becomes + // coherent-persistent and takes the zero-copy path: the app writes straight into the + // backend's GPU storage, so nothing depends on flush calls. + auto* mapped = static_cast( + MapBufferRange(GL_ARRAY_BUFFER, 0, static_cast(kCount * sizeof(GLint)), + GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_FLUSH_EXPLICIT_BIT)); + ASSERT_NE(mapped, nullptr); + EXPECT_EQ(mock.acquireMapCalls, 1); + + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + EXPECT_TRUE(bufferObject->IsBackendPersistentMapped()); + EXPECT_EQ(static_cast(mapped), static_cast(mock.gpu.data())); + + mock.subDataCalls = 0; + mock.flushCalls = 0; + + mapped[7] = 1234; + bufferObject->SyncPersistentMappedRange(); // draw-time hook: nothing to transfer + EXPECT_EQ(reinterpret_cast(mock.gpu.data())[7], 1234); + EXPECT_EQ(mock.subDataCalls, 0); + EXPECT_EQ(mock.flushCalls, 0); + + EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER)); + EXPECT_EQ(mock.flushCalls, 0); + EXPECT_EQ(GetError(), GL_NO_ERROR); + g_zeroCopyMock = nullptr; +} diff --git a/README.md b/README.md index e9ec47d0..f8ac73fc 100644 --- a/README.md +++ b/README.md @@ -160,6 +160,7 @@ MobileGL supports runtime configuration via environment variables. | `MOBILEGL_MAGMA_R11G11B10F_FALLBACK` | Use Magma's R11G11B10F format fallback. | `0`, `1` | `0` | | `MOBILEGL_MAGMA_FRAMESINFLIGHT` | Set Magma frames in flight. | Integer `1`–`64` | `3` | | `MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER` | Avoid sampler mipmap minification filters. | `0`, `1` | `0` | +| `MOBILEGL_COHERENT_AS_FLUSH` | Treat persistent `GL_MAP_FLUSH_EXPLICIT_BIT` maps as coherent (app-compat for engines like Flywheel that never flush them). | `0`, `1` | `0` | | `VK_ICD_FILENAMES` | Select the Vulkan ICD used by the Vulkan loader. | Path to an ICD JSON file | Loader default | ## Notice diff --git a/android-plugin/app/build.gradle.kts b/android-plugin/app/build.gradle.kts index 0beac5ab..06f92f74 100644 --- a/android-plugin/app/build.gradle.kts +++ b/android-plugin/app/build.gradle.kts @@ -86,6 +86,7 @@ val pluginRendererConfig = buildJsonValue { toggleable("MOBILEGL_MAGMA_R11G11B10F_FALLBACK", "1", false, RendererConfig.MetaString("mobilegl_magma_r11g11b10f_fallback_title")) customizable("MOBILEGL_MAGMA_FRAMESINFLIGHT", "3", RendererConfig.MetaString("mobilegl_magma_frames_inflight_title")) toggleable("MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER", "1", false, RendererConfig.MetaString("mobilegl_avoid_sampler_mipmap_min_filter_title")) + toggleable("MOBILEGL_COHERENT_AS_FLUSH", "1", false, RendererConfig.MetaString("mobilegl_coherent_as_flush_title")) toggleable("MOBILEGL_USE_ANGLE", "1", false, RendererConfig.MetaString("mobilegl_use_angle_title")) }, minMCVer = null, diff --git a/android-plugin/app/src/main/AndroidManifest.xml b/android-plugin/app/src/main/AndroidManifest.xml index faf347a9..199d0479 100644 --- a/android-plugin/app/src/main/AndroidManifest.xml +++ b/android-plugin/app/src/main/AndroidManifest.xml @@ -43,6 +43,9 @@ + diff --git a/android-plugin/app/src/main/res/values/strings.xml b/android-plugin/app/src/main/res/values/strings.xml index 25ac0140..dee03f1d 100644 --- a/android-plugin/app/src/main/res/values/strings.xml +++ b/android-plugin/app/src/main/res/values/strings.xml @@ -7,6 +7,7 @@ Use Magma R11G11B10F fallback Magma frames in flight (1-64) Avoid sampler mipmap minification filters + Treat explicit-flush persistent maps as coherent Use ANGLE GLES libraries DirectGLES diff --git a/android-plugin/app/src/trace/AndroidManifest.xml b/android-plugin/app/src/trace/AndroidManifest.xml index 5b38ea99..f20e2313 100644 --- a/android-plugin/app/src/trace/AndroidManifest.xml +++ b/android-plugin/app/src/trace/AndroidManifest.xml @@ -33,6 +33,9 @@ + diff --git a/android-plugin/app/src/trace/cpp/trace_replay_core.cpp b/android-plugin/app/src/trace/cpp/trace_replay_core.cpp index 71d42ef2..9ecde6fc 100644 --- a/android-plugin/app/src/trace/cpp/trace_replay_core.cpp +++ b/android-plugin/app/src/trace/cpp/trace_replay_core.cpp @@ -149,6 +149,11 @@ bool LoadMobileGL(const Request& request, std::string& error) { } else { unsetenv("MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER"); } + if (request.coherentAsFlush) { + setenv("MOBILEGL_COHERENT_AS_FLUSH", "1", 1); + } else { + unsetenv("MOBILEGL_COHERENT_AS_FLUSH"); + } void* handle = dlopen(request.mobileGlLibrary.c_str(), RTLD_NOW | RTLD_GLOBAL); if (handle == nullptr) { diff --git a/android-plugin/app/src/trace/cpp/trace_replay_core.hpp b/android-plugin/app/src/trace/cpp/trace_replay_core.hpp index c417d141..177dcf1d 100644 --- a/android-plugin/app/src/trace/cpp/trace_replay_core.hpp +++ b/android-plugin/app/src/trace/cpp/trace_replay_core.hpp @@ -36,6 +36,7 @@ struct Request { bool useAngle = false; bool usePbuffer = true; bool avoidAngleLlvmpipeSamplerMipmapMinFilter = false; + bool coherentAsFlush = false; int holdMs = 0; }; diff --git a/android-plugin/app/src/trace/cpp/trace_replay_jni.cpp b/android-plugin/app/src/trace/cpp/trace_replay_jni.cpp index d0414867..9fd2a876 100644 --- a/android-plugin/app/src/trace/cpp/trace_replay_jni.cpp +++ b/android-plugin/app/src/trace/cpp/trace_replay_jni.cpp @@ -101,7 +101,8 @@ Java_top_mobilegl_plugin_trace_TraceReplayActivity_nativeRunTraceReplay(JNIEnv* jstring angleVariant, jboolean useAngle, jboolean usePbuffer, - jboolean avoidAngleLlvmpipeSamplerMipmapMinFilter) { + jboolean avoidAngleLlvmpipeSamplerMipmapMinFilter, + jboolean coherentAsFlush) { mobilegl_trace::Request request; request.tracePath = ToString(env, tracePath); request.goldenPath = ToString(env, goldenPath); @@ -126,6 +127,7 @@ Java_top_mobilegl_plugin_trace_TraceReplayActivity_nativeRunTraceReplay(JNIEnv* request.usePbuffer = usePbuffer == JNI_TRUE; request.avoidAngleLlvmpipeSamplerMipmapMinFilter = avoidAngleLlvmpipeSamplerMipmapMinFilter == JNI_TRUE; + request.coherentAsFlush = coherentAsFlush == JNI_TRUE; ScopedTraceReplayState replayState; mobilegl_trace_set_requested_size(request.width, request.height); diff --git a/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java b/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java index 1f9b4fc1..75261604 100644 --- a/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java +++ b/android-plugin/app/src/trace/java/top/mobilegl/plugin/trace/TraceReplayActivity.java @@ -113,7 +113,8 @@ public final class TraceReplayActivity extends Activity { request.angleVariant, request.useAngle, request.usePbuffer, - request.avoidAngleLlvmpipeSamplerMipmapMinFilter + request.avoidAngleLlvmpipeSamplerMipmapMinFilter, + request.coherentAsFlush ); Log.i(TAG, result.toString()); TraceReplayResult finalResult = result; @@ -143,7 +144,8 @@ public final class TraceReplayActivity extends Activity { String angleVariant, boolean useAngle, boolean usePbuffer, - boolean avoidAngleLlvmpipeSamplerMipmapMinFilter + boolean avoidAngleLlvmpipeSamplerMipmapMinFilter, + boolean coherentAsFlush ); private static final class TraceReplayRequest { @@ -166,6 +168,7 @@ public final class TraceReplayActivity extends Activity { final boolean useAngle; final boolean usePbuffer; final boolean avoidAngleLlvmpipeSamplerMipmapMinFilter; + final boolean coherentAsFlush; private TraceReplayRequest( String tracePath, @@ -186,7 +189,8 @@ public final class TraceReplayActivity extends Activity { String angleVariant, boolean useAngle, boolean usePbuffer, - boolean avoidAngleLlvmpipeSamplerMipmapMinFilter + boolean avoidAngleLlvmpipeSamplerMipmapMinFilter, + boolean coherentAsFlush ) { this.tracePath = tracePath; this.goldenPath = goldenPath; @@ -207,6 +211,7 @@ public final class TraceReplayActivity extends Activity { this.useAngle = useAngle; this.usePbuffer = usePbuffer; this.avoidAngleLlvmpipeSamplerMipmapMinFilter = avoidAngleLlvmpipeSamplerMipmapMinFilter; + this.coherentAsFlush = coherentAsFlush; } static TraceReplayRequest from(Intent intent, File filesDir, String defaultBackend) { @@ -231,7 +236,8 @@ public final class TraceReplayActivity extends Activity { readString(intent, "angle_variant", ""), intent.getBooleanExtra("use_angle", false), intent.getBooleanExtra("use_pbuffer", false), - intent.getBooleanExtra("avoid_angle_llvmpipe_sampler_mipmap_min_filter", false) + intent.getBooleanExtra("avoid_angle_llvmpipe_sampler_mipmap_min_filter", false), + intent.getBooleanExtra("coherent_as_flush", false) ); } diff --git a/android-plugin/trace-replay-ci.sh b/android-plugin/trace-replay-ci.sh index 05722a21..ce50aad1 100644 --- a/android-plugin/trace-replay-ci.sh +++ b/android-plugin/trace-replay-ci.sh @@ -28,6 +28,7 @@ Usage: --crop-height N \ [--use-pbuffer] \ [--avoid-angle-llvmpipe-sampler-mipmap-min-filter] \ + [--coherent-as-flush] \ --timeout-seconds N Set MOBILEGL_USE_ANGLE=1 to run DirectGLES replay with packaged ANGLE @@ -38,6 +39,8 @@ Set MOBILEGL_RETRACE_USE_PBUFFER=1 or pass --use-pbuffer to run DirectGLES against an offscreen EGL pbuffer instead of the Activity surface. Pass --avoid-angle-llvmpipe-sampler-mipmap-min-filter for DirectGLES traces that need ANGLE llvmpipe sampler mipmap filters downgraded to avoid driver stalls. +Pass --coherent-as-flush for traces whose engine writes persistent +GL_MAP_FLUSH_EXPLICIT_BIT maps it never flushes (MOBILEGL_COHERENT_AS_FLUSH=1). EOF } @@ -94,6 +97,7 @@ crop_width="" crop_height="" use_pbuffer=0 avoid_angle_llvmpipe_sampler_mipmap_min_filter=0 +coherent_as_flush=0 timeout_seconds="" while [ "$#" -gt 0 ]; do @@ -129,6 +133,7 @@ while [ "$#" -gt 0 ]; do avoid_angle_llvmpipe_sampler_mipmap_min_filter=1 shift 1 ;; + --coherent-as-flush) coherent_as_flush=1; shift 1 ;; --timeout-seconds) timeout_seconds="$(next_arg "$@")"; shift 2 ;; -h|--help) usage; exit 0 ;; *) die "unknown argument: $1" ;; @@ -254,6 +259,9 @@ run_retrace() { if [ "${avoid_angle_llvmpipe_sampler_mipmap_min_filter}" -eq 1 ] && [ "${backend}" = "DirectGLES" ]; then set -- "$@" --ez avoid_angle_llvmpipe_sampler_mipmap_min_filter true fi + if [ "${coherent_as_flush}" -eq 1 ]; then + set -- "$@" --ez coherent_as_flush true + fi set -- "$@" \ --es output_dir "${app_dir}/output" \ --es diff_path "${app_dir}/output/${safe_case}-diff.png" \ diff --git a/tools/trace_replay/CMakeLists.txt b/tools/trace_replay/CMakeLists.txt index cb795167..938266c5 100644 --- a/tools/trace_replay/CMakeLists.txt +++ b/tools/trace_replay/CMakeLists.txt @@ -299,7 +299,8 @@ function(add_trace_replay_test CASE_NAME BACKEND) CROP_X CROP_Y CROP_WIDTH - CROP_HEIGHT) + CROP_HEIGHT + COHERENT_AS_FLUSH) cmake_parse_arguments(TRACE_CASE "" "${oneValueArgs}" "" ${ARGN}) foreach(required TRACE_ARCHIVE GOLDEN TARGET_CALL WIDTH HEIGHT) if(NOT TRACE_CASE_${required}) @@ -343,6 +344,7 @@ function(add_trace_replay_test CASE_NAME BACKEND) -DTRACE_CROP_Y=${TRACE_CASE_CROP_Y} -DTRACE_CROP_WIDTH=${TRACE_CASE_CROP_WIDTH} -DTRACE_CROP_HEIGHT=${TRACE_CASE_CROP_HEIGHT} + -DTRACE_COHERENT_AS_FLUSH=${TRACE_CASE_COHERENT_AS_FLUSH} -DTRACE_OUTPUT_DIR=${CMAKE_CURRENT_BINARY_DIR}/${CASE_NAME}/${BACKEND} -DTRACE_ARTIFACT_DIR=${CMAKE_CURRENT_BINARY_DIR}/${CASE_NAME}/actual-images -P ${MOBILEGL_TRACE_ROOT}/run_trace_case.cmake) diff --git a/tools/trace_replay/README.md b/tools/trace_replay/README.md index 87ab8d8b..bbc34726 100644 --- a/tools/trace_replay/README.md +++ b/tools/trace_replay/README.md @@ -225,4 +225,7 @@ adb exec-out run-as $PKG cat files/trace-replay/output/openra-diff.png > openra- For the Vulkan backend, build and install `:app:assembleMagmaTraceDebug`, set `PKG=top.mobilegl.plugin.magma.trace`, and pass `--es backend DirectVulkan`. DirectGLES also renders to the Activity surface by default; pass -`--ez use_pbuffer true` to use the offscreen pbuffer path. +`--ez use_pbuffer true` to use the offscreen pbuffer path. For cases registered +with `coherent_as_flush` (Flywheel-style unflushed persistent maps, e.g. the +Create fixtures), pass `--ez coherent_as_flush true` so the replay runs with +`MOBILEGL_COHERENT_AS_FLUSH=1`. diff --git a/tools/trace_replay/run_android_retrace_local.py b/tools/trace_replay/run_android_retrace_local.py index 174e072a..84768555 100644 --- a/tools/trace_replay/run_android_retrace_local.py +++ b/tools/trace_replay/run_android_retrace_local.py @@ -176,6 +176,8 @@ def run_case(case, backend): command.append("--use-pbuffer") if backend_info["use_angle"] and case["name"] == BLISS_CASE: command.append("--avoid-angle-llvmpipe-sampler-mipmap-min-filter") + if case.get("coherent_as_flush"): + command.append("--coherent-as-flush") env = dict(**__import__("os").environ) env["PYTHON"] = "python" env["MSYS2_ARG_CONV_EXCL"] = "/data/*" diff --git a/tools/trace_replay/run_macos_window_retrace_local.py b/tools/trace_replay/run_macos_window_retrace_local.py index f5096ae8..eabc0fd3 100755 --- a/tools/trace_replay/run_macos_window_retrace_local.py +++ b/tools/trace_replay/run_macos_window_retrace_local.py @@ -361,6 +361,8 @@ def run_case(case, backend, replay_exe, mobilegl_library, vulkan_icd): str(case["crop_height"]), "--window-surface", ] + if case.get("coherent_as_flush"): + command.append("--coherent-as-flush") env = os.environ.copy() env["MOBILEGL_BACKEND_TYPE"] = backend env["MOBILEGL_MAGMA_R11G11B10F_FALLBACK"] = "1" diff --git a/tools/trace_replay/run_trace_case.cmake b/tools/trace_replay/run_trace_case.cmake index d1905c9b..ce389b47 100644 --- a/tools/trace_replay/run_trace_case.cmake +++ b/tools/trace_replay/run_trace_case.cmake @@ -23,6 +23,10 @@ set(alternate_golden_args) if(DEFINED TRACE_ALTERNATE_GOLDEN AND NOT "${TRACE_ALTERNATE_GOLDEN}" STREQUAL "") list(APPEND alternate_golden_args --alternate-golden "${TRACE_ALTERNATE_GOLDEN}") endif() +set(coherent_as_flush_args) +if(TRACE_COHERENT_AS_FLUSH) + list(APPEND coherent_as_flush_args --coherent-as-flush) +endif() if(EXISTS "${TRACE_OUTPUT_DIR}") file(REMOVE_RECURSE "${TRACE_OUTPUT_DIR}") @@ -64,6 +68,7 @@ execute_process( --crop-y "${TRACE_CROP_Y}" --crop-width "${TRACE_CROP_WIDTH}" --crop-height "${TRACE_CROP_HEIGHT}" + ${coherent_as_flush_args} RESULT_VARIABLE replay_result OUTPUT_VARIABLE replay_stdout ERROR_VARIABLE replay_stderr) diff --git a/tools/trace_replay/skills/trace-fixture-authoring.md b/tools/trace_replay/skills/trace-fixture-authoring.md index f8f46c64..ccf42a3b 100644 --- a/tools/trace_replay/skills/trace-fixture-authoring.md +++ b/tools/trace_replay/skills/trace-fixture-authoring.md @@ -121,7 +121,10 @@ depends on them - the symptom is geometry that renders live but disappears in replay. The in-tree fork shadow-tracks persistent mappings unconditionally; if a replay of `full.trace` is already missing content that the live run showed, fix capture (wrapper) first - no amount of trimming will bring the -data back, and the case must be recaptured. +data back, and the case must be recaptured. There is also a replay-side +requirement: MobileGL only forwards such never-flushed writes when +`MOBILEGL_COHERENT_AS_FLUSH=1`, so register the case with +`"coherent_as_flush": true` (see "Register the case"). ## Select target frame @@ -288,8 +291,12 @@ Values matching the `defaults` block (854x480, `trace.trace`, ssim 0.99, zero crop, 900 s timeout) may be omitted. Available per-case keys: `name`, `trace_archive`, `trace_file`, `golden`, `alternate_golden`, `target_call`, `width`, `height`, `ssim_threshold`, `crop_x/y/width/height`, -`timeout_seconds`, `ci`. Long single-frame replays of heavy in-world scenes -need a raised `timeout_seconds` (the Create fixtures use 1800). +`timeout_seconds`, `ci`, `coherent_as_flush`. Long single-frame replays of +heavy in-world scenes need a raised `timeout_seconds` (the Create fixtures +use 1800). Set `"coherent_as_flush": true` for Flywheel-style engines that +let the GPU read persistent `GL_MAP_FLUSH_EXPLICIT_BIT` mappings they never +flush (both Create fixtures need it); the case then replays with +`MOBILEGL_COHERENT_AS_FLUSH=1` on every runner. Update `tools/trace_replay/README.md` with one fixture sentence and one golden image link. diff --git a/tools/trace_replay/trace_cases.json b/tools/trace_replay/trace_cases.json index eaa407b0..9b3bc43c 100644 --- a/tools/trace_replay/trace_cases.json +++ b/tools/trace_replay/trace_cases.json @@ -263,14 +263,16 @@ "trace_archive": "minecraft-1.21.1-neoforge-create-indirect-in-world.tgz", "golden": "minecraft-1.21.1-neoforge-create-indirect-in-world.0000504631.png", "target_call": 504631, - "timeout_seconds": 1800 + "timeout_seconds": 1800, + "coherent_as_flush": true }, { "name": "minecraft-1.21.1-neoforge-create-instancing-in-world", "trace_archive": "minecraft-1.21.1-neoforge-create-instancing-in-world.tgz", "golden": "minecraft-1.21.1-neoforge-create-instancing-in-world.0000530333.png", "target_call": 530333, - "timeout_seconds": 1800 + "timeout_seconds": 1800, + "coherent_as_flush": true } ] } diff --git a/tools/trace_replay/trace_cases.py b/tools/trace_replay/trace_cases.py index e30699a2..0d225d8a 100644 --- a/tools/trace_replay/trace_cases.py +++ b/tools/trace_replay/trace_cases.py @@ -91,6 +91,7 @@ def emit_cmake(cases, fixture_root): ("CROP_Y", "crop_y", False), ("CROP_WIDTH", "crop_width", False), ("CROP_HEIGHT", "crop_height", False), + ("COHERENT_AS_FLUSH", "coherent_as_flush", False), ] for case in cases: lines.append(f"add_trace_replay_test_for_backends({cmake_quote(case['name'])}") diff --git a/tools/trace_replay/trace_replay_cli.cpp b/tools/trace_replay/trace_replay_cli.cpp index 09ddf779..97f26a2c 100644 --- a/tools/trace_replay/trace_replay_cli.cpp +++ b/tools/trace_replay/trace_replay_cli.cpp @@ -27,7 +27,8 @@ void PrintUsage(const char *argv0) { << " --crop-x N Compare crop x\n" << " --crop-y N Compare crop y\n" << " --crop-width N Compare crop width\n" - << " --crop-height N Compare crop height\n"; + << " --crop-height N Compare crop height\n" + << " --coherent-as-flush Set MOBILEGL_COHERENT_AS_FLUSH=1 for the replay\n"; } bool ReadValue(int argc, char **argv, int &index, std::string &out) { @@ -113,6 +114,8 @@ bool ParseArgs(int argc, char **argv, mobilegl_trace::Request &request) { if (!ReadInt(argc, argv, i, request.cropWidth)) return false; } else if (arg == "--crop-height") { if (!ReadInt(argc, argv, i, request.cropHeight)) return false; + } else if (arg == "--coherent-as-flush") { + request.coherentAsFlush = true; } else if (arg == "--help" || arg == "-h") { return false; } else {