Repository navigation
fix(editor): balance presentation scopes across failure and retirement #28
Description
Verified on branch / awaiting merge
Production implementation: dd5979c613b0fe8ebd9f7026f25001ceee9f8cee. Delivered head: dbae93e8f211d45a69cdffdb6afa3a33a0af0fa5; the subsequent diff contains documentation/acceptance evidence only. Exact-head CI: Engine CI and NuGet dependency graph passed; all six checks on this exact head are successful. All applicable acceptance conditions for this issue are verified; unavailable physical/manual device checks are explicitly scoped, not reported passed.
Owner: Session/context scopes and actual presentation callers. Implementation commits: 65d222ae, e2e6ca32, a25d6302.
Evidence: complete report, exact-head delivery attachment, portable test-case/raw-measurement/requirement indexes linked by the report. Specific proof: Contracts/Visual: extension failure and next-frame scopes; Lifecycle: owner and retirement protocols; e28/export-label: prior-producer negative and corrected public provider failure.
Commands: official tests/Invoke-EditorUiAcceptance.ps1 Contracts/Workflows/Visual/Lifecycle/Products with SDK-selected independent outputs; complete source/entry preparation; formal InnoEngine.Build scripts --ide for the three companions; tests/Invoke-WikiChecks.ps1; controlled public performance fixtures where applicable. The report records original command owners, TRX/binlogs, negative tests, failures, skips, input hashes and device limits.
Profiles overlap; do not sum their pass counts into a distinct-test claim. Windows-only sharing fixtures and Unix-only filesystem fixtures are scoped to the executing host. Real native contexts/simulated display metrics and hidden product feature journeys are not a claim of physical display, OS IME or manual Mac testing. No BGCS/vendor/package/generated-binding modification, parallel Core system, test hook, IVT or private reflection was used.
Keep this issue Open. PR #26 includes an individual Fixes #28 and will close it only when the user merges to master. No automatic merge or premature master-resolution claim.
Original defect, ownership and accepted work
The following baseline observations describe the issue as opened. The verification above supersedes intermediate pending checkpoints; original failed/negative logs remain preserved. Acceptance checkboxes are checked only after the exact delivery-head verification completes.
Parent and original requirement
Parent: #27. Work item E01 from the approved Editor UI closure plan and its original implementation/acceptance requirements.
Baseline facts and risks at issue creation
Shader node labels and parts of Console drawing pair clip/style/child/ID calls sequentially without local finally protection. This is a confirmed source-level exception-safety gap; a user-visible crash has not yet been reproduced.
Owner and dependency boundary
Inno.Editor.ImGui, ShaderEditor and Console presentation; existing Session/owner and History lifetimes.
Dependencies: baseline inventory under #27.
Implementation
- Audit Begin/End and Push/Pop using each actual ImGui API contract; prioritize Shader Canvas and Console.
- Balance frame/window/context/thread borrows and nested owner scopes with existing scopes or local try/finally.
- Do not clear another owner's state or destroy a gesture whose rollback failed.
Acceptance
- Inject failures through actual public Drawer, menu, graph-content and lifecycle extension boundaries.
- Next-frame drawing remains usable and native ID/style/clip/window stacks are balanced.
- Independent sessions/windows remain isolated; invalid release order rejects without damaging valid later completion.
- Preserve diagnostics and public-boundary tests without IVT, private reflection or test-only APIs.
Failure and lifecycle constraints
Preserve diagnostics, cancellation, atomic rollback/publication, Missing recovery and generation retirement. Reuse Core Events, Identity, IO, Serialization and History. Do not modify BGCS/vendor/generated code or add global/implicit imports, compatibility paths, IVT or private reflection.
Evidence and closure
Commits, exact commands, logs/TRX/data and remaining limits will be added during implementation. Until all acceptance passes this issue remains open. After verification mark Verified on branch / awaiting merge; add Fixes #28 with this issue's actual number to PR #26 only when complete. Close on merge to master, not on branch implementation.
Activity
FLwolfy commentedon Oct 27, 2026
The real SDL presentation regression e01/backend-frame-before.trx now reproduces a remaining E01 defect: a balanced draw callback throws, RenderFrame propagates the original exception but leaves ImGui WithinFrameScope == 1. The managed flag alone had been reset, so a subsequent request would enter an unretired native frame. The fix will end only the owned failed native frame without publishing its partial draw data, preserve the original exception, reject reentrant disposal/requests, and verify the next frame succeeds. No vendor code, BGCS code, assertion suppression, or global stack clearing will be used. The same owned frame pipeline will expose a neutral preparation callback so Editor style inputs can be applied after context/metrics selection and before native NewFrame.
FLwolfy commentedon Oct 27, 2026
Post-fix verification: e03/all-native-frame-contracts.trx passes all 72 ImGui contracts. The new actual SDL tests verify preparation runs with selected context/display metrics before Native NewFrame, rejects reentrant RenderFrame/Dispose without releasing owners, propagates preparation failure without drawing, ends a failed draw's native frame, preserves the original exception, and successfully renders the following frame. No global/native stack reset or vendor/BGCS edit was used. SourceDebug compiles cleanly. The neutral presentation SPI now accepts an optional preparation callback; EditorLayer uses ImGuiEditorRuntime.PrepareFrame and cached instance delegates. Final product/CI verification is pending.
FLwolfy commentedon Oct 28, 2026
A deeper E01 review found a remaining concrete exceptional path in ShaderEditorCanvas.SourceSettings: each exported-function row calls PushId, then invokes InspectorRow/control drawing before its manual PopId. A thrown row callback bypasses that PopId; the outer popup/disabled finally blocks do not own the row ID. The explicit PopId before a remove/break only covers normal control flow. I will replace the row lifetime with local try/finally, audit the adjacent Shader parameter/technique loops for the same pattern, and exercise the real drawer boundary. This is a confirmed ownership gap; existing generic canvas exception tests do not establish safety for these separate controls.
FLwolfy commentedon Oct 28, 2026
Local implementation e2e6ca3 replaces six manual Shader row-ID lifetimes with local try/finally: stage outputs, keywords, exported functions, alternative implementations, techniques and technique roles. Removal/break uses the same cleanup, and the capability tree now uses finally as well. The current twelve-project SDK graph builds with zero errors. Eight source-policy positive/negative fixtures were added through the existing public architecture command; its selected 16-case source-contract group passes. Native UI/workflow/lifecycle suites are still being run; this source guard is explicitly not presented as dynamic proof of every presentation scope. Grouped acquisitions and the Console identity wrapper were separately traced to their actual finally-owning callers. Keep Open pending full E01 acceptance and merge.
FLwolfy commentedon Oct 29, 2026
The final per-view review found a concrete remaining failure boundary in Game Export: its three field tables close only on normal return, while target labels are read from the registered public IGameBuildTarget during drawing. A provider getter can throw with the platform table open. This existing scope-safety issue now also owns the local try/finally correction and a real native-modal regression through that replaceable public boundary. The regression checks the active table, preserved diagnostic, quarantine and drawing after extension refresh. No shared exception framework, native-assertion suppression or test-only production API is introduced. Final SDK execution remains required.
FLwolfy commentedon Oct 29, 2026
Commit a25d630 balances the export form's three Tables and removes one redundant text-assignment branch. Ordinary current SDK leaf compilation reports zero warnings/errors; export-table-current.trx passes the actual native provider-failure/quarantine/refresh regression. A separate test-owned copy using the prior official Workbench producer fails at exactly 'A modal failure left its own native Table active' (export-table-prior.trx), proving the failure rather than only static risk. The fixture closes only its own leaked table before reporting that expected negative result. The first unflushed asynchronous-log assertion remains a failed fixture attempt; the corrected case uses the existing LogRouter.Flush boundary. Full final candidate profiles/CI remain pending; keep this issue Open awaiting complete verification and merge.