mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-08 20:28:32 +09:00
[Fix] (MG_Impl, DirectGLES, scripts): take the cheap review minors - the stale validate-point comment, G7's whole-log trip grep and missing INT/TERM traps, G5's CRLF stdout, and the dangling-twin invariant at its member
This commit is contained in:
@@ -979,6 +979,22 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
struct ResolvedDrawBuffers {
|
||||
struct Entry {
|
||||
MG_State::GLState::BufferObject* frontend = nullptr;
|
||||
// A RAW TWIN POINTER, AND IT MAY DANGLE - the invariant that makes that safe
|
||||
// is stated here rather than left in the two callers (espryt-v3 §8, m8).
|
||||
//
|
||||
// Nothing tells this memo when a twin dies: on the handle arm a
|
||||
// resource_destroy takes the twin out of the slot table (ReleaseByHandle)
|
||||
// while this entry still holds its address, and on the legacy arm the same
|
||||
// is true of the registry's own release. So the rule is: THIS POINTER IS
|
||||
// ONLY EVER DEREFERENCED AFTER THE ENTRY'S IDENTITY HAS BEEN RE-RESOLVED IN
|
||||
// THE SAME PASS - FindByHandle(handle) on the handle arm, the frontend
|
||||
// identity compare on the legacy one - and a miss re-resolves through
|
||||
// EnsureBufferResource rather than trusting what is stored here. Both
|
||||
// consumers do that today; a third one that read `resource` straight out of
|
||||
// a "valid" memo would be reading freed memory, and no compare in this
|
||||
// struct would catch it. The pointer stays raw because the alternative -
|
||||
// owning a reference from a per-draw memo - is what keeps a dead driver
|
||||
// buffer alive, which is the leak class P2's death notice exists to remove.
|
||||
BufferImpl::GLESBufferResource* resource = nullptr;
|
||||
Uint8 attribIndex = 0;
|
||||
#if MOBILEGL_PIPE_PUSH
|
||||
|
||||
@@ -1587,10 +1587,12 @@ namespace MobileGL::MG_Pipe {
|
||||
}
|
||||
|
||||
// P3a's vertex segment, in the order the design fixes: vertex elements, then the
|
||||
// vertex buffers that fill them, then the index binding. All three still resolve to
|
||||
// false today - their dirty bits map to no subsystem until the tracker's arms land -
|
||||
// and all three emitters are stubs; the call sites are here so the commit that gives
|
||||
// them bodies does not also have to edit the validate point.
|
||||
// vertex buffers that fill them, then the index binding. All three are LIVE now (m1):
|
||||
// bits 5 / 9 / 10 map onto kMGPipeSubsystemVertexInput in Tracker.h:145-148 and all
|
||||
// three emitters have bodies, so `wants()` answers true whenever bit 8 is in the push
|
||||
// mask - which the phase default 0x1ff sets, on every backend. The sentence that used
|
||||
// to stand here ("all three still resolve to false today - their dirty bits map to no
|
||||
// subsystem") was the contract commit's and stopped being true when the client landed.
|
||||
if (wants(MGPipeDirty::NewVertexElements)) {
|
||||
payloadBytes += EmitVertexElements(*ctx);
|
||||
}
|
||||
|
||||
@@ -171,7 +171,13 @@ say "$TEST_NAME is green before the patch"
|
||||
|
||||
# --- 2. stop copying IsBgra ------------------------------------------------------------------
|
||||
cp -f "$HEADER" "$BACKUP" || exit 2
|
||||
# m3: INT and TERM as well as EXIT. A Ctrl-C during the rebuild used to leave the patched header
|
||||
# in the tree - bash runs no EXIT trap for an uncaught SIGINT - and the next thing that reader
|
||||
# does is build, from a hard-zeroed field, with nothing saying so. The two extra traps repair and
|
||||
# then re-raise with the default disposition, so the exit status still reports the signal.
|
||||
trap 'repair' EXIT
|
||||
trap 'repair; trap - INT; kill -INT $$' INT
|
||||
trap 'repair; trap - TERM; kill -TERM $$' TERM
|
||||
# Armed BEFORE the patcher runs, not after: a python that died half-way through the write must
|
||||
# still be repaired. The cost of arming it early is one unnecessary rebuild in the case where the
|
||||
# patcher matched nothing and the file is byte-identical (cp refreshes its mtime).
|
||||
@@ -216,9 +222,18 @@ say "running $TEST_NAME against the dropped field"
|
||||
if ctest --test-dir "$BUILD_DIR" -R "$TEST_NAME" --no-tests=error --output-on-failure \
|
||||
> "$LOG_DIR/ctest-after.log" 2>&1; then
|
||||
VERDICT=did-not-trip
|
||||
elif grep -q "$FIELD" "$LOG_DIR/ctest-after.log"; then
|
||||
elif grep -qE "^.*(Failure|error:|Expected).*$FIELD|$FIELD.*(Failure|Which is|Expected)" \
|
||||
"$LOG_DIR/ctest-after.log"; then
|
||||
# A red is not yet a pass: a suite that had started failing for an unrelated reason satisfies the
|
||||
# first half of the claim and none of the second.
|
||||
#
|
||||
# m3: matched against the FAILING ASSERTION's own lines rather than against the whole ctest log.
|
||||
# A bare `grep -q IsBgra` over the log was exact today only because the field name happens to
|
||||
# appear exactly once in the tree, inside the case that fails; a future case NAMED after the
|
||||
# field, a skip reason quoting it, or a compiler note echoed into the log would all have made
|
||||
# "tripped" mean "the string exists somewhere". The alternation keeps both orders because gtest
|
||||
# prints the field on the `Failure`/`Expected` line for an EXPECT_EQ and on the following
|
||||
# `Which is` line for a streamed message.
|
||||
VERDICT=tripped
|
||||
else
|
||||
VERDICT=wrong-reason
|
||||
|
||||
Reference in New Issue
Block a user