mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 18:13:23 +09:00
[MG_Backend, MG_IntegrationTest] (P7): the busy predicate's comments name the floor as the guarantee and the submission term as a probe, and the term is consulted and re-stamped wherever the serial is
Review ID-P7-34, item 3. 02bb5789 presented the submission term as the fix and still carried the every-8th-drain mechanism that magma-b3.md §2.4 refutes (the probe saw one successful drain and zero boundary works across the OpenRA replay). The term is not sound alone: lastUseSubmitIndex is stamped at AcquireWireSlice, but a mid-draw flush (BindProgramUniformBuffers -> SyncWireTextureShape's preserve path -> FlushWirePendingCommandsFor- TextureUpdate) can submit the stamped index without the draw, so the stamp names S1 while the draw rides S2. The guarantee is the floor in OnSubmitsCompletedUpTo; the term is a defence and a device probe. - WriteWireBuffer's block comment, the WireBufferResource field comment, the AcquireWireSlice stamp comment, the StreamedBufferSubData CMake comment and the F1WireScenario case comment are rewritten on that footing; the drain story is gone. The drain probe's comment now says it refuted the drain as the mechanism. - WaitForWireBufferHostAccess's early return also requires the submission term complete, so a caller with the serial term idle cannot skip a wait the term demands. - ReadWireBuffer re-stamps lastUseSubmitIndex beside lastUseSerial after its wait, preserving the pending draw's reservation in both halves. All disaggregated-only code. OpenRA DirectVulkan SPLIT / SPAWN retraces ssim 1.000000, 0 px, zero MGWIRE-FLOOR lines; integration-magma-split 90/90, -spawn 69/69, -tcp 70/70.
This commit is contained in:
@@ -270,7 +270,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
}
|
||||
|
||||
Bool VkBufferManager::WaitForWireBufferHostAccess(WireBufferResource& resource) {
|
||||
if (resource.lastUseSerial <= GetCompletedSerial() && !resource.gpuWritesPending) return true;
|
||||
// B3 review: the early return must consult the same submission term WriteWireBuffer
|
||||
// does, or a caller that reaches here with the serial term idle skips the wait the
|
||||
// term would have demanded.
|
||||
if (resource.lastUseSerial <= GetCompletedSerial() && !resource.gpuWritesPending &&
|
||||
(pVulkanRenderer == nullptr || pVulkanRenderer->IsSubmitIndexComplete(resource.lastUseSubmitIndex))) {
|
||||
return true;
|
||||
}
|
||||
if (!m_copyProvider || !pVulkanRenderer) return false;
|
||||
const VkCommandBuffer commands = m_copyProvider->AcquireBufferCopyCommandBuffer();
|
||||
if (commands == VK_NULL_HANDLE) return false;
|
||||
@@ -303,27 +309,34 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
const Uint64 probeLastUse = resource->lastUseSerial;
|
||||
const Uint64 probeCompleted = GetCompletedSerial();
|
||||
|
||||
// ---- P7 wave 2 package B3: WHY THIS PREDICATE HAS TWO TERMS --------------------------
|
||||
// ---- P7 wave 2 package B3: WHAT EACH TERM OF THIS PREDICATE IS FOR -----------------
|
||||
//
|
||||
// A streamed `glBufferSubData -> draw` pair is only correct if the write is ordered
|
||||
// against every draw that still reads the bytes it overwrites. The question "is this
|
||||
// buffer still busy?" used to be answered by the FRAME SERIAL alone, and that answer is
|
||||
// a guess: GetCompletedSerial() is max(m_frameSerial - frameCount, m_completedSerialFloor)
|
||||
// and its first term is PURE COUNTING - it asserts that work from frameCount serials ago
|
||||
// has finished with no fence behind it. On the wire arm the serial is advanced not only
|
||||
// by Present but by TryDrainFrameTransients -> VkBufferManager::BeginFrame on every 8th
|
||||
// drain, which happens MID-FRAME (VulkanRenderer.cpp's drain block; ReadWireBuffer below
|
||||
// already carries a comment acknowledging "even if the idle drain advanced the frame"),
|
||||
// and package B established that mid-frame pooled-fence submissions are not covered by
|
||||
// frame.imageInFlightFence. So the counting term can say "idle" about a buffer a
|
||||
// recorded-or-in-flight draw is still reading, and the write is then applied as an
|
||||
// unsynchronised host memcpy. The draw executes and reads bytes that are not its own -
|
||||
// it renders another batch's geometry, or nothing at all. That is the OpenRA device
|
||||
// divergence: one draw's quads missing, the terrain under them intact.
|
||||
// against every draw that still reads the bytes it overwrites; a write this predicate
|
||||
// answers "idle" is applied as an unsynchronised host memcpy, and a draw still reading
|
||||
// the range then renders bytes that are not its own - silently, with no log line, no GL
|
||||
// error and no Fatal. That was the OpenRA device divergence (magma-b3.md §2).
|
||||
//
|
||||
// The second term is the fence-backed one and it is the fix. IsSubmitIndexComplete
|
||||
// polls the real fences and answers false for an index that is not even submitted yet,
|
||||
// which is exactly the state a draw being recorded right now is in.
|
||||
// THE GUARANTEE IS THE SERIAL TERM, and it is sound because of the FLOOR, not because
|
||||
// of anything here. GetCompletedSerial() is max(m_frameSerial - frameCount,
|
||||
// m_completedSerialFloor). The floor used to be raised to a retired submission's frame
|
||||
// serial even while another submission carrying the SAME serial was still executing -
|
||||
// on the wire arm a serial has two (a mid-frame FlushPendingCommands under a pooled
|
||||
// fence, then Present) - so "frame N-1 complete" was asserted on a fence nobody waited.
|
||||
// VulkanRenderer::OnSubmitsCompletedUpTo now raises it only to a serial no in-flight
|
||||
// submission still carries, and run_trace_case.cmake reds a split retrace on any
|
||||
// "MGWIRE-FLOOR unsound-serial-complete" line, which is that fix's lane.
|
||||
//
|
||||
// THE SUBMISSION TERM IS A PROBE AND A DEFENCE, NOT A SECOND GUARANTEE. It is not sound
|
||||
// on its own: lastUseSubmitIndex is stamped at AcquireWireSlice with the next submission
|
||||
// index, but the draw that acquired the slice can still be pushed into a LATER
|
||||
// submission - BindProgramUniformBuffers -> SyncWireTextureShape's preserve path ->
|
||||
// FlushWirePendingCommandsForTextureUpdate submits the stamped index without the draw -
|
||||
// so the stamp can name S1 while the draw rides S2. What it does do: with the floor
|
||||
// sound it should never be the term that says "busy" (the WBUF probe reports bySubmit
|
||||
// separately, so a device that shows one has found a floor hole), and it is the only
|
||||
// thing the MGITEST_MAGMA_FORCE_STALE_BUFFER_SERIAL knob leaves standing, which is what
|
||||
// the StaleSerial. entries pin.
|
||||
Bool busyBySerial = resource->lastUseSerial > GetCompletedSerial();
|
||||
if (busyBySerial && ForceStaleSerialSelector() != 0) {
|
||||
const Uint64 index = ++g_orderedWireWriteCounter;
|
||||
@@ -403,8 +416,10 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
auto* resource = FindWireBuffer(res);
|
||||
if (!resource || !resource->buffer.IsValid() || resource->size == 0) return false;
|
||||
resource->lastUseSerial = m_frameSerial;
|
||||
// B3: the draw this slice is being acquired for is recorded into the NEXT submission.
|
||||
// Until that submission's fence signals, a host write to these bytes is a data race.
|
||||
// B3: the draw this slice is being acquired for is normally recorded into the NEXT
|
||||
// submission - normally, not always: a mid-draw flush (SyncWireTextureShape's preserve
|
||||
// path) can submit this index without the draw. This stamp is the defence term's input
|
||||
// (see WriteWireBuffer); the floor behind lastUseSerial is the guarantee.
|
||||
if (pVulkanRenderer) resource->lastUseSubmitIndex = pVulkanRenderer->GetWireNextSubmitIndex();
|
||||
outSlice = resource->buffer.GetSlice(0, resource->size);
|
||||
outSlice.mapped = nullptr;
|
||||
@@ -429,6 +444,9 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
// work, not the draw that will bind that previously acquired slice.
|
||||
// Preserve its reservation even if the idle drain advanced the frame.
|
||||
resource->lastUseSerial = m_frameSerial;
|
||||
// B3 review: and the submission half of the same reservation - the wait cleared it,
|
||||
// and the draw holding the slice has not been submitted yet.
|
||||
if (pVulkanRenderer) resource->lastUseSubmitIndex = pVulkanRenderer->GetWireNextSubmitIndex();
|
||||
}
|
||||
if (!resource->buffer.Invalidate(size, offset)) return false;
|
||||
Memcpy(dst, static_cast<const Uint8*>(resource->buffer.GetMappedData()) + offset, static_cast<SizeT>(size));
|
||||
|
||||
@@ -228,11 +228,11 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
VkBufferObject buffer;
|
||||
Uint64 size = 0;
|
||||
Uint64 lastUseSerial = 0;
|
||||
// P7 wave 2 package B3: the submission that carries the most recent GPU use of
|
||||
// this buffer. lastUseSerial above is a FRAME COUNTER and cannot answer "has the
|
||||
// GPU finished reading these bytes" (see WriteWireBuffer); this can, because
|
||||
// IsSubmitIndexComplete polls the submission's own fence and reports an
|
||||
// unsubmitted index as incomplete.
|
||||
// P7 wave 2 package B3: the submission expected to carry the most recent GPU use
|
||||
// of this buffer - the busy predicate's DEFENCE term, not its guarantee (see
|
||||
// WriteWireBuffer). IsSubmitIndexComplete polls the real fence and reports an
|
||||
// unsubmitted index as incomplete, but the stamp is taken before the draw is
|
||||
// recorded and a mid-draw flush can submit it without the draw.
|
||||
Uint64 lastUseSubmitIndex = 0;
|
||||
Bool gpuWritesPending = false;
|
||||
// Only ranges actually submitted by resource_subdata are covered. No
|
||||
|
||||
@@ -13772,7 +13772,9 @@ void main() {
|
||||
++m_drainsSinceLastPresent;
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED
|
||||
// B3 probe: a drain that reaches here found the GPU caught up and nothing recording.
|
||||
// Every 8th one is treated as a FRAME BOUNDARY - mid-frame. See the note.
|
||||
// Every 8th one is treated as a FRAME BOUNDARY - mid-frame. This probe is what REFUTED
|
||||
// that as the OpenRA mechanism (one successful drain, zero boundary works across the
|
||||
// replay; magma-b3.md §2.4): the cause was the floor in OnSubmitsCompletedUpTo.
|
||||
{
|
||||
static const Bool probe = [] {
|
||||
const char* value = std::getenv("MOBILEGL_MAGMA_WIREBUF_PROBE");
|
||||
|
||||
@@ -3108,12 +3108,13 @@ if (MOBILEGL_BUILD_DISAGGREGATED)
|
||||
# ---- P7 wave 2 package B3, EXIT GATE 3: the streamed `glBufferSubData -> draw` pair.
|
||||
#
|
||||
# TWO ENTRIES OVER ONE CASE, for the reason the ShaderMip pair gives. The plain entry is
|
||||
# a regression gate that runs on every driver. The forced entry is the RED-ONCE: it makes
|
||||
# the frame-counting half of VkBufferManager's busy predicate answer "idle", which is the
|
||||
# state a mid-frame idle drain puts the wire arm into on the device and which no host
|
||||
# lane reaches on its own - measured, zero mid-frame frame-boundary drains across the
|
||||
# whole OpenRA replay, which is why the divergence is device-only and why the lane could
|
||||
# not arm against it before this knob existed.
|
||||
# a regression gate that runs on every driver. The forced entry is the red-once of the
|
||||
# DEFENCE term: it makes the serial half of VkBufferManager's busy predicate answer
|
||||
# "idle" - the state an unsound completed-serial floor put the wire arm into on the
|
||||
# device - so only the submission-fence half is left to order the copy. It does NOT
|
||||
# cover the floor fix itself (forcing the serial term bypasses the floor entirely); the
|
||||
# floor's lane is the OpenRA split retrace, which run_trace_case.cmake reds on any
|
||||
# "MGWIRE-FLOOR unsound-serial-complete" line.
|
||||
#
|
||||
# ITS OWN ENTRY RATHER THAN A LANE-WIDE FLIP: the knob changes how EVERY streamed buffer
|
||||
# write in the process is answered, so putting it in the shared Fm environment would
|
||||
|
||||
@@ -2078,12 +2078,13 @@ TEST_F(F1WireScenario, CopyImageInPlaceOverlapDeclinesAndLeavesTheLevelAlone) {
|
||||
// somebody else's vertices. So the case cannot assert "no error"; it has to assert the pixels.
|
||||
//
|
||||
// Registered TWICE per arm. The plain entry is a regression gate on every driver. The
|
||||
// `StaleSerial.` entry carries MGITEST_MAGMA_FORCE_STALE_BUFFER_SERIAL=1, which forces the
|
||||
// frame-counting half of the busy predicate to answer "idle" - the state a mid-frame idle drain
|
||||
// puts the wire arm into on the device, and one no host lane reaches by itself (measured: zero
|
||||
// mid-frame frame-boundary drains across the whole OpenRA replay). With the counting half
|
||||
// forced to lie, only the submission-fence half can still order the copy, which is exactly what
|
||||
// this package added and exactly what the entry pins.
|
||||
// `StaleSerial.` entry (split and spawn only - the tcp arm's server never sees an entry's
|
||||
// environment) carries MGITEST_MAGMA_FORCE_STALE_BUFFER_SERIAL=1, which forces the serial half
|
||||
// of the busy predicate to answer "idle" - the state the unsound completed-serial floor put the
|
||||
// wire arm into on the device. With the serial half forced to lie, only the submission-fence
|
||||
// half can still order the copy, so the entry pins that DEFENCE term. The guarantee is the
|
||||
// floor (VulkanRenderer::OnSubmitsCompletedUpTo), which the knob bypasses; its lane is the
|
||||
// OpenRA split retrace's "MGWIRE-FLOOR unsound-serial-complete" red condition.
|
||||
TEST_F(F1WireScenario, StreamedBufferSubDataBeforeEachDrawIsOrdered) {
|
||||
if (!Ready()) return;
|
||||
constexpr int kSize = 8, kBatches = 4, kColumn = kSize / kBatches;
|
||||
|
||||
Reference in New Issue
Block a user