feat: add 2D canvas rendering for shape indicators#7708
Conversation
Implements canvas-based rendering for shape indicators with indicator2d methods across all shape utilities.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
|
API Changes Check Passed Great! The PR description now includes the required "### API changes" section. This helps reviewers and SDK users understand the impact of your changes. |
packages/editor/src/lib/components/default-components/CanvasShapeIndicators.tsx
Outdated
Show resolved
Hide resolved
packages/editor/src/lib/components/default-components/CanvasShapeIndicators.tsx
Show resolved
Hide resolved
packages/editor/src/lib/components/default-components/CanvasShapeIndicators.tsx
Outdated
Show resolved
Hide resolved
…il method for cheap indicator check. better logging
|
@ds300 added the computed cache, computed properties and a new method in the I've also added better perf logging using the chrome perf api. So you can now run a trace and look in the "Timings" tab and you'll see our traces there! pretty cool I think. 2026-01-21.at.17.52.52.-.Amber.Aardvark_3x.mp4all the logging and debug flags will be removed when we merge. |
packages/editor/src/lib/components/default-components/CanvasShapeIndicators.tsx
Outdated
Show resolved
Hide resolved
packages/editor/src/lib/components/default-components/DefaultShapeIndicators.tsx
Outdated
Show resolved
Hide resolved
Corrects the filter condition to exclude shapes that do not exist, ensuring only valid shapes are processed for legacy indicators.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| rSelectedColor.current = null | ||
| }, 0) | ||
| return () => clearTimeout(timer) | ||
| }, [isDarkMode, editor]) |
There was a problem hiding this comment.
Canvas indicators don't repaint after dark mode toggle
Medium Severity
When dark mode toggles, the useEffect clears the cached color on the next tick, but the useQuickReactor that repaints the canvas only runs when its dependencies (editor, $renderData) change. Since neither changes on dark mode toggle, the canvas indicators continue displaying the old theme's color until some other interaction (like selection or camera movement) triggers a repaint.
| const hovered = editor.getHoveredShapeId() | ||
| if (hovered) idsToDisplay.add(hovered) | ||
| } | ||
| } |
There was a problem hiding this comment.
Duplicated indicator display logic between components
Medium Severity
The logic for computing which shape IDs to display is duplicated between CanvasShapeIndicators and DefaultShapeIndicators. Both components independently check isChangingStyle, isIdleOrEditing (same editor.isInAny call), isInSelectState (same five state paths), and apply identical logic for adding selected and hovered shape IDs. This duplicated logic increases maintenance burden and risks inconsistent behavior if one is updated but not the other.
Additional Locations (1)
| : getDot(allPointsFromSegments[0], sw) | ||
|
|
||
| return new Path2D(solidStrokePath) | ||
| } |
There was a problem hiding this comment.
Duplicated path computation in indicator and getIndicatorPath methods
Medium Severity
Multiple shape utilities duplicate their indicator path computation logic between the indicator() method and the new getIndicatorPath() method. For example, DrawShapeUtil has nearly identical code in both methods: computing allPointsFromSegments, forceSolid, sw, strokePoints, and solidStrokePath. The only difference is the return type (<path d={...} /> vs new Path2D(...)). This pattern repeats in HighlightShapeUtil, GeoShapeUtil, and LineShapeUtil.
Additional Locations (1)
|
Good heavens. This is even better than I thought it would be. Fantastic work @kostyafarber! |
|
I'm seeing huge improvements across basically everything, including just dragging lots of shapes around (35ms -> 25ms in this casual test). I didn't realize just how much was being spent on the signals / hooks around indicators. |
After the 2D canvas indicator rendering change (#7708), collaborator shape indicators were no longer being rendered. This was because CanvasShapeIndicators only rendered the current user's selected shapes, while collaborator indicators went through the SVG-based CollaboratorShapeIndicator which now returns null for canvas-enabled shapes. This PR fixes the issue by: - Adding collaborator indicator rendering to CanvasShapeIndicators - Extracting shared collaborator state logic to collaboratorState.ts - Updating LiveCollaborators to only render SVG indicators for shapes that use legacy indicators (for backwards compatibility with custom shapes)
After the 2D canvas indicator rendering change (#7708), collaborator shape indicators were no longer being rendered. This was because `CanvasShapeIndicators` only rendered the current user's selected shapes, while collaborator indicators went through the SVG-based `CollaboratorShapeIndicator` which now returns null for canvas-enabled shapes. This PR fixes the issue by: - Adding collaborator indicator rendering to `CanvasShapeIndicators` with per-collaborator colors and 0.5 opacity - Extracting shared collaborator state logic to `collaboratorState.ts` to avoid duplication - Updating `LiveCollaborators` to only render SVG indicators for shapes that use legacy indicators (for backwards compatibility with custom shapes) Relates to #7437 ### Change type - [x] `bugfix` ### Test plan 1. Open a multiplayer session with two users 2. Have one user select shapes 3. Verify the other user sees the collaborator's selection indicators with the collaborator's color - [ ] Unit tests - [ ] End to end tests ### Release notes - Fixed collaborator shape indicators not rendering after the canvas indicator change <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Restores collaborator selection indicators after moving shape indicators to the canvas renderer. > > - Renders collaborator shape indicators in `CanvasShapeIndicators` with per-collaborator color and 0.7 opacity, drawn beneath local indicators > - Adds `useActivePeerIds$` in `usePeerIds` to track which collaborators should be shown based on timed activity transitions > - Extracts shared collaborator logic to `utils/collaboratorState.ts` (`getCollaboratorStateFromElapsedTime`, `shouldShowCollaborator`) > - Updates `LiveCollaborators` to use `shouldShowCollaborator` and only render SVG indicators for shapes using legacy indicators (canvas handles the rest) > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ce0f1f9. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Implements canvas-based rendering for shape indicators with indicator2d methods across all shape utilities.
ShapeUtiland a new type to allow for more complex shape drawing (arrows require clip paths for the label etc)I've also appended a snippet that you can run in the console which will create a bunch of shapes to test out the new feature too.
pr-7708-walkthrough.mp4
create shapes snippet
Change type
bugfiximprovementfeatureapiotherAPI changes
getIndicatorPath()for canvas indicatorsTLIndicatorPathfor the return canvas 2d pathsNote
Introduces 2D canvas rendering for shape indicators and migrates built-in shapes to it, reducing DOM overhead.
CanvasShapeIndicatorscanvas overlay hooked intoDefaultCanvas; legacy SVG indicators are now rendered only whenShapeUtil.useLegacyIndicator()returnstrueShapeUtil.getIndicatorPath(shape)anduseLegacyIndicator(), plusTLIndicatorPathtype andPathBuilder.toPath2Dfor convertingPathBuildertoPath2Darrowwith clip support,bookmark,draw,embed,frame,geo,highlight,image,line,note,text,video); each overridesuseLegacyIndicator()tofalse.tl-canvas-indicatorslayer; tests updated to assert its presence instead of per-shape SVGsPath2D.roundRectpolyfill in Vitest setupapi-reportchanges,index.tsexportsTLIndicatorPath)Written by Cursor Bugbot for commit 0eabd14. This will update automatically on new commits. Configure here.