-
Notifications
You must be signed in to change notification settings - Fork 2.4k
editor: compass sync + fast align-to-north #478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -457,9 +457,11 @@ type GuideHandleHintAnchor = { | |
| function FloorplanCompassButton({ | ||
| northRotationDeg, | ||
| onAlignNorth, | ||
| needleRef, | ||
| }: { | ||
| northRotationDeg: number | ||
| onAlignNorth: () => void | ||
| needleRef?: React.RefObject<SVGSVGElement | null> | ||
| }) { | ||
| return ( | ||
| <Tooltip> | ||
|
|
@@ -480,7 +482,8 @@ function FloorplanCompassButton({ | |
| <span className="relative flex h-6 w-6 items-center justify-center rounded-full bg-[#b8b8b8] shadow-inner dark:bg-neutral-700"> | ||
| <svg | ||
| aria-hidden="true" | ||
| className="h-6 w-6 transition-transform duration-150 ease-out" | ||
| className="h-6 w-6" | ||
| ref={needleRef} | ||
| style={{ transform: `rotate(${northRotationDeg}deg)` }} | ||
| viewBox="0 0 48 48" | ||
| > | ||
|
|
@@ -5078,6 +5081,7 @@ export function FloorplanPanel({ | |
| const latestNavigationSyncPoseRef = useRef<NavigationSyncPose | null>( | ||
| useEditor.getState().navigationSyncPose, | ||
| ) | ||
| const compassNeedleRef = useRef<SVGSVGElement | null>(null) | ||
| const levelId = useViewer((state) => state.selection.levelId) | ||
| const buildingId = useViewer((state) => state.selection.buildingId) | ||
| const selectedZoneId = useViewer((state) => state.selection.zoneId) | ||
|
|
@@ -5177,7 +5181,11 @@ export function FloorplanPanel({ | |
| const buildingRotationDeg = (buildingRotationY * 180) / Math.PI | ||
| const floorplanSceneRotationDeg = | ||
| FLOORPLAN_VIEW_ROTATION_DEG + floorplanUserRotationDeg - buildingRotationDeg | ||
| latestFloorplanUserRotationDegRef.current = floorplanUserRotationDeg | ||
| // Only sync ref from state when floorplan is open (state is source of truth). | ||
| // When hidden, the imperative 3D path owns the ref and must not be clobbered. | ||
| if (isFloorplanOpenRef.current) { | ||
| latestFloorplanUserRotationDegRef.current = floorplanUserRotationDeg | ||
| } | ||
|
|
||
| // Draft START points stay in panel state (set per click). The live END points | ||
| // are the per-move hot values — they live in `useFloorplanDraftPreview` so a | ||
|
|
@@ -6539,19 +6547,18 @@ export function FloorplanPanel({ | |
| ) | ||
|
|
||
| useEffect(() => { | ||
| if (!isFloorplanOpen) return | ||
|
|
||
| const pose = useEditor.getState().navigationSyncPose | ||
| if (!pose) { | ||
| return | ||
| } | ||
|
|
||
| latestNavigationSyncPoseRef.current = pose | ||
| if (pose.source === '3d') { | ||
| // Re-runs when the panel reopens (`isFloorplanOpen`) so the viewport | ||
| // catches up to the camera after the per-frame sync was skipped while | ||
| // hidden; a no-op while closed (the sync early-returns). | ||
| syncFloorplanViewportToNavigationPose(pose) | ||
| } | ||
| }, [syncFloorplanViewportToNavigationPose]) | ||
| }, [syncFloorplanViewportToNavigationPose, isFloorplanOpen]) | ||
|
|
||
| useEffect(() => { | ||
| return useEditor.subscribe((state) => { | ||
|
|
@@ -6563,11 +6570,35 @@ export function FloorplanPanel({ | |
| latestNavigationSyncPoseRef.current = pose | ||
|
|
||
| if (pose.source === '3d') { | ||
| if (!isFloorplanOpenRef.current) { | ||
| // Panel hidden — drive the compass needle imperatively without | ||
| // triggering React state (setViewport) that would re-render the | ||
| // full floorplan SVG every camera frame. | ||
| const nextDeg = floorplanRotationFromCameraAzimuth( | ||
| pose.azimuth, | ||
| latestFloorplanUserRotationDegRef.current, | ||
| ) | ||
| latestFloorplanUserRotationDegRef.current = nextDeg | ||
| if (compassNeedleRef.current) { | ||
| compassNeedleRef.current.style.transform = `rotate(${nextDeg}deg)` | ||
| } | ||
| return | ||
| } | ||
| syncFloorplanViewportToNavigationPose(pose) | ||
| } | ||
| }) | ||
| }, [syncFloorplanViewportToNavigationPose]) | ||
|
|
||
| // When the panel is hidden the imperative path owns the compass needle. | ||
| // React re-renders can overwrite the needle's inline transform with stale | ||
| // state; this layout effect restores the authoritative ref value before | ||
| // the browser paints so the needle never visibly snaps to a stale angle. | ||
| useLayoutEffect(() => { | ||
| if (!isFloorplanOpen && compassNeedleRef.current) { | ||
| compassNeedleRef.current.style.transform = `rotate(${latestFloorplanUserRotationDegRef.current}deg)` | ||
| } | ||
| }) | ||
|
|
||
| useEffect(() => { | ||
| const host = viewportHostRef.current | ||
| if (!host) { | ||
|
|
@@ -7345,6 +7376,25 @@ export function FloorplanPanel({ | |
| ) | ||
|
|
||
| const alignFloorplanViewToNorth = useCallback(() => { | ||
| if (!isFloorplanOpenRef.current) { | ||
| // Panel hidden — derive from the live 3D camera pose and publish | ||
| // directly. The compass animates via the imperative subscription as | ||
| // the 3D camera transitions. | ||
| const pose = latestNavigationSyncPoseRef.current | ||
| if (!pose) return | ||
| const currentRotation = latestFloorplanUserRotationDegRef.current | ||
| const northAzimuth = cameraAzimuthFromFloorplanRotation( | ||
| nearestEquivalentDegrees(0, currentRotation), | ||
| ) | ||
| useEditor.getState().publishNavigationSyncPose({ | ||
| source: '2d', | ||
| target: [...pose.target], | ||
| azimuth: northAzimuth, | ||
| viewWidth: pose.viewWidth, | ||
| }) | ||
| return | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compass stuck after 3D northMedium Severity After an 'align to north' action, the compass needle and floorplan view can become stale or jump. The action publishes a Additional Locations (1)Reviewed by Cursor Bugbot for commit d4f1053. Configure here. |
||
| } | ||
|
|
||
| const currentViewport = latestViewportRef.current ?? latestFittedViewportRef.current | ||
| if (!currentViewport) { | ||
| return | ||
|
|
@@ -10761,13 +10811,15 @@ export function FloorplanPanel({ | |
| (compassHost ? ( | ||
| createPortal( | ||
| <FloorplanCompassButton | ||
| needleRef={compassNeedleRef} | ||
| northRotationDeg={floorplanUserRotationDeg} | ||
| onAlignNorth={alignFloorplanViewToNorth} | ||
| />, | ||
| compassHost, | ||
| ) | ||
| ) : ( | ||
| <FloorplanCompassButton | ||
| needleRef={compassNeedleRef} | ||
| northRotationDeg={floorplanUserRotationDeg} | ||
| onAlignNorth={alignFloorplanViewToNorth} | ||
| /> | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale compass on panel reopen
Medium Severity
When the 2D panel opens after 3D-only orbit, the first render copies stale
floorplanUserRotationDegstate intolatestFloorplanUserRotationDegRefand reapplies it on the compass via Reactstyle, overwriting the live imperative rotation. Catch-up runs in a lateruseEffect, so the needle (and floorplan rotation) can visibly snap or flash wrong for at least one paint.Additional Locations (1)
packages/editor/src/components/editor/floorplan-panel.tsx#L6595-L6600Reviewed by Cursor Bugbot for commit d4f1053. Configure here.