Repository navigation
fix(halo): mac finish pass, a viewer drag moves the dragged layer without redrawing any - #6070
Draft
prakashUXtech wants to merge 20 commits into
Draft
prakashUXtech wants to merge 20 commits into
prakashUXtech wants to merge 20 commits into
Conversation
The viewer drag split the frame once around the dragged layer (HaloDrawLift) and moves only that part per sample with a compositor offset; mouse-up draws the moved frame once. Layers with a non-normal blend at or over the dragged one keep the per-step offset list.
…raw gate stays on
pickedFrames(of:) rebuilt every key of the composition for each row, so one timeline body on huge (2,000 layers) was O(layers squared) and the release app sat in layout at full CPU after launch.
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #5950
Mac finish pass on the Halo quality gate (halo-journey-repairs, #1899). This is still a draft. The last re-audit failed the H-J5 box for four reasons: each viewer drag step redrew every layer, the viewer-drag timing wasn't gated, nothing sampled the real process during a drag, and nothing was timed on a release build. This PR fixes the first, gates the draw count, times the drag on a release build, and adds the real-app perf journey. The real-app sample isn't proved yet (see Status).
Fix 1: a viewer drag step redraws no layer
HaloDrawLift(HaloModel) splits the frame once, at the drag's first move, into the layers under the dragged one, the dragged layer and the layers over it. If the dragged layer or any layer over it uses a blend mode other than normal, the drag keeps the old per-step path, because the split would change how those layers blend.HaloLiftedLayers(HaloApp) draws each part once. Each sample re-runs onlyHaloLiftOffset, avisualEffectoffset on the middle part, inside one linear drawing group so the blending matches the single list (Halo viewer: a half-opaque layer draws differently on the canvas than in the render (HF-1 keyedFrame6) #1161). Mouse-up draws the moved frame once. The selection handles follow the drag delta.Fix 2: the timeline no longer stalls huge's first layout
Sampling the real release Halo on
huge(sample <pid>) showed the main thread at about 98% CPU, stuck in window layout for minutes after launch. 1,021 of 1,021 samples were inHaloTimelineView.content→HaloProject.pickedFrames(of:)→compositionKeys. Each row'spickedFrames(of:)rebuilt every key in the composition, so one timeline body was O(layers²): about 2,000 × 2,000 layers' keys. It now reads only its own layer's keys, andcompositionKeysuses the same per-layer helper. Same results, checked byHaloKeySelectionTests(16 pass). Two new mutations are inTests/mutations/HaloModel.HaloKeyPick+Kernel.json.Timing
swift test -c release -Xswiftc -DDEBUG --filter RenderBudgetHaloLiftTests),huge, 120 drag samples:step p50 0.26 ms p95 0.67 ms, 0 window bodies, 0 layer draws per sample. Six earlier repeats came in at p95 0.5 to 6.1 ms. All are under the 8 ms target.RenderBudgetHaloLiftTests.liftedDraggates the draw count (0 layer draws per sample, the offset re-runs, mouse-up draws once) and runs in the normal gate.liftedDragTiminggates p95 ≤ 8 ms. It lands.disabledbecause the gate refuses a wall-clock test that isn't inTests/perf-suites.txt, which is supervisor-only. Supervisor: addhalo AppSnapshotTests.BodyCountSuites/RenderBudgetHaloLiftTests/liftedDragTimingtoTests/perf-suites.txtand drop the.disabled.Real-app perf journey
UITests/HaloJourneyTests+RealPerf.swift(HaloRealPerfJourneyTests.testHugeViewerDrag) openshugein the release real-perf copy and drags the viewer for about 3 s throughJourney.health, withscripts/realperf-watchsampling the Halo pid. It asserts step p95 ≤ 16 ms on theviewer-dragprobe series. It isn't inscripts/ui-test'srealperflist yet (scripts/is protected), so run it withscripts/ui-test perf -only-testing:HaloJourneyTests/HaloRealPerfJourneyTestsplus-skip-testing:for the Photo, Design and Flare real-perf classes.Status
testHugeViewerDragstill failed, now with "Failed to get matching snapshots: Timed out while evaluating UI query". So no step p95 came from the real process.open -g -n .build/realperf/AfterGlow.app/Contents/Helpers/Halo.app --args -HaloSample huge -HaloWindowSize 1700x1000 -SuitePerfProbe) and ransample <pid> 10 1. The main thread spent 24 s building the first window and then went idle (0.3% CPU at 35 s, 0.0% at 60 s). The likely cause is the size of the accessibility tree: the timeline draws all 2,000 tracks plus 2,000halo.timeline.keysprobe elements, and none of them are lazy. That is the lazy-rows work in Halo: a 2,000-layer composition puts every row in the accessibility tree, so H-J10 times out #1262 (the H-J10 quarantine), which this issue lists under After beta. I couldn't count the AX elements here because this shell has no accessibility trust.HaloRealPerfJourneyTests.testHugeViewerDrag.HaloRealPerfJourneyTests.testHugeViewerDrag, and the no-real-input rule leaves no other way to drag the real app. The app has no CLI or MCP channel that can run a viewer drag. I left Halo: a 2,000-layer composition puts every row in the accessibility tree, so H-J10 times out #1262's lazy rows alone for the same reason as the last pass: the lanes still sit in a horizontal scroll view that builds every row, so the fix is a TimelineStrip restructure shared with Flare, plus moving 2,000halo.timeline.keysprobes into their rows, and only a journey can show it clears the AX query timeout.HaloRealPerfJourneyTests.testHugeViewerDrag, with Halo: a 2,000-layer composition puts every row in the accessibility tree, so H-J10 times out #1262's lazy rows first.HaloRealPerfJourneyTests.testHugeViewerDragafter Halo: a 2,000-layer composition puts every row in the accessibility tree, so H-J10 times out #1262's lazy rows.sampleduring a viewer drag and the box stays unticked. No code change.scripts/ci --changedpassed on 1270f6d. On 9008b26 it failed twice, each time on a different Halo export test hitting its 600 s time limit while the Mac's load average sat at 17 to 28:HaloMovieWriterTests.keyedRendersAsATwoSecondMovieMatchingFrameZero(831 s; it passes alone in 449 s on the same head) and thenHaloPNGWriterTests.keyedFrameSevenEqualsTheRender(772 s; the same suite passed in 261 s on 1270f6d). This branch doesn't touch either writer. The gate needs another run on 9008b26 when the Mac is quieter.Proof
Proof: H-J5 drag moves Position X |
HaloJourneyTests/testDragLayerToKeyPositionthroughscripts/ui-test(APP=halo, the full-scope quarantine doesn't skip it) on b3de8e1 | after a 100 px viewer drag at frame 12, the inspector reads Position X 660.0 px with a key at 12 (was stuck at 560). The test's only failure was an audit overlap in the suite switcher (suite.switcher.libraryoversuite.switcher.haloby 5 pt), which isn't Halo code |halo-drag-ball-keyed.png,halo-drag-ball-frame-12.pngfrom.build/ui-screensSnapshots
Before: each drag step redrew all 2,000 layers of huge. After: no visible change at rest. The drag draws the same pixels from the three cached parts, and mouse-up redraws once. No golden PNG changed.
Supersedes #5876