[Fix] (DirectVulkan, SelfTest): honour the primitives-generated probe's leak-on-timeout contract in both of its callers, count paused-span draws the frontend cannot price, refuse a substitute measured worse than the stream query, and derive the POST row's failure clause from the measurement

This commit is contained in:
2026-08-28 06:19:40 -04:00
parent 19f4402fbf
commit feea131d8b
6 changed files with 238 additions and 40 deletions
@@ -1221,11 +1221,19 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// stale queries are always safe to delete.
Uint64 rendererGeneration = 0;
// Kind::XfbGenerated - the frontend's paused-draw primitive counter when the
// query began. VK_QUERY_TYPE_TRANSFORM_FEEDBACK_STREAM_EXT counts only what the
// capture saw, so a draw made while the span was paused is invisible to it -
// but GL_PRIMITIVES_GENERATED counts what the last vertex processing stage
// emitted regardless. The delta closes that gap at result time.
// query began. On the affected drivers VK_QUERY_TYPE_TRANSFORM_FEEDBACK_STREAM_EXT
// counts only what the capture saw, so a draw made while the span was paused is
// invisible to it - but GL_PRIMITIVES_GENERATED counts what the last vertex
// processing stage emitted regardless. The delta closes that gap at result time.
Uint64 pausedPrimitiveSnapshot = 0;
// ...unless the GPU already counted those paused draws when the span opened -
// through the reroute pool (VulkanRenderer::BeginXfbQueryForDraw reroutes every
// draw with no open capture, paused ones included) or, where the probe measured
// the stream query as counting capture-less draws, through the stream slot the
// paused draw still takes. Adding the CPU delta on top would count them twice,
// and the CPU counter is the weaker source anyway: only 3 of the ~15 draw entry
// points write it and it answers 0 for GL_PATCHES.
Bool pausedPrimitivesCountedByGpu = false;
};
} // namespace
@@ -1324,7 +1332,8 @@ namespace MobileGL::MG_Backend::DirectVulkan {
primitives)) {
return false;
}
if (query->kind == VulkanTimerQuery::Kind::XfbGenerated && MG_State::pGLContext != nullptr) {
if (query->kind == VulkanTimerQuery::Kind::XfbGenerated &&
!query->pausedPrimitivesCountedByGpu && MG_State::pGLContext != nullptr) {
primitives += MG_State::pGLContext->GetTransformFeedbackPausedPrimitiveCounter() -
query->pausedPrimitiveSnapshot;
}
@@ -1373,6 +1382,9 @@ namespace MobileGL::MG_Backend::DirectVulkan {
query->rendererGeneration = GetRendererGeneration();
query->pausedPrimitiveSnapshot =
MG_State::pGLContext ? MG_State::pGLContext->GetTransformFeedbackPausedPrimitiveCounter() : 0;
// Read AFTER StartXfbQueryCapture, which is where a failed reroute-pool creation
// disarms: the answer is then what this span will actually do for every draw.
query->pausedPrimitivesCountedByGpu = generated && pVulkanRenderer->ArePausedDrawsGpuCounted();
return query;
}
@@ -3285,9 +3285,10 @@ void main() {
m_primGenRerouteActiveSlots.clear();
m_primGenRerouteSlotCursor = 0;
m_primGenRerouteSlotOpen = false;
// Not sticky across renderers: the next bring-up re-decides it (from the
// Not sticky across renderers: the next bring-up re-decides both (from the
// per-process probe memo, so it re-decides without re-probing).
m_primGenRerouteKind = MG_Util::SelfTest::PrimGenRerouteKind::None;
m_primGenStreamCountsXfbInactiveDraws = false;
m_bufferManager.Shutdown();
// Device is idle (vkDeviceWaitIdle above); query pools can be destroyed.
@@ -11436,6 +11437,17 @@ void main() {
return true;
}
Bool VulkanRenderer::ArePausedDrawsGpuCounted() const {
// Exactly the gate BeginXfbQueryForDraw applies per draw, so a span told "armed"
// really does get a reroute slot for every draw with no open capture - a paused
// span's draws included.
const Bool rerouteArmed = m_primGenRerouteKind != MG_Util::SelfTest::PrimGenRerouteKind::None &&
m_primGenReroutePool != VK_NULL_HANDLE;
// Otherwise the paused draw takes a stream slot, which is an exact count of it
// on a driver the probe measured as counting capture-less draws.
return rerouteArmed || m_primGenStreamCountsXfbInactiveDraws;
}
void VulkanRenderer::StopXfbQueryCapture(Uint32 kind, Vector<Uint32>& outSlots,
Vector<Uint32>& outRerouteSlots) {
if (kind > 1) {
@@ -11503,17 +11515,21 @@ void main() {
if ((!m_xfbQueryCaptureActive[0] && !m_xfbQueryCaptureActive[1]) || m_xfbQueryPool == VK_NULL_HANDLE) {
return;
}
// A draw made while the GL span is merely PAUSED is CPU-accounted by the
// frontend's paused-primitive counter, whose delta the GENERATED resolve adds
// (DirectVulkan.cpp); a reroute slot for it would count it twice. Only a
// draw with no capture AND no paused span is the stream query's silent case.
const Bool glSpanPausedDraw = MG_State::pGLContext != nullptr &&
MG_State::pGLContext->IsTransformFeedbackActive() &&
MG_State::pGLContext->IsTransformFeedbackPaused();
// Every draw with no OPEN capture is the stream query's silent case, and that
// includes a draw made while the GL span is merely PAUSED (the pause closes the
// capture, so BeginXfbCaptureForDraw already answered false for it). Paused
// draws are rerouted like any other: the frontend's CPU paused-primitive
// counter cannot stand in for them - it is written by only 3 of the ~15 draw
// entry points (never the instanced, indirect or multi-draw ones) and answers 0
// for GL_PATCHES by design, since the tessellator's amplification is not
// knowable on the CPU - which is exactly the CTS's shape. Double counting is
// prevented on the other side instead: a GENERATED span opened while this
// reroute is armed ignores that CPU counter entirely (see
// ArePausedDrawsGpuCounted and DirectVulkan.cpp's XfbGenerated resolve), so
// every XFB-inactive draw in the span is priced exactly once, by this pool.
const Bool rerouteGenerated = m_xfbQueryCaptureActive[1] &&
m_primGenRerouteKind != MG_Util::SelfTest::PrimGenRerouteKind::None &&
m_primGenReroutePool != VK_NULL_HANDLE && !xfbActive &&
!glSpanPausedDraw;
m_primGenReroutePool != VK_NULL_HANDLE && !xfbActive;
// The stream slot stays for WRITTEN whatever the reroute does (with capture
// inactive its primitivesWritten is 0, which is the correct WRITTEN answer),
// and for GENERATED wherever this draw is not rerouted - so one GL query span
@@ -14234,7 +14250,18 @@ void main() {
return RunPrimitivesGeneratedNoXfbProbe(probeContext);
}();
verdict = EvaluatePrimitivesGeneratedNoXfbVerdict(s_measurement);
if (!s_measurement.ran) {
if (s_measurement.fenceWaitTimedOut) {
// The probe's submission never signaled within its bound, so it left its
// command pool, query pools, render pass, framebuffer, shader modules,
// pipeline layout, pipelines and fence alive on purpose. This device is the
// renderer's own and outlives them, so nothing here may destroy them or
// wait the device idle - the queue may still be executing that submission,
// and an idle wait is the hang the bound exists to prevent. They leak for
// the process's life; a device this sick has bigger problems.
MGLOG_W("PRIMITIVES_GENERATED probe timed out waiting on its own submission (%s); its "
"Vulkan objects are deliberately leaked and XFB-inactive draws keep the stream "
"query", s_measurement.failureReason.c_str());
} else if (!s_measurement.ran) {
MGLOG_W("PRIMITIVES_GENERATED probe did not run (%s); XFB-inactive draws keep the "
"stream query", s_measurement.failureReason.c_str());
} else {
@@ -14256,6 +14283,13 @@ void main() {
logShape("patches+discard", s_measurement.patchesDiscard);
}
}
// A driver whose stream query counts capture-less draws counts a PAUSED span's
// draws through the stream slot they take, so that span's result must not have
// the frontend's CPU paused counter added on top of it either (the pre-reroute
// accounting did exactly that, double counting every paused draw the CPU could
// price). Measured, not assumed: the forced arms never ask the probe and leave
// this false.
m_primGenStreamCountsXfbInactiveDraws = verdict == PrimitivesGeneratedNoXfbVerdict::StreamCounts;
m_primGenRerouteKind = ChoosePrimitivesGeneratedReroute(
overrideSetting, verdict, primitivesGeneratedQueryUsable, m_pipelineStatisticsQueryFeatureEnabled);
if (m_primGenRerouteKind != PrimGenRerouteKind::None) {
@@ -732,9 +732,12 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// per primitive reaching primitive clipping - after every vertex processing
// stage, before rasterizer discard - which is the same set).
// XFB-ACTIVE draws keep the stream slot (exact today, and WRITTEN needs it);
// paused-span draws keep the frontend's CPU accounting (see the paused counter
// in DirectVulkan.cpp) and never open a reroute slot, or they would count
// twice. One GL query span may therefore hold slots of both pools.
// every draw with no open capture - a PAUSED span's draws included - takes a
// reroute slot, and the span then ignores the frontend's CPU paused-primitive
// counter rather than adding it on top (see IsPrimGenRerouteArmed): that
// counter is written by only 3 of the ~15 draw entry points and answers 0 for
// GL_PATCHES, so it cannot price the draws this reroute exists to repair. One
// GL query span may therefore hold slots of both pools.
Bool m_pipelineStatisticsQueryFeatureEnabled = false;
// VK_EXT_primitives_generated_query: base feature, and the
// ...WithRasterizerDiscard feature without which a discarding draw inside the
@@ -747,6 +750,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Bool m_tessellationShaderFeatureEnabled = false;
MG_Util::SelfTest::PrimGenRerouteKind m_primGenRerouteKind =
MG_Util::SelfTest::PrimGenRerouteKind::None;
// The bring-up probe measured this device's stream query as counting draws made
// with no capture span open (the StreamCounts verdict) - so it counts the
// PAUSED-span ones too, through the stream slot they take when nothing is
// rerouted. Only the probe can know this, so it stays false wherever the probe
// is not consulted (the forced arms), which keeps those lanes' accounting as it
// was.
Bool m_primGenStreamCountsXfbInactiveDraws = false;
VkQueryPool m_primGenReroutePool = VK_NULL_HANDLE;
Uint32 m_primGenRerouteSlotCursor = 0;
Vector<Uint32> m_primGenRerouteActiveSlots;
@@ -758,6 +768,16 @@ namespace MobileGL::MG_Backend::DirectVulkan {
void ArmPrimGenReroute();
public:
// Whether a GENERATED span opened now will have the draws made while the GL
// span is PAUSED counted on the GPU - through the reroute pool, which takes
// every draw with no open capture, or (where the reroute is not armed because
// the stream query was measured to count capture-less draws) through the stream
// slot such a draw still takes. The frontend's CPU paused-primitive counter
// must not be added on top of either: it would double count, and it cannot
// price the draws that matter anyway - only 3 of the ~15 draw entry points
// write it and it answers 0 for GL_PATCHES. Read once per span, after
// StartXfbQueryCapture (whose pool creation may disarm the reroute).
Bool ArePausedDrawsGpuCounted() const;
// kind: 0 = PRIMITIVES_WRITTEN, 1 = PRIMITIVES_GENERATED.
Bool StartXfbQueryCapture(Uint32 kind);
void StopXfbQueryCapture(Uint32 kind, Vector<Uint32>& outSlots, Vector<Uint32>& outRerouteSlots);
+101 -6
View File
@@ -1681,7 +1681,21 @@ namespace MobileGL::MG_Util::SelfTest {
fail(format("vkCreateDevice failed (VkResult = {})", static_cast<Int>(createResult)));
return;
}
const ScopeGuard destroyDevice([&]() { vkDestroyDeviceFn(device, nullptr); });
// The probe's own teardown destroys (and idle-waits) everything it created -
// EXCEPT when its bounded fence wait expires, where it deliberately leaks
// every child object rather than touch a possibly hung GPU. This device must
// then leak with them: vkDestroyDevice requires its children destroyed and its
// queues idle, and on the driver that just missed a 5 s deadline the realistic
// outcome is a block inside vkDestroyDevice - the POST hang the bound exists to
// prevent. Same shape as the timestamp probe's guard above and the iterationRP
// witness's below.
Bool probeFenceWaitTimedOut = false;
const ScopeGuard destroyDevice([&]() {
if (probeFenceWaitTimedOut) {
return;
}
vkDestroyDeviceFn(device, nullptr);
});
VkQueue queue = VK_NULL_HANDLE;
vkGetDeviceQueueFn(device, graphicsQueueFamilyIndex, 0, &queue);
@@ -1750,6 +1764,8 @@ namespace MobileGL::MG_Util::SelfTest {
const PrimitivesGeneratedNoXfbMeasurement measurement =
RunPrimitivesGeneratedNoXfbProbe(probeContext);
// Before any return below: the guard above owns the device and must know.
probeFenceWaitTimedOut = measurement.fenceWaitTimedOut;
if (!measurement.ran) {
fail(format("the probe could not run ({}); the renderer's bring-up probe decides the "
"reroute independently",
@@ -1780,6 +1796,52 @@ namespace MobileGL::MG_Util::SelfTest {
shapeFacts("triangles under discard", measurement.trianglesDiscard) +
"; " + shapeFacts("patches under discard", measurement.patchesDiscard);
const auto statisticsExactOn = [](const PrimitivesGeneratedNoXfbShapeMeasurement& shape) {
return shape.statisticsMeasured && shape.statisticsClippingInput == shape.expectedPrimitives;
};
// What the PLAIN-ONLY verdict actually measured, named from the numbers rather
// than assumed: the shape the substitute misses may be the tessellated one
// alone, and a missed shape may read a wrong NONZERO count rather than 0. A
// row that always blamed rasterizer discard would put a false statement about
// the driver into the campaign's evidence artifact, contradicted by the facts
// string printed right after it.
const auto describeMissedStatisticsShapes = [&]() {
String missed;
const auto note = [&](const char* name,
const PrimitivesGeneratedNoXfbShapeMeasurement& shape) {
if (!shape.drawn || statisticsExactOn(shape)) {
return;
}
if (!missed.empty()) {
missed += " and ";
}
missed += name;
missed += shape.statisticsMeasured
? format(" (read {} of {} expected)", shape.statisticsClippingInput,
shape.expectedPrimitives)
: String(" (its statistics slot did not read back)");
};
note("the plain draw", measurement.trianglesPlain);
note("triangles under rasterizer discard", measurement.trianglesDiscard);
note("patches under rasterizer discard", measurement.patchesDiscard);
return missed;
};
// The CTS's tessellator-measuring shape is a PATCHES draw under discard; say
// whether THIS driver's substitute covers it instead of assuming it does not.
const auto describeCtsShape = [&]() -> String {
if (!measurement.patchesDiscard.drawn) {
return "the CTS's tessellator-measuring shape (a PATCHES draw under discard) could "
"not be measured here - this device has no tessellationShader - so whether "
"the substitute covers it is unknown";
}
return statisticsExactOn(measurement.patchesDiscard)
? "the CTS's tessellator-measuring shape (a PATCHES draw under discard) is "
"NOT among them: the substitute answers it exactly, so those tests are "
"repaired"
: "the CTS's tessellator-measuring shape (a PATCHES draw under discard) is "
"among them, so those tests stay broken on this driver";
};
switch (EvaluatePrimitivesGeneratedNoXfbVerdict(measurement)) {
case PrimitivesGeneratedNoXfbVerdict::StreamCounts:
builder.Pass(RowName,
@@ -1812,18 +1874,51 @@ namespace MobileGL::MG_Util::SelfTest {
case PrimitivesGeneratedNoXfbVerdict::StatisticsSubstitutePlainOnly:
fail("the stream query answers 0 for a draw made with no capture span open, and the "
"clipping-invocations statistics substitute counts the plain draw exactly but "
"reads 0 under rasterizer discard - so the renderer reroutes XFB-inactive draws "
"(repairing undiscarded queries at no cost to the rest) and the CTS's "
"tessellator-measuring shape, which needs the discarded count, remains broken "
"on this driver (" +
"misses " +
describeMissedStatisticsShapes() +
" - each of them a shape the stream query answered 0 for as well, so the renderer "
"reroutes XFB-inactive draws (repairing every shape the substitute answers, at no "
"cost to the rest, which is what the verdict requires); " +
describeCtsShape() + " (" + facts + ")");
return;
case PrimitivesGeneratedNoXfbVerdict::Unfixable: {
// Two ways to land here, and the report must not conflate them: no
// substitute answers even the plain draw, or one does but it is WRONG on a
// shape the stream query answers EXACTLY - arming it would trade a correct
// answer for a wrong one, so MobileGL refuses (see the verdict's
// domination rule).
String downgradeShapes;
const auto noteDowngrade = [&](const char* name,
const PrimitivesGeneratedNoXfbShapeMeasurement& shape) {
if (!shape.drawn || statisticsExactOn(shape) ||
shape.streamGenerated != shape.expectedPrimitives) {
return;
}
if (!downgradeShapes.empty()) {
downgradeShapes += " and ";
}
downgradeShapes += name;
};
noteDowngrade("the plain draw", measurement.trianglesPlain);
noteDowngrade("triangles under rasterizer discard", measurement.trianglesDiscard);
noteDowngrade("patches under rasterizer discard", measurement.patchesDiscard);
if (statisticsExactOn(measurement.trianglesPlain) && !downgradeShapes.empty()) {
fail("the stream query answers 0 for a draw made with no capture span open, and the "
"clipping-invocations statistics substitute repairs the plain draw but is wrong "
"on " +
downgradeShapes +
", which the stream query answers exactly - rerouting every XFB-inactive draw "
"would trade a correct count for a wrong one, so MobileGL arms nothing and the "
"capture-less query keeps the driver's answers (" +
facts + ")");
return;
case PrimitivesGeneratedNoXfbVerdict::Unfixable:
}
fail("the stream query answers 0 for a draw made with no capture span open and the "
"device offers no working statistics substitute; an application sizing a capture "
"buffer from GL_PRIMITIVES_GENERATED gets 0 (" +
facts + ")");
return;
}
case PrimitivesGeneratedNoXfbVerdict::Inconclusive:
break;
}
@@ -96,14 +96,17 @@ namespace MobileGL::MG_Util::SelfTest {
VkPipeline triangleDiscardPipeline = VK_NULL_HANDLE;
VkPipeline patchDiscardPipeline = VK_NULL_HANDLE;
VkFence fence = VK_NULL_HANDLE;
Bool fenceWaitTimedOut = false;
// Teardown on every path. When the fence wait timed out the submission may
// still be executing on a hung GPU: vkDeviceWaitIdle could block forever
// and destroying in-flight objects is undefined, so everything is
// deliberately leaked - a hung GPU must not hang the caller.
// deliberately leaked - a hung GPU must not hang the caller. The same flag
// is returned in the measurement, because a caller that OWNS the device must
// make the same choice for it (see the header): destroying a device whose
// children are alive and whose queue may still be executing is the very hang
// this bound exists to prevent.
const ProbeScopeGuard teardown([&]() {
if (fenceWaitTimedOut) {
if (measurement.fenceWaitTimedOut) {
return;
}
fns.vkDeviceWaitIdle(device);
@@ -393,7 +396,9 @@ namespace MobileGL::MG_Util::SelfTest {
}
constexpr Uint64 kFenceTimeoutNs = 5'000'000'000ull; // a probe must never hang its caller
if (fns.vkWaitForFences(device, 1, &fence, VK_TRUE, kFenceTimeoutNs) != VK_SUCCESS) {
fenceWaitTimedOut = true; // see the scope guard
// Set BEFORE failing: the scope guard reads it to skip every destroy, and
// the caller reads it out of the measurement to skip destroying the device.
measurement.fenceWaitTimedOut = true;
return fail("the probe submission did not complete within 5 s");
}
@@ -453,6 +458,15 @@ namespace MobileGL::MG_Util::SelfTest {
Bool allStreamExact = true;
Bool allPrimitivesGeneratedExtExact = true;
Bool allStatisticsExact = true;
// Whether the statistics substitute DOMINATES the stream query shape by shape:
// every shape the statistics do not answer exactly must be one the stream query
// answered 0 for anyway. Without this, a plain-shape-only substitute could be
// armed on a device whose stream query was RIGHT on a shape the statistics get
// wrong - and the renderer reroutes every XFB-inactive draw, so that shape would
// be downgraded from correct to wrong. "Never worse per draw" is what makes
// arming on an uncharacterised driver defensible; it has to be measured, not
// assumed.
Bool statisticsDominateStream = true;
for (const auto* shape : shapes) {
if (!shape->drawn) {
continue;
@@ -476,6 +490,11 @@ namespace MobileGL::MG_Util::SelfTest {
if (!shape->statisticsMeasured ||
shape->statisticsClippingInput != shape->expectedPrimitives) {
allStatisticsExact = false;
// Only a shape the stream query was silent on may be left behind by
// the substitute; a shape it answered exactly must not be traded away.
if (shape->streamGenerated == shape->expectedPrimitives) {
statisticsDominateStream = false;
}
}
}
if (allStreamExact) {
@@ -492,7 +511,10 @@ namespace MobileGL::MG_Util::SelfTest {
const auto& plain = measurement.trianglesPlain;
const Bool plainStatisticsExact =
plain.statisticsMeasured && plain.statisticsClippingInput == plain.expectedPrimitives;
return plainStatisticsExact ? PrimitivesGeneratedNoXfbVerdict::StatisticsSubstitutePlainOnly
// Both halves are required: the substitute must repair the plain shape, AND it
// must not cost any shape an answer the stream query already had right.
return (plainStatisticsExact && statisticsDominateStream)
? PrimitivesGeneratedNoXfbVerdict::StatisticsSubstitutePlainOnly
: PrimitivesGeneratedNoXfbVerdict::Unfixable;
}
@@ -102,6 +102,14 @@ namespace MobileGL::MG_Util::SelfTest {
// The probe submitted and read back at least the two triangle shapes.
// False when any setup step failed; failureReason then names the step.
Bool ran = false;
// The probe's bounded fence wait expired with the submission possibly
// still executing. The probe then deliberately LEAKED every child object
// it created (no vkDeviceWaitIdle, no destroys - a hung GPU must not hang
// the caller), so a caller that owns the device MUST NOT destroy or
// idle-wait it either: vkDestroyDevice with live children and in-flight
// work is the exact hang the bound exists to prevent. The POST leaks its
// throwaway device on this flag, mirroring its sibling probes.
Bool fenceWaitTimedOut = false;
String failureReason;
PrimitivesGeneratedNoXfbShapeMeasurement trianglesPlain;
PrimitivesGeneratedNoXfbShapeMeasurement trianglesDiscard;
@@ -200,15 +208,22 @@ namespace MobileGL::MG_Util::SelfTest {
// proven whole through clipping statistics.
StatisticsSubstitute,
// The defect is present and the statistics control is exact on the PLAIN
// shape but silent or wrong under rasterizer discard (llvmpipe's discard
// short-circuit does this to its statistics - its dedicated query is what
// rescues it to the verdict above). Rerouting still repairs every
// undiscarded query and is never worse per draw - a rerouted draw would
// have answered 0 through the silent stream query - but the CTS shape
// stays broken and the report must say so.
// shape but not on every drawn shape (llvmpipe's discard short-circuit
// does this to its statistics - its dedicated query is what rescues it to
// the verdict above). This verdict additionally GUARANTEES domination:
// every shape the statistics missed measured exactly 0 through the stream
// query too, so rerouting is never worse per draw - it repairs every
// shape the statistics answer exactly and leaves the rest at the 0 they
// already read. A measurement where the stream was EXACT on a shape the
// statistics missed does not qualify (rerouting would downgrade that
// shape) and falls to Unfixable instead. The shapes the substitute
// misses - the CTS's discarded shapes among them wherever they are the
// missed ones - stay broken, and the report must say which.
StatisticsSubstitutePlainOnly,
// The defect is present and no substitute qualifies even for the plain
// shape: the honest verdict is the current behaviour.
// The defect is present and no substitute qualifies: none is exact
// everywhere, and the plain-only fallback either misses the plain shape
// or fails the domination rule above. The honest verdict is the current
// behaviour.
Unfixable,
};