From b11bb9650a269c89bb3c683eb6192b1a614028f9 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 8 Sep 2026 02:56:40 -0400 Subject: [PATCH] [Feat] (Pipe): widen the dirty-surface scan to MG_State so the MGP_NOTE_MUTATION sites stop being outside the gate --- MobileGL/MG_Pipe/DirtySurface.def | 59 ++++++++++++++++++++++++++----- MobileGL/MG_Pipe/FillPoints.def | 10 ++++-- scripts/gen_pipe_dirty_surface.py | 50 +++++++++++++++++++++----- 3 files changed, 100 insertions(+), 19 deletions(-) diff --git a/MobileGL/MG_Pipe/DirtySurface.def b/MobileGL/MG_Pipe/DirtySurface.def index a72a7305..abd0aeb4 100644 --- a/MobileGL/MG_Pipe/DirtySurface.def +++ b/MobileGL/MG_Pipe/DirtySurface.def @@ -14,9 +14,18 @@ // forgets to publish renders stale, and no purity gate can see it. // // So the surface is enumerated MECHANICALLY. scripts/gen_pipe_dirty_surface.py scans -// MG_Impl/GLImpl for every pGLContext-> mutator call and, with --check, fails if a scanned -// 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. +// MG_Impl/GLImpl AND MG_State/GLState for every pGLContext-> mutator call and every +// MGP_NOTE_MUTATION site, and, with --check, fails if a scanned 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. +// +// THE SECOND ROOT AND THE SECOND MECHANISM ARE P3a's, and they were not cosmetic: reading +// MG_State/GLState found BumpSamplingResolutionGeneration, a mutator with no row at all +// whose every caller lives in that directory, and reading MGP_NOTE_MUTATION brought +// NoteUnitTouched - which no `pGLContext->` pattern can see, because every texture and +// sampler bind routes through it rather than calling it through the context - inside the +// gate. A notice site carries a FIELD name, so the mutator a row names is the ENCLOSING +// function. // // 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 @@ -134,14 +143,16 @@ // that should have named one. // // KNOWN BLIND SPOTS OF THE SCANNER, recorded here rather than left implicit -// (gen_pipe_dirty_surface.py's own notes plus its scan root): +// (gen_pipe_dirty_surface.py's own notes plus its scan roots): // 1. it matches braced function bodies textually, so a mutator inside a LAMBDA is // attributed to the enclosing function; -// 2. a mutation published through a HELPER the entry point calls reads as deferred here; -// 3. the scan root is MG_Impl/GLImpl only, so the four MGP_NOTE_MUTATION sites in -// MG_State/GLState/TextureState/TextureState.h are outside it entirely. +// 2. a mutation published through a HELPER the entry point calls reads as deferred here. +// 3. CLOSED AT P3a. The scan root was MG_Impl/GLImpl only, so the four MGP_NOTE_MUTATION +// sites in MG_State/GLState/TextureState/TextureState.h were outside it entirely. The +// root is now both directories and the notice is a recognised publish mechanism; what +// that found is the two rows marked "P3a" below. // The gate is therefore a COMPLETENESS gate over what the scanner does see. The semantic -// proof stays the MOBILEGL_PIPE_VERIFY lane, which is blind to none of the three. +// proof stays the MOBILEGL_PIPE_VERIFY lane, which is blind to none of them. // // clang-format off @@ -235,6 +246,29 @@ X(SetCurrentVertexAttributeUint, NEW_VERTEX_ATTRIB_DEFAULTS) \ /* ---- object class ---- */ \ X(BumpTextureBindGeneration, NEW_SAMPLER_VIEWS) \ + /* P3a, FOUND BY THE WIDENED SCAN ROOT and not by anything before it: every caller of */ \ + /* this one is inside MG_State/GLState (SamplerObject::BumpVersion for any sampler */ \ + /* parameter, TextureObjectBase's shape bump), which the scan did not read, so it had */ \ + /* no row at all while its sibling above did. NEW_SAMPLERS is the bit whose shutter */ \ + /* mixes the texture-params aggregate with exactly this generation, and the bump is */ \ + /* unconditional on every path that reaches it - the early-outs are in the setters */ \ + /* above it, which is the redundant-write guard this file's rule explicitly allows. */ \ + X(BumpSamplingResolutionGeneration, NEW_SAMPLERS) \ + /* P3a, and the reason the scan now reads MGP_NOTE_MUTATION as a publish mechanism of */ \ + /* its own: this function is not a pGLContext-> mutator at all - every texture and */ \ + /* sampler bind entry point routes THROUGH it - and it moves two pushed PipeInputs */ \ + /* fields with two different answers. */ \ + /* GetTextureBindGeneration moves only on the bindingChanged arm, so NEW_SAMPLER_ */ \ + /* VIEWS fires on SOME of the paths that mutate and not all: a redundant re-bind */ \ + /* of the object a unit already holds advances the high-water mark alone. That is */ \ + /* precisely what kPulledPartialShutter is for. */ \ + /* GetMaxTouchedTextureUnit has no shutter at all; it is in its verb class's may-read */ \ + /* mask and the residual fill copies it at every verb. */ \ + /* Both are ALSO published inline by MGP_NOTE_MUTATION when the write happens inside a */ \ + /* verb already in flight - a backend binding its own synthesised fallback texture - */ \ + /* which is the window no shutter and no pull can cover, and the reason those sites */ \ + /* had to come inside this gate rather than stay a recorded blind spot. */ \ + X(NoteUnitTouched, kPulledPartialShutter|NEW_SAMPLER_VIEWS) \ /* NOT NEW_SO_TARGETS, and this one was false on EVERY path: GLContext::SetNamed */ \ /* TransformFeedbackBinding either binds a BufferState binding point (index == the */ \ /* bound XFB object) or writes a saved-bindings entry, and NEW_SO_TARGETS mixes the */ \ @@ -249,6 +283,15 @@ /* to carry one. Espryt 0b's delete_* / resource_destroy publishes the kinds */ \ /* that have a handle on the wire; programs, program pipelines and shaders have */ \ /* none in P2, so nothing publishes theirs - kUnpublishedDestroy, a known hole. */ \ + /* TWO OF THESE ROWS STOPPED BEING ASPIRATIONAL AT P3a, and the call that makes */ \ + /* each true is named rather than implied: */ \ + /* MarkBufferObjectForDeletion -> resource_destroy (PipeCalls.def), emitted */ \ + /* from ~BufferObject the moment the last reference drops - which is the */ \ + /* glDelete* that only marks the name, followed by whatever unbind actually */ \ + /* releases it - and followed IN THAT ORDER by the client freeing the slot. */ \ + /* MarkVertexArrayForDeletion -> delete_vertex_elements, published through the */ \ + /* death notice ~VertexArrayObject already raises for the VertexElementsCso */ \ + /* kind; the CSO handle is minted per frontend VAO off its lifetime id. */ \ X(MarkBufferObjectForDeletion, kExplicitDestroy) \ X(MarkFramebufferObjectForDeletion, kExplicitDestroy) \ X(MarkProgramForDeletion, kUnpublishedDestroy) \ diff --git a/MobileGL/MG_Pipe/FillPoints.def b/MobileGL/MG_Pipe/FillPoints.def index 01edcd78..de614f4b 100644 --- a/MobileGL/MG_Pipe/FillPoints.def +++ b/MobileGL/MG_Pipe/FillPoints.def @@ -60,8 +60,14 @@ // What WOULD retire a row: the poison build already answers "was this field read at this // verb" exactly (MOBILEGL_PIPE_POISON_OMIT withholds one field's stamp for one verb and a // read of it aborts naming the pair). Turning that into a retirement gate means running the -// omission across the full CTS caselist on both devices, not the desktop corpus, and that is -// recorded as P3a work rather than done here on evidence that cannot support it. +// omission across the full CTS caselist on both devices, not the desktop corpus. +// +// P3a STATUS: still not done, and deliberately not done here. P3a is one of the five +// architecture boundaries that owe a full gl44to46 caselist run on both devices, so the +// omission sweep rides that run rather than duplicating it - the verdict lands with the +// caselist result at the phase exit, off the critical path, and every row above stays in +// place until it does. A row retired on desktop evidence would be retired on evidence that +// cannot support it, which is the reason this item exists rather than a scheduling excuse. // --------------------------------------------------------------------------------------- // // gen_pipe.py's block regexes end at a blank line: keep the empty line after each macro. diff --git a/scripts/gen_pipe_dirty_surface.py b/scripts/gen_pipe_dirty_surface.py index bbd81dc5..b8282c44 100644 --- a/scripts/gen_pipe_dirty_surface.py +++ b/scripts/gen_pipe_dirty_surface.py @@ -18,6 +18,14 @@ script reports every place in MG_Impl/GLImpl where a GL entry point BOTH mutates state through pGLContext AND reaches the backend in the same function - those are the publish points, the ones that must map onto an aggregate generation. +P3a widens the scan root to MG_State/GLState as well, and with it reads the OTHER publish +mechanism: MGP_NOTE_MUTATION, which a state object spells when it moves a pushed PipeInputs +field from inside a backend's own verb. Those sites carry a FIELD name rather than a mutator +name and none of them is a `pGLContext->` call, so the scan attributes each to its ENCLOSING +function - which is the name a DirtySurface.def row is written against. Until this, the four +in TextureState.h were outside the gate entirely, which is the one of the three blind spots +P2 recorded that a scan can actually close. + 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. @@ -51,13 +59,27 @@ import re import sys REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -SCAN_ROOT = os.path.join(REPO_ROOT, "MobileGL", "MG_Impl", "GLImpl") +# P3a WIDENS THE SCAN ROOT, and this is the one blind spot of the three the P2 review +# recorded that a scan can actually close (DirtySurface.def's header keeps the other two). +# `pGLContext->Set*` is not the only way a frontend mutation becomes observable: a state +# object that moves a PUSHED PipeInputs field from inside a backend's own verb publishes it +# with MGP_NOTE_MUTATION instead, and every one of those sites lives in MG_State/GLState - +# which the scan did not read at all, so the four in TextureState.h were outside the gate +# entirely. Reading both roots and both mechanisms is what makes "every mutation has an +# answer" a claim over the whole surface rather than over one directory of it. +SCAN_ROOTS = (os.path.join(REPO_ROOT, "MobileGL", "MG_Impl", "GLImpl"), + os.path.join(REPO_ROOT, "MobileGL", "MG_State", "GLState")) # The mutating half of GLContext's surface. Prefix-matched, per the plan's list. MUTATOR_PREFIXES = ("Add", "Set", "Mark", "Bump", "Allocate", "Truncate", "Record", "Notify", "Begin", "End") MUTATOR_RE = re.compile(r"pGLContext->\s*((?:%s)\w*)\s*\(" % "|".join(MUTATOR_PREFIXES)) +# The SECOND publish mechanism (MG_Pipe/PipeMutation.h). It carries the FIELD, not a mutator +# name, and it sits inside the state object rather than at a GL entry point - so the mutator +# it belongs to is the ENCLOSING function, which is what needs a row here. Reading it is what +# the widened root buys: MUTATOR_RE alone finds nothing at all under MG_State/GLState. +NOTE_MUTATION_RE = re.compile(r"MGP_NOTE_MUTATION\(\s*(\w+)\s*\)") BACKEND_RE = re.compile(r"gBackendFunctionsTable\.GL\.(\w+)|pActiveBackendObject->\s*(\w+)") FUNCTION_RE = re.compile(r"(?:^|\n)[ \t]*(?:[A-Za-z_][\w:<>,&*\s]*?)\b(\w+)\s*\(([^;{}]*)\)\s*" r"(?:const\s*)?(?:noexcept\s*)?\{") @@ -150,6 +172,12 @@ def scan_file(path): all_mutators = [(m.group(1), line_of(masked, m.start())) for m in MUTATOR_RE.finditer(masked)] for name, start, end in function_bodies(masked): body = masked[start:end] + # The push-on-mutation notice: the enclosing function is the mutator, because that is + # what a row in DirtySurface.def names and what a reader of this report has to look + # up. It is DEFERRED-shaped by construction (a state object does not reach the + # backend), so it joins all_mutators and never the immediate publish points below. + for note in NOTE_MUTATION_RE.finditer(body): + all_mutators.append((name, line_of(masked, start + note.start()))) mutators = [(m.group(1), line_of(masked, start + m.start())) for m in MUTATOR_RE.finditer(body)] if not mutators: continue @@ -1495,12 +1523,13 @@ def analyse_snippet(text): def scan_all(): - """(findings-per-file, {mutator: call count}) over the whole scan root.""" + """(findings-per-file, {mutator: call count}) over every scan root.""" sources = [] - for root, _, files in os.walk(SCAN_ROOT): - for name in sorted(files): - if name.endswith((".cpp", ".h")): - sources.append(os.path.join(root, name)) + for scan_root in SCAN_ROOTS: + for root, _, files in os.walk(scan_root): + for name in sorted(files): + if name.endswith((".cpp", ".h")): + sources.append(os.path.join(root, name)) sources.sort() per_file = [] @@ -1789,8 +1818,9 @@ def main(): help="run the canned negative controls; each must trip") args = parser.parse_args() - if not os.path.isdir(SCAN_ROOT): - sys.exit("missing %s" % SCAN_ROOT) + for scan_root in SCAN_ROOTS: + if not os.path.isdir(scan_root): + 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): @@ -1891,7 +1921,9 @@ def main(): for mutator, line in finding["mutators"]: print(" %-44s :%d %s" % (mutator, line, mapping.get(mutator, "UNMAPPED"))) - print("\ndirty-surface: %d files scanned under MG_Impl/GLImpl" % len(sources)) + print("\ndirty-surface: %d files scanned under %s" + % (len(sources), " + ".join(os.path.relpath(root, REPO_ROOT).replace(os.sep, "/") + for root in SCAN_ROOTS))) print("dirty-surface: %d mutator calls in total, %d distinct mutators" % (deferred_mutators, len(distinct_all))) print("dirty-surface: %d of them sit in %d IMMEDIATE PUBLISH POINTS - functions that also "