From c574043c1330c6b2ab137245f3062f8fc35c8f4c Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 6 Sep 2026 10:08:11 -0400 Subject: [PATCH] [Fix] (Pipe): derive the render-state answers of the dirty-surface map from RenderState.cpp instead of believing them, and correct the two rows that named a publisher which does not always fire - SetCapability named NEW_PIPELINE_STATE, but its ClipDistance0..7 arms write ClipDistanceEnabledMask (dynamic chunk D7) and deliberately do not BumpVersions, so that publisher does not fire at all for glEnable(GL_CLIP_DISTANCE0); SetStencilFunc named it too, while ++m_pipelineStateVersion there is conditional on Func moving, so a ref-only glStencilFunc does not move it either. Both are now NEW_RENDER_STATE, the answer that holds on every path - a row may now carry several publishers joined with '|', which is what lets the 18 setters that call BumpVersions on every path state both counters, and the patch trio state its own bit and the two render counters it also moves - --check no longer validates only row existence and answer vocabulary: it reads RenderState.cpp, derives per setter which of the two counters moves on EVERY path (BumpVersions moves both, a bare ++m_version only the first, a setter with both kinds of path only the first, a delegating setter inherits its callee's) and fails when a row claims a publisher that under-fires or omits one that always fires - two new self-test negative controls, one per direction, both built from the defects that were actually in the file - MarkProgram/MarkProgramPipeline/MarkShaderForDeletion answered kExplicitDestroy, a mechanism D13 does not build for them: Espryt 0b's explicit destroy is scoped to six object kinds that exclude programs, pipelines and shaders. They answer kUnpublishedDestroy now - a recorded hole rather than a mechanism that does not exist --- MobileGL/MG_Pipe/DirtySurface.def | 237 +++++++++++++++++------------- scripts/gen_pipe_dirty_surface.py | 178 ++++++++++++++++++++-- 2 files changed, 296 insertions(+), 119 deletions(-) diff --git a/MobileGL/MG_Pipe/DirtySurface.def b/MobileGL/MG_Pipe/DirtySurface.def index 3fe79067..fbee04a9 100644 --- a/MobileGL/MG_Pipe/DirtySurface.def +++ b/MobileGL/MG_Pipe/DirtySurface.def @@ -18,8 +18,21 @@ // mutator has no row here or a row here names a mutator the scan no longer finds. Both // directions, so a deleted mutator cannot leave a stale row behind either. // -// ANSWERS. One per row, and where a mutator has more than one true answer the row carries -// the COARSER one - the one that cannot under-fire: +// ANSWERS. A row lists EVERY publisher that fires on EVERY path through that mutator, +// and only those; several are joined with '|'. A publisher that fires on some paths but +// not all must not appear, because a shutter built from this file would then UNDER-fire, +// and ARCHITECTURE.md 13.2 names under-firing as the dangerous direction. +// +// For the RenderState family that answer is not a matter of taste and it is CHECKED +// rather than asserted: scripts/gen_pipe_dirty_surface.py reads RenderState.cpp and +// derives, per setter, which of NEW_RENDER_STATE / NEW_PIPELINE_STATE moves on every +// path - BumpVersions() moves both, a bare ++m_version moves only NEW_RENDER_STATE, and +// a setter with both kinds of path therefore always-fires only NEW_RENDER_STATE - and +// --check fails when a row disagrees, in either direction. That check exists because +// this file got exactly two rows wrong: SetCapability, whose ClipDistance0..7 arms move +// only m_version, and SetStencilFunc, whose pipeline bump is conditional on Func moving. +// Both named NEW_PIPELINE_STATE, which does not fire for glEnable(GL_CLIP_DISTANCE0) or +// for a reference-only glStencilFunc. // // NEW_* a MGPipeDirty bit (MG_Impl/Pipe/Tracker.h). The tracker's shutter for // that bit moves when this mutator runs, so the next verb publishes it. @@ -27,9 +40,19 @@ // mutation is published inline and needs no shutter at all. // kReverseChannel not state: a write INTO the frontend from the backend's side. // kNoBackendRead no backend read point observes this state at all. -// kExplicitDestroy published by the delete_* call the Track H slice emits when the object's -// last reference drops - an object's DEATH, which no generation shutters -// because there is no longer an object to carry one. +// kExplicitDestroy published by the delete_* call the Track H slice emits when the +// object's last reference drops - an object's DEATH, which no +// generation shutters because there is no longer an object to carry +// one. Only the six object kinds P2 brief D13 scopes Espryt 0b's +// explicit destroy to (buffers, framebuffers, renderbuffers, +// samplers, textures, vertex arrays) may use this answer. +// kUnpublishedDestroy +// the same event for a kind NO call publishes yet: programs, program +// pipelines and shaders are outside D13's six, so their death reaches +// a backend only through the object's own teardown path. Recorded as +// a hole rather than dressed up as a mechanism that exists - naming +// kExplicitDestroy here would be the same defect the RenderState +// check above exists to stop, one class down in stakes. // kPulledEveryVerb no shutter exists, and none is needed yet: the PipeInputs field this // writes is in its verb class's may-read mask, so the residual fill copies // it at EVERY verb of that class. A shutter here is a P3/P4 optimisation, @@ -48,104 +71,110 @@ // clang-format off // X(Mutator, Answer) -#define MGP_DIRTY_SURFACE_LIST(X) \ - /* ---- the reverse channel: 836 of the 926 calls, 90% of the surface ---- */ \ - X(RecordError, kReverseChannel) \ - /* ---- immediate publish points: the same body reaches the backend ---- */ \ - X(SetActiveTextureUnit, kImmediate) \ - X(BeginTransformFeedback, kImmediate) \ - X(EndTransformFeedback, kImmediate) \ - X(SetTransformFeedbackPaused, kImmediate) \ - X(MarkTransformFeedbackObjectForDeletion, kImmediate) \ - /* ---- the render state: NEW_PIPELINE_STATE when a setter calls */ \ - /* BumpVersions (P2 brief D6's only rule), NEW_RENDER_STATE otherwise */ \ - X(SetBlendEquation, NEW_PIPELINE_STATE) \ - X(SetBlendEquationIndexed, NEW_PIPELINE_STATE) \ - X(SetBlendFunc, NEW_PIPELINE_STATE) \ - X(SetBlendFuncIndexed, NEW_PIPELINE_STATE) \ - /* SetCapability's ClipDistance0..7 arms move only m_version; every other */ \ - /* arm calls BumpVersions, and the coarser answer is the one that holds. */ \ - X(SetCapability, NEW_PIPELINE_STATE) \ - X(SetCapabilityIndexed, NEW_PIPELINE_STATE) \ - X(SetColorMask, NEW_PIPELINE_STATE) \ - X(SetColorMaskIndexed, NEW_PIPELINE_STATE) \ - X(SetCullFaceMode, NEW_PIPELINE_STATE) \ - X(SetDepthFunc, NEW_PIPELINE_STATE) \ - X(SetDepthMask, NEW_PIPELINE_STATE) \ - X(SetFrontFaceMode, NEW_PIPELINE_STATE) \ - X(SetLogicOp, NEW_PIPELINE_STATE) \ - X(SetMinSampleShadingValue, NEW_PIPELINE_STATE) \ - X(SetPolygonMode, NEW_PIPELINE_STATE) \ - X(SetProvokingVertexMode, NEW_PIPELINE_STATE) \ - X(SetSampleCoverage, NEW_PIPELINE_STATE) \ - X(SetSampleMaskValue, NEW_PIPELINE_STATE) \ - /* SetStencilFunc writes Func (pipeline chunk P2/P3) AND Ref/ValueMask */ \ - /* (dynamic D3/D4); SetStencilOp is wholly pipeline, SetStencilMask wholly */ \ - /* dynamic. That split is what keeps a glStencilFunc that moves only the */ \ - /* reference from evicting a cached pipeline. */ \ - X(SetStencilFunc, NEW_PIPELINE_STATE) \ - X(SetStencilOp, NEW_PIPELINE_STATE) \ - X(SetStencilMask, NEW_RENDER_STATE) \ - X(SetBlendColor, NEW_RENDER_STATE) \ - X(SetClampReadColor, NEW_RENDER_STATE) \ - X(SetClearColor, NEW_RENDER_STATE) \ - X(SetClearDepth, NEW_RENDER_STATE) \ - X(SetClearStencil, NEW_RENDER_STATE) \ - X(SetClipControl, NEW_RENDER_STATE) \ - X(SetDepthRange, NEW_RENDER_STATE) \ - X(SetDepthRangeIndexed, NEW_RENDER_STATE) \ - X(SetHint, NEW_RENDER_STATE) \ - X(SetLineWidth, NEW_RENDER_STATE) \ - X(SetPointFadeThresholdSize, NEW_RENDER_STATE) \ - X(SetPointSize, NEW_RENDER_STATE) \ - X(SetPointSpriteCoordOrigin, NEW_RENDER_STATE) \ - X(SetPolygonOffset, NEW_RENDER_STATE) \ - X(SetPolygonOffsetClamped, NEW_RENDER_STATE) \ - X(SetPrimitiveRestartIndex, NEW_RENDER_STATE) \ - X(SetScissorBox, NEW_RENDER_STATE) \ - X(SetScissorBoxIndexed, NEW_RENDER_STATE) \ - X(SetViewport, NEW_RENDER_STATE) \ - X(SetViewportIndexed, NEW_RENDER_STATE) \ - /* ---- the other value-class bits ---- */ \ - X(SetPixelStoreParam, NEW_PIXEL_PACK) \ - X(SetPatchDefaultInnerLevel, NEW_PATCH_STATE) \ - X(SetPatchDefaultOuterLevel, NEW_PATCH_STATE) \ - /* Also an immediate publish point, but it has a real bit and the bit is */ \ - /* the more useful answer: set_patch_state carries it whatever the caller */ \ - /* does next. */ \ - X(SetPatchVertices, NEW_PATCH_STATE) \ - X(SetCurrentVertexAttributeFloat, NEW_VERTEX_ATTRIB_DEFAULTS) \ - X(SetCurrentVertexAttributeInt, NEW_VERTEX_ATTRIB_DEFAULTS) \ - X(SetCurrentVertexAttributeUint, NEW_VERTEX_ATTRIB_DEFAULTS) \ - /* ---- object class ---- */ \ - X(BumpTextureBindGeneration, NEW_SAMPLER_VIEWS) \ - X(SetNamedTransformFeedbackBinding, NEW_SO_TARGETS) \ - /* ---- an object's death: no generation, because there is no longer an */ \ - /* object to carry one. Espryt 0b's delete_* is what publishes these. */ \ - X(MarkBufferObjectForDeletion, kExplicitDestroy) \ - X(MarkFramebufferObjectForDeletion, kExplicitDestroy) \ - X(MarkProgramForDeletion, kExplicitDestroy) \ - X(MarkProgramPipelineForDeletion, kExplicitDestroy) \ - X(MarkRenderbufferObjectForDeletion, kExplicitDestroy) \ - X(MarkSamplerObjectForDeletion, kExplicitDestroy) \ - X(MarkShaderForDeletion, kExplicitDestroy) \ - X(MarkTextureObjectForDeletion, kExplicitDestroy) \ - X(MarkVertexArrayForDeletion, kExplicitDestroy) \ - /* ---- no backend read point observes these at all ---- */ \ - /* GL_ANY_SAMPLES_PASSED conditional rendering is resolved wholly in the */ \ - /* frontend: IsConditionalRenderActive / GetConditionalRenderQuery have no */ \ - /* reader under MG_Backend and no Coverage.def row. */ \ - X(BeginConditionalRender, kNoBackendRead) \ - X(EndConditionalRender, kNoBackendRead) \ - /* ---- pulled at every verb of the class, so the next verb publishes them */ \ - /* unconditionally. The transform-feedback accounting counters reach the */ \ - /* backend through GetTransformFeedbackCapturedVertices and friends, which */ \ - /* are in the kDraw and kXfbSpan may-read masks. */ \ - X(AddTransformFeedbackAccountedCaptureDraw, kPulledEveryVerb) \ - X(AddTransformFeedbackCapturedVertices, kPulledEveryVerb) \ - X(AddTransformFeedbackGeometryCaptureDraw, kPulledEveryVerb) \ - X(AddTransformFeedbackInputPrimitives, kPulledEveryVerb) \ - X(AddTransformFeedbackPausedPrimitives, kPulledEveryVerb) \ - X(AddTransformFeedbackPrimitives, kPulledEveryVerb) +#define MGP_DIRTY_SURFACE_LIST(X) \ + /* ---- the reverse channel: 836 of the 926 calls, 90% of the surface ---- */ \ + X(RecordError, kReverseChannel) \ + /* ---- immediate publish points: the same body reaches the backend ---- */ \ + X(SetActiveTextureUnit, kImmediate) \ + X(BeginTransformFeedback, kImmediate) \ + X(EndTransformFeedback, kImmediate) \ + X(SetTransformFeedbackPaused, kImmediate) \ + X(MarkTransformFeedbackObjectForDeletion, kImmediate) \ + /* ---- the render state. Derived from RenderState.cpp and gated by --check: */ \ + /* a setter that calls BumpVersions() on every path publishes BOTH counters; */ \ + /* one that also has a bare ++m_version path publishes only NEW_RENDER_STATE. */ \ + X(SetBlendEquation, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetBlendEquationIndexed, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetBlendFunc, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetBlendFuncIndexed, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + /* SetCapability's ClipDistance0..7 arms write ClipDistanceEnabledMask (dynamic */ \ + /* chunk D7) and deliberately do NOT BumpVersions, so NEW_PIPELINE_STATE does */ \ + /* not fire at all for glEnable(GL_CLIP_DISTANCE0): set_dynamic_state publishes */ \ + /* it, and NEW_RENDER_STATE is the only answer that holds on every arm. */ \ + X(SetCapability, NEW_RENDER_STATE) \ + X(SetCapabilityIndexed, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetColorMask, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetColorMaskIndexed, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetCullFaceMode, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetDepthFunc, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetDepthMask, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetFrontFaceMode, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetLogicOp, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetMinSampleShadingValue, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetPolygonMode, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetProvokingVertexMode, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetSampleCoverage, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetSampleMaskValue, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + /* SetStencilFunc writes Func (pipeline chunk P2/P3) AND Ref/ValueMask (dynamic */ \ + /* D3/D4), and ++m_pipelineStateVersion is CONDITIONAL on Func moving - which is */ \ + /* what keeps a glStencilFunc that moves only the reference from evicting a */ \ + /* cached pipeline, and is why only NEW_RENDER_STATE fires on every call. */ \ + /* SetStencilOp is wholly pipeline, SetStencilMask wholly dynamic. */ \ + X(SetStencilFunc, NEW_RENDER_STATE) \ + X(SetStencilOp, NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetStencilMask, NEW_RENDER_STATE) \ + X(SetBlendColor, NEW_RENDER_STATE) \ + X(SetClampReadColor, NEW_RENDER_STATE) \ + X(SetClearColor, NEW_RENDER_STATE) \ + X(SetClearDepth, NEW_RENDER_STATE) \ + X(SetClearStencil, NEW_RENDER_STATE) \ + X(SetClipControl, NEW_RENDER_STATE) \ + X(SetDepthRange, NEW_RENDER_STATE) \ + X(SetDepthRangeIndexed, NEW_RENDER_STATE) \ + X(SetHint, NEW_RENDER_STATE) \ + X(SetLineWidth, NEW_RENDER_STATE) \ + X(SetPointFadeThresholdSize, NEW_RENDER_STATE) \ + X(SetPointSize, NEW_RENDER_STATE) \ + X(SetPointSpriteCoordOrigin, NEW_RENDER_STATE) \ + X(SetPolygonOffset, NEW_RENDER_STATE) \ + X(SetPolygonOffsetClamped, NEW_RENDER_STATE) \ + X(SetPrimitiveRestartIndex, NEW_RENDER_STATE) \ + X(SetScissorBox, NEW_RENDER_STATE) \ + X(SetScissorBoxIndexed, NEW_RENDER_STATE) \ + X(SetViewport, NEW_RENDER_STATE) \ + X(SetViewportIndexed, NEW_RENDER_STATE) \ + /* ---- the other value-class bits ---- */ \ + X(SetPixelStoreParam, NEW_PIXEL_PACK) \ + X(SetPatchDefaultInnerLevel, NEW_PATCH_STATE|NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetPatchDefaultOuterLevel, NEW_PATCH_STATE|NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + /* Also an immediate publish point, but it has a real bit and the bit is */ \ + /* the more useful answer: set_patch_state carries it whatever the caller */ \ + /* does next. */ \ + X(SetPatchVertices, NEW_PATCH_STATE|NEW_RENDER_STATE|NEW_PIPELINE_STATE) \ + X(SetCurrentVertexAttributeFloat, NEW_VERTEX_ATTRIB_DEFAULTS) \ + X(SetCurrentVertexAttributeInt, NEW_VERTEX_ATTRIB_DEFAULTS) \ + X(SetCurrentVertexAttributeUint, NEW_VERTEX_ATTRIB_DEFAULTS) \ + /* ---- object class ---- */ \ + X(BumpTextureBindGeneration, NEW_SAMPLER_VIEWS) \ + X(SetNamedTransformFeedbackBinding, NEW_SO_TARGETS) \ + /* ---- an object's death: no generation, because there is no longer an object */ \ + /* to carry one. Espryt 0b's delete_* publishes the six kinds D13 scopes it to; */ \ + /* programs, program pipelines and shaders are NOT among them and nothing else */ \ + /* publishes their death, so they answer kUnpublishedDestroy - a recorded hole. */ \ + X(MarkBufferObjectForDeletion, kExplicitDestroy) \ + X(MarkFramebufferObjectForDeletion, kExplicitDestroy) \ + X(MarkProgramForDeletion, kUnpublishedDestroy) \ + X(MarkProgramPipelineForDeletion, kUnpublishedDestroy) \ + X(MarkRenderbufferObjectForDeletion, kExplicitDestroy) \ + X(MarkSamplerObjectForDeletion, kExplicitDestroy) \ + X(MarkShaderForDeletion, kUnpublishedDestroy) \ + X(MarkTextureObjectForDeletion, kExplicitDestroy) \ + X(MarkVertexArrayForDeletion, kExplicitDestroy) \ + /* ---- no backend read point observes these at all ---- */ \ + /* GL_ANY_SAMPLES_PASSED conditional rendering is resolved wholly in the */ \ + /* frontend: IsConditionalRenderActive / GetConditionalRenderQuery have no */ \ + /* reader under MG_Backend and no Coverage.def row. */ \ + X(BeginConditionalRender, kNoBackendRead) \ + X(EndConditionalRender, kNoBackendRead) \ + /* ---- pulled at every verb of the class, so the next verb publishes them */ \ + /* unconditionally. The transform-feedback accounting counters reach the */ \ + /* backend through GetTransformFeedbackCapturedVertices and friends, which */ \ + /* are in the kDraw and kXfbSpan may-read masks. */ \ + X(AddTransformFeedbackAccountedCaptureDraw, kPulledEveryVerb) \ + X(AddTransformFeedbackCapturedVertices, kPulledEveryVerb) \ + X(AddTransformFeedbackGeometryCaptureDraw, kPulledEveryVerb) \ + X(AddTransformFeedbackInputPrimitives, kPulledEveryVerb) \ + X(AddTransformFeedbackPausedPrimitives, kPulledEveryVerb) \ + X(AddTransformFeedbackPrimitives, kPulledEveryVerb) // clang-format on diff --git a/scripts/gen_pipe_dirty_surface.py b/scripts/gen_pipe_dirty_surface.py index f11ed2c4..18e6a90a 100644 --- a/scripts/gen_pipe_dirty_surface.py +++ b/scripts/gen_pipe_dirty_surface.py @@ -21,6 +21,12 @@ publish points, the ones that must map onto an aggregate generation. P0 is the skeleton: it reports. P1 adds the mapping file and CI regenerates it with `git diff --exit-code` and zero unmapped mutators, the same shape as gen_pipe.py's G6. +P2 adds the half a completeness gate cannot have: for the RenderState family the ANSWER is +derived from RenderState.cpp rather than believed, so a row that names a publisher which +fires on only some paths through the setter (or omits one that always fires) is red. Without +it a row could be wrong in exactly the direction ARCHITECTURE.md 13.2 calls dangerous while +--check stayed green, which is how two rows in this mapping were wrong for a whole review. + python3 scripts/gen_pipe_dirty_surface.py # human-readable report python3 scripts/gen_pipe_dirty_surface.py --summary # counts only python3 scripts/gen_pipe_dirty_surface.py --check # THE GATE: rc 1 on any hole @@ -139,15 +145,83 @@ def scan_file(path): DEF_PATH = os.path.join(REPO_ROOT, "MobileGL", "MG_Pipe", "DirtySurface.def") TRACKER_PATH = os.path.join(REPO_ROOT, "MobileGL", "MG_Impl", "Pipe", "Tracker.h") +RENDER_STATE_PATH = os.path.join(REPO_ROOT, "MobileGL", "MG_State", "GLState", "RenderState", + "RenderState.cpp") -ROW_RE = re.compile(r"^[ \t]*X\((\w+),\s*(\w+)\)\s*\\?\s*$", re.M) +# An answer is one or more publishers joined with "|" - every publisher that fires on EVERY +# path through the mutator (DirtySurface.def's header states the rule). +ROW_RE = re.compile(r"^[ \t]*X\((\w+),\s*([\w|]+)\)\s*\\?\s*$", re.M) DIRTY_NAME_RE = re.compile(r'^\s*"(NEW_[A-Z0-9_]+)",\s*$', re.M) # The answers that are not a dirty-bit name. Each one is documented in DirtySurface.def's # header; a row that uses anything else is a typo, and a typo that read as "mapped" would be # exactly the silent hole this gate exists to close. NON_BIT_ANSWERS = ("kImmediate", "kReverseChannel", "kNoBackendRead", "kExplicitDestroy", - "kPulledEveryVerb") + "kUnpublishedDestroy", "kPulledEveryVerb") + +# ---- the render-state answers, DERIVED rather than believed ----------------------------- +# The two RenderState counters are the one place in the mapping where "what publishes this" +# has a mechanical answer, and where getting it wrong is not a documentation slip: P3a builds +# its narrow shutters from this file, so a row that claims NEW_PIPELINE_STATE for a setter +# whose pipeline bump is conditional (SetStencilFunc) or absent (SetCapability's +# ClipDistance0..7 arms) encodes exactly the under-firing ARCHITECTURE.md 13.2 calls the +# dangerous direction. So the gate derives the answer from RenderState.cpp: +# +# BumpVersions() moves m_version AND m_pipelineStateVersion (RenderState.h); +# a bare ++m_version moves only the first; +# a setter that has BOTH kinds of path always-fires only NEW_RENDER_STATE. +# +# A setter whose body has no bump at all is resolved through the RenderState setter it +# delegates to (SetPolygonOffset -> SetPolygonOffsetClamped). +RENDER_STATE_BIT = "NEW_RENDER_STATE" +PIPELINE_STATE_BIT = "NEW_PIPELINE_STATE" +BUMP_VERSIONS_RE = re.compile(r"\bBumpVersions\s*\(\s*\)") +BARE_VERSION_RE = re.compile(r"\+\+\s*m_version\b") +BARE_PIPELINE_RE = re.compile(r"\+\+\s*m_pipelineStateVersion\b") +SETTER_CALL_RE = re.compile(r"\b(Set\w+)\s*\(") + + +def render_state_publishers(): + """{setter: set of always-firing render-state publishers} read out of RenderState.cpp. + + A setter absent from the result is not a RenderState setter at all; a setter mapped to an + EMPTY set moves neither counter (SetPixelStoreParam).""" + with open(RENDER_STATE_PATH, "r", encoding="utf-8", errors="replace") as handle: + masked = mask_comments_and_strings(handle.read()) + + bodies = {} + for name, start, end in function_bodies(masked): + if name.startswith("Set"): + bodies.setdefault(name, []).append(masked[start:end]) + + def direct(name): + publishers = set() + for body in bodies[name]: + bump = BUMP_VERSIONS_RE.search(body) is not None + bare_version = BARE_VERSION_RE.search(body) is not None + bare_pipeline = BARE_PIPELINE_RE.search(body) is not None + if bump or bare_version: + publishers.add(RENDER_STATE_BIT) + if bump and not bare_version and not bare_pipeline: + publishers.add(PIPELINE_STATE_BIT) + return publishers + + def resolve(name, seen): + if name in seen: + return set() + seen.add(name) + publishers = direct(name) + if publishers: + return publishers + # No bump of its own: whatever the setter it delegates to publishes. + for body in bodies[name]: + for match in SETTER_CALL_RE.finditer(body): + callee = match.group(1) + if callee != name and callee in bodies: + publishers |= resolve(callee, seen) + return publishers + + return {name: resolve(name, set()) for name in bodies} def dirty_bit_names(): @@ -160,7 +234,8 @@ def dirty_bit_names(): def load_mapping(text=None): - """{mutator: answer} from DirtySurface.def, or from `text` for the self-test.""" + """{mutator: answer} from DirtySurface.def, or from `text` for the self-test. An answer + keeps its "|"-joined spelling; answer_set() below is what compares them.""" if text is None: with open(DEF_PATH, "r", encoding="utf-8", errors="replace") as handle: text = handle.read() @@ -174,10 +249,16 @@ def load_mapping(text=None): return rows, duplicates -def check_mapping(mapping, duplicates, scanned, bits): +def answer_set(answer): + return {part.strip() for part in answer.split("|") if part.strip()} + + +def check_mapping(mapping, duplicates, scanned, bits, publishers=None): """Every problem the gate fails on, as a list of human-readable lines. BOTH directions: an unmapped mutator renders stale, and a row naming a mutator the scan no longer finds is - a stale row that would keep a real hole looking covered.""" + a stale row that would keep a real hole looking covered. `publishers` is + render_state_publishers()'s table; passing None checks only existence and vocabulary, + which is what the mutator-level negative controls want.""" problems = [] for mutator in sorted(set(scanned) - set(mapping)): problems.append("UNMAPPED mutator %s - add a row to MG_Pipe/DirtySurface.def" % mutator) @@ -187,13 +268,50 @@ def check_mapping(mapping, duplicates, scanned, bits): for mutator in sorted(duplicates): problems.append("DUPLICATE row %s" % mutator) for mutator in sorted(mapping): - answer = mapping[mutator] - if answer in NON_BIT_ANSWERS: + answers = answer_set(mapping[mutator]) + if not answers: + problems.append("BAD answer for %s - empty" % mutator) continue - if answer in bits: + for answer in sorted(answers): + if answer in NON_BIT_ANSWERS or answer in bits: + continue + problems.append("BAD answer %s for %s - not a MGPipeDirty bit name and not one of %s" + % (answer, mutator, ", ".join(NON_BIT_ANSWERS))) + if len(answers) > 1 and answers & set(NON_BIT_ANSWERS): + problems.append("BAD answer %s for %s - a non-bit answer stands alone" + % (mapping[mutator], mutator)) + + if publishers is None: + return problems + + # THE TRUTH HALF, and it is the half a row can be green and wrong without. For every + # mutator that is a RenderState setter, the render-state publishers the row claims must + # be exactly the ones RenderState.cpp always moves - a claimed publisher that does not + # always fire is an under-firing shutter waiting to be built from this file, and a + # publisher that always fires but is not claimed hides one. + render_bits = {RENDER_STATE_BIT, PIPELINE_STATE_BIT} + for mutator in sorted(mapping): + claimed = answer_set(mapping[mutator]) & render_bits + if mutator not in publishers: + if claimed: + problems.append( + "UNVERIFIABLE answer %s for %s - it claims a render-state publisher but " + "RenderState.cpp has no such setter to derive it from" + % (mapping[mutator], mutator)) continue - problems.append("BAD answer %s for %s - not a MGPipeDirty bit name and not one of %s" - % (answer, mutator, ", ".join(NON_BIT_ANSWERS))) + derived = publishers[mutator] & render_bits + if claimed == derived: + continue + for missing in sorted(derived - claimed): + problems.append( + "MISSING publisher %s for %s - RenderState.cpp moves it on every path, so the " + "row must name it (derived: %s)" + % (missing, mutator, "|".join(sorted(derived)) or "none")) + for extra in sorted(claimed - derived): + problems.append( + "UNDER-FIRING answer %s for %s - RenderState.cpp does NOT move it on every " + "path, so a shutter built on it would miss a mutation (derived: %s)" + % (extra, mutator, "|".join(sorted(derived)) or "none")) return problems @@ -224,7 +342,7 @@ def scan_all(): return sources, per_file, distinct_all -def self_test(scanned, bits): +def self_test(scanned, bits, publishers): """Canned negative controls. Each MUST trip; trips == 0 is an error, which is the shape check_include_closure.py and gen_pipe.py --self-test already use.""" trips = 0 @@ -257,6 +375,28 @@ def self_test(scanned, bits): else: failures.append("negative control 3 (a bad answer) did NOT trip") + # 4. THE CONTROL FOR THE TRUTH HALF, and it is the shape of the defect that was actually + # in this file: a row claiming a publisher that fires on only some paths through the + # setter. SetCapability's ClipDistance0..7 arms move m_version alone, so + # NEW_PIPELINE_STATE here must read as under-firing rather than as a valid answer. + with_under_firing = dict(real) + with_under_firing["SetCapability"] = PIPELINE_STATE_BIT + problems = check_mapping(with_under_firing, real_duplicates, scanned, bits, publishers) + if any(p.startswith("UNDER-FIRING") for p in problems): + trips += 1 + else: + failures.append("negative control 4 (an under-firing render-state answer) did NOT trip") + + # 5. the other direction: a row that drops a publisher which DOES always fire. Silent + # today, load-bearing the moment P3a builds a shutter from the file. + with_missing = dict(real) + with_missing["SetBlendEquation"] = RENDER_STATE_BIT + problems = check_mapping(with_missing, real_duplicates, scanned, bits, publishers) + if any(p.startswith("MISSING publisher") for p in problems): + trips += 1 + else: + failures.append("negative control 5 (a dropped render-state publisher) did NOT trip") + for failure in failures: print("dirty-surface self-test: %s" % failure) if trips == 0: @@ -283,26 +423,34 @@ def main(): sys.exit("missing %s" % SCAN_ROOT) if not os.path.isfile(DEF_PATH): sys.exit("missing %s" % DEF_PATH) + if not os.path.isfile(RENDER_STATE_PATH): + sys.exit("missing %s" % RENDER_STATE_PATH) sources, per_file, distinct_all = scan_all() bits = dirty_bit_names() if not bits: sys.exit("could not read the MGPipeDirty bit names out of %s" % TRACKER_PATH) + publishers = render_state_publishers() + if not publishers: + sys.exit("could not derive any RenderState setter out of %s" % RENDER_STATE_PATH) if args.self_test: - return self_test(distinct_all, bits) + return self_test(distinct_all, bits, publishers) mapping, duplicates = load_mapping() if args.check: - problems = check_mapping(mapping, duplicates, distinct_all, bits) + problems = check_mapping(mapping, duplicates, distinct_all, bits, publishers) for problem in problems: print("dirty-surface: %s" % problem) if problems: print("dirty-surface: %d problem(s); the mapping must cover every mutator the scan " - "finds, in both directions" % len(problems)) + "finds, in both directions, and every render-state answer must be the one " + "RenderState.cpp actually publishes" % len(problems)) return 1 - print("dirty-surface: %d mutators, all mapped, no stale rows" % len(mapping)) + derived = sum(1 for m in mapping if m in publishers) + print("dirty-surface: %d mutators, all mapped, no stale rows; %d render-state answers " + "derived from RenderState.cpp and matching" % (len(mapping), derived)) return 0 total_functions = 0