test(editor): retain a tested offscreen editor surface capture path - #323
Conversation
PLAN.md WS-7.3 had two halves: a repeatable capture mechanism, and the final Personal/light artwork. This closes the mechanism half. Session 177 proved the idea but retained no helper, produced RGBA rather than the manifest's 24-bit RGB, and emitted an IMGUI cursor diagnostic. EditorSurfaceCapture renders a real shipped surface into an offscreen linear RenderTexture, crops to the surface's own worldBound, and writes RGB24 after verifying PNG color type 2. It restores the render target and GL.sRGBWrite, destroys everything it made on both paths, and reports the editor skin instead of switching it. Eight tests pin that contract; the full editor assembly is 557/0. Three findings are recorded in the manifest so they are not rediscovered: - A never-shown window has no panel, so there is nothing to lay out or render. That also surfaced a real defect in the shared test host -- EditorWindowTestUtility.CloseWindow called EditorWindow.Close() unconditionally, and Close() dereferences a null parent for a window that was never shown. Because the throw came from a finally, it replaced the original exception and hid the actual failure. CloseWindow now destroys a parentless window instead. - Omitting the Repaint step between ValidateLayout and Render yields a valid PNG of a blank frame, which is why the tests count distinct colors. - A full-canvas readback frames the host window's tab strip and clips the surface underneath it. Cropping is also what gives each image the tight frame the capture list asks for. The AddCursorRect diagnostic came from driving a whole live Inspector with an IMGUI body; the overlay crops render the package-owned UI Toolkit view directly and emit nothing. No tracked screenshot is replaced. The artwork half stays blocked on the host: it is Pro/dark, the manifest requires Personal/light, and skin switching is banned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Cursor (@cursor) review Please focus on the capture helper's failure paths and on whether the tests can pass while the capture is actually broken. Specifically:
Evidence in the description: 8/0 capture tests, 557/0 full editor assembly, a visually inspected 720x63 RGB24 crop with no chrome or clipping, a clean Console, and a host left at its seven standard windows. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dec91ce. Configure here.
…tore Two review findings, one of which an adversarial pass had already found. Refuse a surface that does not fit the canvas instead of clamping it. Clamping returned a silently trimmed image, so the helper built to prevent clipped screenshots could quietly produce one. Bugbot found the same defect from the bottom edge, where the clamped origin kept the full height and pulled rows above the surface into the readback. Make the failure-path restore test actually exercise the finally. It triggered the failure with a zero canvas width, which throws in argument validation BEFORE the capture takes over the render target or creates its host window, so the whole finally block could have been deleted and the test would still pass. It now fails from inside the try via an oversized surface, and also asserts the host window was closed. Verified by mutation: deleting the two restore lines from the finally fails both restore tests including the failure-path one (7 passed, 2 failed). Under the old test body that mutation went undetected. Full editor assembly 558/0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Cursor (@cursor) review Both findings are fixed in Bottom crop clamp keeps full height. Correct, and the fix goes further than the report: the clamp is gone entirely. Failure restore test skips finally. Also correct, and it was the more serious of the two, because it meant the Proven by mutation rather than by inspection: deleting the two restore lines from the Full editor assembly after the fixes: 558 passed, 0 failed. Worth re-reviewing with fresh eyes: the refusal predicate now compares |
The crop rounded bounds.x and bounds.width independently, which lets the two drift a pixel apart: a surface at x=10.5 w=10.5 rounds to x=10 w=10, a right edge of 20 where the real edge is 21. In a helper whose whole purpose is unclipped documentation images, a lost pixel is a clipped surface. Round the edges and derive the size from them, so the integer rect always tracks the float rect. The existing exact-size assertions cover the systematic case; full editor assembly 558/0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Closes the mechanism half of PLAN.md WS-7.3 / #314: a retained, test-pinned path that renders a real shipped editor surface into an offscreen render target and writes a cropped 24-bit PNG, never reading the desktop.
Session 177 proved the idea but retained no helper, produced RGBA rather than the manifest's 24-bit RGB, and emitted an IMGUI cursor diagnostic.
Tests/Editor/EditorSurfaceCapture.csis that helper, with eight tests inEditorSurfaceCaptureTests.It hosts the surface in a
HideAndDontSavewindow, drives the panel throughValidateLayout()->Repaint(Event)->Render()into a linearRenderTexture, crops to the surface's ownworldBound, and verifies PNG color type 2 before writing. It restores the render target andGL.sRGBWrite, destroys everything it made on both paths, and reports the editor skin instead of switching it.No tracked screenshot is replaced. The artwork half of WS-7.3 stays blocked on the host: it is Pro/dark, the manifest requires Personal/light, and skin switching is banned. #314 stays open for that.
Findings
Four things were wrong on the first attempt. Each is now fixed in code or recorded in the manifest.
A never-shown window has no panel, so there is nothing to lay out or render. That also surfaced a real defect in the shared test host:
EditorWindowTestUtility.CloseWindowcalledEditorWindow.Close()unconditionally, andClose()dereferences a null parent for a window that was never shown. Because the throw came from afinally, it replaced the original exception and hid the actual failure.CloseWindownow destroys a parentless window instead.Omitting the repaint step yields a valid PNG of a blank frame.
ValidateLayout()thenRender()alone produces a correctly sized, correctly typed, entirely empty image. The tests count distinct colors so a blank frame fails rather than passing as a valid PNG.A full-canvas readback frames the host window's chrome. The first visually inspected artifact showed the test host's tab strip with the surface clipped underneath it. Cropping to
worldBoundremoves the chrome and is also what gives each image the tight frame the capture list asks for. Render-target rows start at the bottom while UI Toolkit measures from the top, so the vertical origin iscanvasHeight - worldBound.yMax.The
AddCursorRectdiagnostic does not apply to the overlay crops. Session 177 hitEditorGUIUtility.AddCursorRect called outside an editor OnGUIwhile rendering a whole live Inspector with an IMGUI body. The overlay images the manifest asks for are crops of the package-owned UI Toolkit view, which renders directly and emits nothing.Red/green evidence
Live host, Unity 6000.4.6f1, Direct3D11. The capture runs are the red-green record: 3/5 on the first run (teardown NRE), 6/2 once the panel existed (blank frames), 8/0 once the repaint step was added, 8/0 again after the crop change.
EditorSurfaceCaptureTests: 8 passed, 0 failed;WallstopStudios.DxMessaging.Tests.Editorassembly: 557 passed, 0 failed (549 before this session's 8 new tests, so no regression from theCloseWindowchange);MessageAwareComponentInspectorViewrenders to a 720x63 RGB24 crop, 293 distinct colors, no window chrome, no clipping, complete 4-side border;isProSkin=True/UserSkin=1;design-system-dumps.test.jsandeditor-window-test-host.test.jsgreen, including the blocked-capture-primitive bans;npm run validate:all, spelling, prettier, markdownlint, csharpier, pre-commit: passed.Progress log:
progress/session-180-editor-surface-capture.md.Refs #314
Note
Low Risk
Changes are confined to editor test infrastructure and documentation; production runtime code is untouched, with tests guarding render-state cleanup and crop refusal.
Overview
Adds
EditorSurfaceCaptureand nineEditorSurfaceCaptureTeststo close the WS-7.3 mechanism half: render real shipped UI Toolkit editor surfaces offscreen into a linear render target, crop to the surface’sworldBound, and write manifest-compliant 24-bit RGB PNGs (color type 2) without desktop or screen capture.The capture path shows a hidden test host window, reflects inherited panel
ValidateLayout→Repaint→Render, restoresRenderTexture.activeandGL.sRGBWrite, and refuses oversized crops instead of silently clipping.EditorWindowTestUtility.CloseWindownow destroys never-shown windows (nom_Parent) so offscreen capture teardown does not NRE.Docs in
docs/images/inspector-overlay/README.mddocument the retained helper;progress/session-180-editor-surface-capture.mdlogs findings. No tracked screenshot PNGs are replaced—final Personal/light artwork remains blocked on host theme (#314).Reviewed by Cursor Bugbot for commit 2ac96a6. Bugbot is set up for automated code reviews on this repo. Configure here.