[improvement] Improve rotation (#94)

* Fix rotation handle, rotation for arrows and shapes with handles

* Fix bug on draw bounds when cloning

packages/core/src/components/bounds/bounds.tsx

@@ -23,14 +23,22 @@ export function Bounds({
   rotation,
   isLocked,
 }: BoundsProps): JSX.Element {
-  const targetSize = (viewportWidth < 768 ? 16 : 8) / zoom // Touch target size
+  // Touch target size
-  const size = 8 / zoom // Touch target size
+  const targetSize = (viewportWidth < 768 ? 16 : 8) / zoom
+  // Handle size
+  const size = 8 / zoom
+
+  const smallDimension = Math.min(bounds.width, bounds.height) * zoom
+  // If the bounds are small, don't show the rotate handle
+  const showRotateHandle = !isLocked && smallDimension > 32
+  // If the bounds are very small, don't show the corner handles
+  const showHandles = !isLocked && smallDimension > 16
 
   return (
     <Container bounds={bounds} rotation={rotation}>
       <SVGContainer>
         <CenterHandle bounds={bounds} isLocked={isLocked} />
-        {!isLocked && (
+        {showHandles && (
           <>
             <EdgeHandle
               targetSize={targetSize}
@@ -80,7 +88,9 @@ export function Bounds({
               bounds={bounds}
               corner={TLBoundsCorner.BottomLeft}
             />
-            <RotateHandle targetSize={targetSize} size={size} bounds={bounds} />
+            {showRotateHandle && (
+              <RotateHandle targetSize={targetSize} size={size} bounds={bounds} />
+            )}
           </>
         )}
       </SVGContainer>

packages/core/src/components/bounds/rotate-handle.tsx

@@ -18,7 +18,7 @@ export const RotateHandle = React.memo(
           className="tl-transparent"
           cx={bounds.width / 2}
           cy={size * -2}
-          r={targetSize * 2}
+          r={targetSize}
           pointerEvents="all"
           {...events}
         />

packages/core/src/utils/utils.ts

@@ -448,7 +448,7 @@ export class Utils {
    * @param r
    * @param segments
    */
-  static clampToRotationToSegments(r: number, segments: number): number {
+  static snapAngleToSegments(r: number, segments: number): number {
     const seg = (Math.PI * 2) / segments
     return Math.floor((Utils.clampRadians(r) + seg / 2) / seg) * seg
   }

packages/tldraw/src/shape/shapes/arrow/arrow.tsx

@@ -348,11 +348,11 @@ export const Arrow = new ShapeUtil<ArrowShape, SVGSVGElement, TLDrawMeta>(() =>
 
     nextHandles['bend'] = {
       ...bend,
-      point: Math.abs(bendDist) < 10 ? midPoint : point,
+      point: Vec.round(Math.abs(bendDist) < 10 ? midPoint : point),
     }
 
     return {
-      point: [bounds.minX, bounds.minY],
+      point: Vec.round([bounds.minX, bounds.minY]),
       handles: nextHandles,
     }
   },
@@ -486,7 +486,7 @@ export const Arrow = new ShapeUtil<ArrowShape, SVGSVGElement, TLDrawMeta>(() =>
         const other = handle.id === 'start' ? shape.handles.end : shape.handles.start
         const angle = Vec.angle(other.point, point)
         const distance = Vec.dist(other.point, point)
-        const newAngle = Utils.clampToRotationToSegments(angle, 24)
+        const newAngle = Utils.snapAngleToSegments(angle, 24)
         handle.point = Vec.nudgeAtAngle(other.point, newAngle, distance)
       }
     })

packages/tldraw/src/shape/shapes/draw/draw.tsx

@@ -10,7 +10,7 @@ import { EASINGS } from '~state/utils'
 const pointsBoundsCache = new WeakMap<DrawShape['points'], TLBounds>([])
 const shapeBoundsCache = new Map<string, TLBounds>()
 const rotatedCache = new WeakMap<DrawShape, number[][]>([])
-const pointCache = new WeakSet<DrawShape['point']>([])
+const pointCache: Record<string, number[]> = {}
 
 export const Draw = new ShapeUtil<DrawShape, SVGSVGElement, TLDrawMeta>(() => ({
   type: TLDrawShapeType.Draw,
@@ -167,17 +167,17 @@ export const Draw = new ShapeUtil<DrawShape, SVGSVGElement, TLDrawMeta>(() => ({
     // previous bounds-from-points result if we can.
 
     const pointsHaveChanged = !pointsBoundsCache.has(shape.points)
-    const pointHasChanged = !pointCache.has(shape.point)
+    const pointHasChanged = !(pointCache[shape.id] === shape.point)
 
     if (pointsHaveChanged) {
       // If the points have changed, then bust the points cache
       const bounds = Utils.getBoundsFromPoints(shape.points)
       pointsBoundsCache.set(shape.points, bounds)
       shapeBoundsCache.set(shape.id, Utils.translateBounds(bounds, shape.point))
-      pointCache.add(shape.point)
+      pointCache[shape.id] = shape.point
     } else if (pointHasChanged && !pointsHaveChanged) {
       // If the point have has changed, then bust the point cache
-      pointCache.add(shape.point)
+      pointCache[shape.id] = shape.point
       shapeBoundsCache.set(
         shape.id,
         // eslint-disable-next-line @typescript-eslint/no-non-null-assertion

packages/tldraw/src/shape/shapes/ellipse/ellipse.tsx

@@ -180,6 +180,7 @@ export const Ellipse = new ShapeUtil<EllipseShape, SVGSVGElement, TLDrawMeta>(()
 
   hitTestBounds(shape, bounds) {
     return (
+      Utils.boundsContain(bounds, this.getBounds(shape)) ||
       intersectBoundsEllipse(
         bounds,
         this.getCenter(shape),

packages/tldraw/src/shape/shapes/group/group.tsx

@@ -86,20 +86,42 @@ export const Group = new ShapeUtil<GroupShape, SVGSVGElement, TLDrawMeta>(() =>
   },
 
   Indicator({ shape }) {
-    const [width, height] = shape.size
+    const { id, size } = shape
 
     const sw = 2
+    const w = Math.max(0, size[0] - sw / 2)
+    const h = Math.max(0, size[1] - sw / 2)
 
-    return (
+    const strokes: [number[], number[], number][] = [
-      <rect
+      [[sw / 2, sw / 2], [w, sw / 2], w - sw / 2],
-        x={sw / 2}
+      [[w, sw / 2], [w, h], h - sw / 2],
-        y={sw / 2}
+      [[w, h], [sw / 2, h], w - sw / 2],
-        rx={1}
+      [[sw / 2, h], [sw / 2, sw / 2], h - sw / 2],
-        ry={1}
+    ]
-        width={Math.max(1, width - sw)}
+
-        height={Math.max(1, height - sw)}
+    const paths = strokes.map(([start, end, length], i) => {
-      />
+      const { strokeDasharray, strokeDashoffset } = getPerfectDashProps(
-    )
+        length,
+        sw,
+        DashStyle.Dotted
+      )
+
+      return (
+        <line
+          key={id + '_' + i}
+          x1={start[0]}
+          y1={start[1]}
+          x2={end[0]}
+          y2={end[1]}
+          strokeWidth={sw}
+          strokeLinecap="round"
+          strokeDasharray={strokeDasharray}
+          strokeDashoffset={strokeDashoffset}
+        />
+      )
+    })
+
+    return <g>{paths}</g>
   },
 
   shouldRender(prev, next) {

packages/tldraw/src/state/command/rotate/rotate.command.ts

@@ -1,48 +1,46 @@
 import { Utils } from '@tldraw/core'
 import { Vec } from '@tldraw/vec'
-import type { TLDrawCommand, Data } from '~types'
+import type { TLDrawCommand, Data, TLDrawShape } from '~types'
 import { TLDR } from '~state/tldr'
 
 const PI2 = Math.PI * 2
 
-export function rotate(data: Data, ids: string[], delta = -PI2 / 4): TLDrawCommand {
+export function rotate(data: Data, ids: string[], delta = -PI2 / 4): TLDrawCommand | void {
   const { currentPageId } = data.appState
-  const initialShapes = ids.map((id) => TLDR.getShape(data, id, currentPageId))
 
-  const boundsForShapes = initialShapes.map((shape) => {
+  // The shapes for the before patch
-    const utils = TLDR.getShapeUtils(shape)
+  const before: Record<string, Partial<TLDrawShape>> = {}
-    return {
+
-      id: shape.id,
+  // The shapes for the after patch
-      point: [...shape.point],
+  const after: Record<string, Partial<TLDrawShape>> = {}
-      bounds: utils.getBounds(shape),
+
-      center: utils.getCenter(shape),
+  // Find the shapes that we want to rotate.
-      rotation: shape.rotation,
+  // We don't rotate groups: we rotate their children instead.
-    }
+  const shapesToRotate = ids.flatMap((id) => {
+    const shape = TLDR.getShape(data, id, currentPageId)
+    return shape.children
+      ? shape.children.map((childId) => TLDR.getShape(data, childId, currentPageId))
+      : shape
   })
 
-  const commonBounds = Utils.getCommonBounds(boundsForShapes.map(({ bounds }) => bounds))
+  // Find the common center to all shapes
-  const commonBoundsCenter = Utils.getBoundsCenter(commonBounds)
+  // This is the point that we'll rotate around
+  const origin = shapesToRotate.reduce((acc, shape) => {
+    return Vec.med(acc, TLDR.getCenter(shape))
+  }, TLDR.getCenter(shapesToRotate[0]))
 
-  const rotations = Object.fromEntries(
+  // Find the rotate mutations for each shape
-    boundsForShapes.map(({ id, point, center, rotation }) => {
+  shapesToRotate.forEach((shape) => {
-      const offset = Vec.sub(center, point)
+    const change = TLDR.getRotatedShapeMutation(shape, TLDR.getCenter(shape), origin, delta)
-      const nextPoint = Vec.sub(Vec.rotWith(center, commonBoundsCenter, -(PI2 / 4)), offset)
+    if (!change) return
-      const nextRotation = (PI2 + ((rotation || 0) + delta)) % PI2
+    before[shape.id] = TLDR.getBeforeShape(shape, change)
+    after[shape.id] = change
+  })
 
-      return [id, { point: nextPoint, rotation: nextRotation }]
+  // Also rotate the bounds.
-    })
+  const beforeBoundsRotation = TLDR.getPageState(data, currentPageId).boundsRotation
-  )
 
-  const pageState = TLDR.getPageState(data, currentPageId)
+  const afterBoundsRotation = Utils.clampRadians((beforeBoundsRotation || 0) + delta)
-  const prevBoundsRotation = pageState.boundsRotation
-  const nextBoundsRotation = (PI2 + ((pageState.boundsRotation || 0) + delta)) % PI2
-
-  const { before, after } = TLDR.mutateShapes(
-    data,
-    ids,
-    (shape) => rotations[shape.id],
-    currentPageId
-  )
 
   return {
     id: 'rotate',
@@ -52,7 +50,7 @@ export function rotate(data: Data, ids: string[], delta = -PI2 / 4): TLDrawComma
           [currentPageId]: { shapes: before },
         },
         pageStates: {
-          [currentPageId]: { boundsRotation: prevBoundsRotation },
+          [currentPageId]: { boundsRotation: beforeBoundsRotation },
         },
       },
     },
@@ -62,7 +60,7 @@ export function rotate(data: Data, ids: string[], delta = -PI2 / 4): TLDrawComma
           [currentPageId]: { shapes: after },
         },
         pageStates: {
-          [currentPageId]: { boundsRotation: nextBoundsRotation },
+          [currentPageId]: { boundsRotation: afterBoundsRotation },
         },
       },
     },

packages/tldraw/src/state/session/sessions/rotate/rotate.session.ts

@@ -4,7 +4,7 @@ import { Session, TLDrawShape, TLDrawStatus } from '~types'
 import type { Data } from '~types'
 import { TLDR } from '~state/tldr'
 
-const PI2 = Math.PI * 2
+const centerCache = new WeakMap<string[], number[]>()
 
 export class RotateSession implements Session {
   id = 'rotate'
@@ -12,48 +12,45 @@ export class RotateSession implements Session {
   delta = [0, 0]
   origin: number[]
   snapshot: RotateSnapshot
-  prev = 0
+  initialAngle: number
+  changes: Record<string, Partial<TLDrawShape>> = {}
 
   constructor(data: Data, point: number[]) {
     this.origin = point
     this.snapshot = getRotateSnapshot(data)
+    this.initialAngle = Vec.angle(this.snapshot.commonBoundsCenter, this.origin)
   }
 
   start = () => void null
 
   update = (data: Data, point: number[], isLocked = false) => {
     const { commonBoundsCenter, initialShapes } = this.snapshot
+
     const pageId = data.appState.currentPageId
-    const pageState = TLDR.getPageState(data, pageId)
 
     const shapes: Record<string, Partial<TLDrawShape>> = {}
 
-    const a1 = Vec.angle(commonBoundsCenter, this.origin)
+    const nextDirection = Vec.angle(commonBoundsCenter, point) - this.initialAngle
-    const a2 = Vec.angle(commonBoundsCenter, point)
 
-    let rot = a2 - a1
+    let nextBoundsRotation = this.snapshot.boundsRotation + nextDirection
-
-    this.prev = rot
 
     if (isLocked) {
-      rot = Utils.clampToRotationToSegments(rot, 24)
+      nextBoundsRotation = Utils.snapAngleToSegments(nextBoundsRotation, 24)
     }
 
-    pageState.boundsRotation = (PI2 + (this.snapshot.boundsRotation + rot)) % PI2
+    const delta = nextBoundsRotation - this.snapshot.boundsRotation
 
-    initialShapes.forEach(({ id, center, offset, shape: { rotation = 0 } }) => {
+    // Update the shapes
-      const nextRotation = isLocked
+    initialShapes.forEach(({ id, center, shape }) => {
-        ? Utils.clampToRotationToSegments(rotation + rot, 24)
+      const change = TLDR.getRotatedShapeMutation(shape, center, commonBoundsCenter, delta)
-        : rotation + rot
 
-      const nextPoint = Vec.sub(Vec.rotWith(center, commonBoundsCenter, rot), offset)
+      if (change) {
-
+        shapes[id] = change
-      shapes[id] = {
-        point: nextPoint,
-        rotation: (PI2 + nextRotation) % PI2,
       }
     })
 
+    this.changes = shapes
+
     return {
       document: {
         pages: {
@@ -61,6 +58,9 @@ export class RotateSession implements Session {
             shapes,
           },
         },
+        pageState: {
+          boundsRotation: Utils.clampRadians(nextBoundsRotation),
+        },
       },
     }
   }
@@ -87,18 +87,16 @@ export class RotateSession implements Session {
   }
 
   complete(data: Data) {
-    const { hasUnlockedShapes, initialShapes } = this.snapshot
+    const { initialShapes } = this.snapshot
     const pageId = data.appState.currentPageId
 
-    if (!hasUnlockedShapes) return data
+    // if (!hasUnlockedShapes) return data
 
     const beforeShapes = {} as Record<string, Partial<TLDrawShape>>
-    const afterShapes = {} as Record<string, Partial<TLDrawShape>>
+    const afterShapes = this.changes
 
-    initialShapes.forEach(({ id, shape: { point, rotation } }) => {
+    initialShapes.forEach(({ id, shape: { point, rotation, handles } }) => {
-      beforeShapes[id] = { point, rotation }
+      beforeShapes[id] = { point, rotation, handles }
-      const afterShape = TLDR.getShape(data, id, pageId)
-      afterShapes[id] = { point: afterShape.point, rotation: afterShape.rotation }
     })
 
     return {
@@ -131,42 +129,30 @@ export function getRotateSnapshot(data: Data) {
   const pageState = TLDR.getPageState(data, currentPageId)
   const initialShapes = TLDR.getSelectedBranchSnapshot(data, currentPageId)
 
-  if (initialShapes.length === 0) {
+  const commonBoundsCenter = Utils.getFromCache(centerCache, pageState.selectedIds, () => {
-    throw Error('No selected shapes!')
+    if (initialShapes.length === 0) {
-  }
+      throw Error('No selected shapes!')
+    }
 
-  const hasUnlockedShapes = initialShapes.length > 0
+    const shapesBounds = Object.fromEntries(
+      initialShapes.map((shape) => [shape.id, TLDR.getBounds(shape)])
+    )
 
-  const shapesBounds = Object.fromEntries(
+    const bounds = Utils.getCommonBounds(Object.values(shapesBounds))
-    initialShapes.map((shape) => [shape.id, TLDR.getBounds(shape)])
-  )
 
-  const rotatedBounds = Object.fromEntries(
+    return Utils.getBoundsCenter(bounds)
-    initialShapes.map((shape) => [shape.id, TLDR.getRotatedBounds(shape)])
+  })
-  )
-
-  const bounds = Utils.getCommonBounds(Object.values(shapesBounds))
-
-  const commonBoundsCenter = Utils.getBoundsCenter(bounds)
 
   return {
-    hasUnlockedShapes,
     boundsRotation: pageState.boundsRotation || 0,
     commonBoundsCenter,
     initialShapes: initialShapes
       .filter((shape) => shape.children === undefined)
       .map((shape) => {
-        const bounds = TLDR.getBounds(shape)
+        const center = TLDR.getShapeUtils(shape).getCenter(shape)
-        const center = Utils.getBoundsCenter(bounds)
-        const offset = Vec.sub(center, shape.point)
-
-        const rotationOffset = Vec.sub(center, Utils.getBoundsCenter(rotatedBounds[shape.id]))
-
         return {
           id: shape.id,
-          shape: Utils.deepClone(shape),
+          shape,
-          offset,
-          rotationOffset,
           center,
         }
       }),

packages/tldraw/src/state/session/sessions/translate/translate.session.ts

@@ -10,6 +10,7 @@ import {
   TLDrawStatus,
   ArrowShape,
   GroupShape,
+  TLDrawPatch,
 } from '~types'
 import { TLDR } from '~state/tldr'
 import type { Patch } from 'rko'
@@ -115,14 +116,14 @@ export class TranslateSession implements Session {
       }
 
       // Either way, move the clones
-      clones.forEach((shape) => {
+      clones.forEach((clone) => {
-        const current = (nextShapes[shape.id] ||
+        const current = (nextShapes[clone.id] ||
-          TLDR.getShape(data, shape.id, data.appState.currentPageId)) as TLDrawShape
+          TLDR.getShape(data, clone.id, data.appState.currentPageId)) as TLDrawShape
 
         if (!current.point) throw Error('No point on that clone!')
 
-        nextShapes[shape.id] = {
+        nextShapes[clone.id] = {
-          ...nextShapes[shape.id],
+          ...nextShapes[clone.id],
           point: Vec.round(Vec.add(current.point, trueDelta)),
         }
       })
@@ -391,12 +392,14 @@ export function getTranslateSnapshot(data: Data) {
 
     cloneMap[shape.id] = newId
 
-    clones.push({
+    const clone = {
       ...Utils.deepClone(shape),
       id: newId,
       parentId: shape.parentId,
       childIndex: TLDR.getChildIndexAbove(data, shape.id, currentPageId),
-    })
+    }
+
+    clones.push(clone)
   })
 
   clones.forEach((clone) => {
@@ -425,6 +428,7 @@ export function getTranslateSnapshot(data: Data) {
       if (clonedShapeIds.has(binding.fromId)) {
         if (clonedShapeIds.has(binding.toId)) {
           const cloneId = Utils.uniqueId()
+
           const cloneBinding = {
             ...Utils.deepClone(binding),
             id: cloneId,

packages/tldraw/src/state/tldr.ts

@@ -79,6 +79,10 @@ export class TLDR {
     return TLDR.getPage(data, pageId).shapes[shapeId] as T
   }
 
+  static getCenter<T extends TLDrawShape>(shape: T) {
+    return TLDR.getShapeUtils(shape).getCenter(shape)
+  }
+
   static getBounds<T extends TLDrawShape>(shape: T) {
     return TLDR.getShapeUtils(shape).getBounds(shape)
   }
@@ -308,6 +312,12 @@ export class TLDR {
   /*                      Mutations                     */
   /* -------------------------------------------------- */
 
+  static getBeforeShape<T extends TLDrawShape>(shape: T, change: Partial<T>): Partial<T> {
+    return Object.fromEntries(
+      Object.keys(change).map((k) => [k, shape[k as keyof T]])
+    ) as Partial<T>
+  }
+
   static mutateShapes<T extends TLDrawShape>(
     data: Data,
     ids: string[],
@@ -325,9 +335,7 @@ export class TLDR {
       const shape = TLDR.getShape<T>(data, id, pageId)
       const change = fn(shape, i)
       if (change) {
-        beforeShapes[id] = Object.fromEntries(
+        beforeShapes[id] = TLDR.getBeforeShape(shape, change)
-          Object.keys(change).map((key) => [key, shape[key as keyof T]])
-        ) as Partial<T>
         afterShapes[id] = change
       }
     })
@@ -559,6 +567,61 @@ export class TLDR {
     return { ...shape, ...delta }
   }
 
+  /**
+   * Rotate a shape around an origin point.
+   * @param shape a shape.
+   * @param center the shape's center in page space.
+   * @param origin the page point to rotate around.
+   * @param rotation the amount to rotate the shape.
+   */
+  static getRotatedShapeMutation<T extends TLDrawShape>(
+    shape: T, // in page space
+    center: number[], // in page space
+    origin: number[], // in page space (probably the center of common bounds)
+    delta: number // The shape's rotation delta
+  ): Partial<T> | void {
+    // The shape's center relative to the shape's point
+    const relativeCenter = Vec.sub(center, shape.point)
+
+    // Rotate the center around the origin
+    const rotatedCenter = Vec.rotWith(center, origin, delta)
+
+    // Get the top left point relative to the rotated center
+    const nextPoint = Vec.round(Vec.sub(rotatedCenter, relativeCenter))
+
+    // If the shape has handles, we need to rotate the handles instead
+    // of rotating the shape. Shapes with handles should never be rotated,
+    // because that makes a lot of other things incredible difficult.
+    if (shape.handles !== undefined) {
+      const change = this.getShapeUtils(shape).onHandleChange(
+        // Base the change on a shape with the next point
+        { ...shape, point: nextPoint },
+        Object.fromEntries(
+          Object.entries(shape.handles).map(([handleId, handle]) => {
+            // Rotate each handle's point around the shape's center
+            // (in relative shape space, as the handle's point will be).
+            const point = Vec.round(Vec.rotWith(handle.point, relativeCenter, delta))
+            return [handleId, { ...handle, point }]
+          })
+        ) as T['handles'],
+        { shiftKey: false }
+      )
+
+      return change
+    }
+
+    // If the shape has no handles, move the shape to the new point
+    // and set the rotation.
+
+    // Clamp the next rotation between 0 and PI2
+    const nextRotation = Utils.clampRadians((shape.rotation || 0) + delta)
+
+    return {
+      point: nextPoint,
+      rotation: nextRotation,
+    } as Partial<T>
+  }
+
   /* -------------------------------------------------- */
   /*                       Parents                      */
   /* -------------------------------------------------- */

packages/tldraw/src/state/tlstate.ts

@@ -582,7 +582,7 @@ export class TLDrawState extends StateManager<Data> {
    * @todo
    */
   saveProject = () => {
-    // TODO
+    this.persist()
   }
 
   /**
@@ -1906,7 +1906,9 @@ export class TLDrawState extends StateManager<Data> {
    */
   rotate = (delta = Math.PI * -0.5, ids = this.selectedIds): this => {
     if (ids.length === 0) return this
-    return this.setState(Commands.rotate(this.state, ids, delta))
+    const change = Commands.rotate(this.state, ids, delta)
+    if (!change) return this
+    return this.setState(change)
   }
 
   /**

packages/tldraw/src/state/utils.ts

@@ -1,5 +1,7 @@
 import type { Easing } from '~types'
 
+export const PI2 = Math.PI * 2
+
 export const EASINGS: Record<Easing, (t: number) => number> = {
   linear: (t) => t,
   easeInQuad: (t) => t * t,