Skip to content

fix(halo): mac finish pass, a viewer drag moves the dragged layer without redrawing any - #6070

Draft
prakashUXtech wants to merge 20 commits into
devfrom
fix/mac-finish-halo-20261029-165500
Draft

prakashUXtech wants to merge 20 commits into
devfrom
fix/mac-finish-halo-20261029-165500

Conversation

@prakashUXtech

@prakashUXtech prakashUXtech commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

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 only HaloLiftOffset, a visualEffect offset 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 in HaloTimelineView.content → HaloProject.pickedFrames(of:) → compositionKeys. Each row's pickedFrames(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, and compositionKeys uses the same per-layer helper. Same results, checked by HaloKeySelectionTests (16 pass). Two new mutations are in Tests/mutations/HaloModel.HaloKeyPick+Kernel.json.

Timing

  • Release build (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.liftedDrag gates the draw count (0 layer draws per sample, the offset re-runs, mouse-up draws once) and runs in the normal gate.
  • liftedDragTiming gates p95 ≤ 8 ms. It lands .disabled because the gate refuses a wall-clock test that isn't in Tests/perf-suites.txt, which is supervisor-only. Supervisor: add halo AppSnapshotTests.BodyCountSuites/RenderBudgetHaloLiftTests/liftedDragTiming to Tests/perf-suites.txt and drop the .disabled.

Real-app perf journey

UITests/HaloJourneyTests+RealPerf.swift (HaloRealPerfJourneyTests.testHugeViewerDrag) opens huge in the release real-perf copy and drags the viewer for about 3 s through Journey.health, with scripts/realperf-watch sampling the Halo pid. It asserts step p95 ≤ 16 ms on the viewer-drag probe series. It isn't in scripts/ui-test's realperf list yet (scripts/ is protected), so run it with scripts/ui-test perf -only-testing:HaloJourneyTests/HaloRealPerfJourneyTests plus -skip-testing: for the Photo, Design and Flare real-perf classes.

Status

  • Run on b3de8e1, before fix 2: the real-perf launch failed with "has not loaded accessibility". The sample above is what that hang was. Fix 2 removes it.
  • Rerun on 2022ca5, with fix 2: testHugeViewerDrag still failed, now with "Failed to get matching snapshots: Timed out while evaluating UI query". So no step p95 came from the real process.
  • That failure isn't app CPU any more. I launched the release real-perf Halo on huge with no input (open -g -n .build/realperf/AfterGlow.app/Contents/Helpers/Halo.app --args -HaloSample huge -HaloWindowSize 1700x1000 -SuitePerfProbe) and ran sample <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,000 halo.timeline.keys probe 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.
  • Journeys weren't allowed in this pass (FINISHER_JOURNEYS is not 1), so I didn't rerun the real-perf journey. Until it reports a step p95 from the real process, the H-J5 box stays unticked and the PR stays a draft.
  • 10-31 pass: dev merged in again (ed2dbb7). Journeys still weren't allowed, and the no-real-input rule leaves an XCUITest drive as the only way to drag the real app, so the real-process sample is still missing. I didn't take on the lazy timeline rows (Halo: a 2,000-layer composition puts every row in the accessibility tree, so H-J10 times out #1262) here: they change SuiteUI's TimelineStrip, and with no journey run I couldn't show that they clear the AX query timeout. The next pass that has journeys allowed should do Halo: a 2,000-layer composition puts every row in the accessibility tree, so H-J10 times out #1262's rows first, then run HaloRealPerfJourneyTests.testHugeViewerDrag.
  • 10-31 second pass: dev merged in again (5554cb6). Journeys still weren't allowed, so I couldn't run 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,000 halo.timeline.keys probes into their rows, and only a journey can show it clears the AX query timeout.
  • 10-31 third pass: dev merged in again (6527ee3, with feat: shared Select tool, kernel rule plus Photo and Design adapters, Photo's Move folded in #6018). Journeys still weren't allowed. I looked for a way to drive a viewer drag in the real app without real input. The only option was a launch flag that feeds the drag samples itself, but that skips SwiftUI's DragGesture, so it wouldn't be a sample of the real gesture. I didn't add it. The box stays unticked until a pass that has journeys allowed runs 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.
  • 10-31 fourth pass: dev had not moved since 6527ee3 (origin/dev is still 66d9fbe, already merged), so there was nothing to merge and nothing new to rerun. Journeys still weren't allowed (FINISHER_JOURNEYS=0). No code change; the H-J5 box stays unticked until a journeys-allowed pass runs HaloRealPerfJourneyTests.testHugeViewerDrag after Halo: a 2,000-layer composition puts every row in the accessibility tree, so H-J10 times out #1262's lazy rows.
  • 11-01 pass: dev merged in twice (1270f6d, then 9008b26 with Photo stroke test: stop the flip test's leftover 8K renders starving its first picture #6135 and Fix the lane removing a quarantine while its owner is still open #6137, Photo tests and lane scripts only). Journeys still weren't allowed (FINISHER_JOURNEYS is not 1), so there's still no real-process sample during a viewer drag and the box stays unticked. No code change.
  • scripts/ci --changed passed 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 then HaloPNGWriterTests.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/testDragLayerToKeyPosition through scripts/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.library over suite.switcher.halo by 5 pt), which isn't Halo code | halo-drag-ball-keyed.png, halo-drag-ball-frame-12.png from .build/ui-screens

Snapshots

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

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.
@prakashUXtech prakashUXtech added mac-finish Mac finisher pass PR quality-gate 10-26 audit bugs the Mac finisher fixes and proves in the built app labels Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mac-finish Mac finisher pass PR quality-gate 10-26 audit bugs the Mac finisher fixes and proves in the built app

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant