[improvement] Scope getShapeAtPoint to rendering shapes only (#2043)

This PR:

1. Adds a `renderingOnly` option to the `Editor.getShapeAtPoint` method.
When true, the method will only hit test against rendering shapes
(shapes that are inside of the current `renderingBounds`) rather than
all shapes on the canvas.
2. Includes some low level improvements to the way that edges find their
nearest point.
3. Includes a fix to circle geometry that could produce NaN values
 
### Change Type

- [x] `minor` — New feature

### Test Plan

1. Check whether hovering shapes still works as you would expect.

- [x] Unit Tests

### Release Notes

- Improve perf for hovering shapes / shape hit tests
This commit is contained in:
Steve Ruiz 2023-10-09 15:18:42 +01:00 committed by GitHub
parent 2a8987d8b9
commit 8892176b1a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 197 additions and 46 deletions

View file

@ -23,7 +23,9 @@ export class MicroSelectTool extends StateNode {
const hitShape =
hoveredShape && !this.editor.isShapeOfType<TLGroupShape>(hoveredShape, 'group')
? hoveredShape
: this.editor.getShapeAtPoint(this.editor.inputs.currentPagePoint)
: this.editor.getShapeAtPoint(this.editor.inputs.currentPagePoint, {
renderingOnly: true,
})
if (hitShape) {
this.onPointerDown({

View file

@ -31,7 +31,9 @@ class IdleState extends StateNode {
const hitShape =
hoveredShape && !this.editor.isShapeOfType<TLGroupShape>(hoveredShape, 'group')
? hoveredShape
: this.editor.getShapeAtPoint(this.editor.inputs.currentPagePoint)
: this.editor.getShapeAtPoint(this.editor.inputs.currentPagePoint, {
renderingOnly: true,
})
if (hitShape) {
this.onPointerDown({
...info,

View file

@ -535,7 +535,9 @@ export class Edge2d extends Geometry2d {
// (undocumented)
hitTestLineSegment(A: Vec2d, B: Vec2d, _zoom: number): boolean;
// (undocumented)
length: number;
get length(): number;
// (undocumented)
_length?: number;
// (undocumented)
midPoint(): Vec2d;
// (undocumented)
@ -544,6 +546,8 @@ export class Edge2d extends Geometry2d {
start: Vec2d;
// (undocumented)
u: Vec2d;
// (undocumented)
ul: number;
}
// @public (undocumented)
@ -608,6 +612,7 @@ export class Editor extends EventEmitter<TLEventMap> {
get currentPage(): TLPage;
get currentPageBounds(): Box2d | undefined;
get currentPageId(): TLPageId;
get currentPageRenderingShapesSorted(): TLShape[];
get currentPageShapeIds(): Set<TLShapeId>;
get currentPageShapes(): TLShape[];
get currentPageShapesSorted(): TLShape[];
@ -680,6 +685,7 @@ export class Editor extends EventEmitter<TLEventMap> {
getShapeAncestors(shape: TLShape | TLShapeId, acc?: TLShape[]): TLShape[];
getShapeAndDescendantIds(ids: TLShapeId[]): Set<TLShapeId>;
getShapeAtPoint(point: VecLike, opts?: {
renderingOnly?: boolean | undefined;
margin?: number | undefined;
hitInside?: boolean | undefined;
hitLabels?: boolean | undefined;

View file

@ -6156,6 +6156,36 @@
"name": "Edge2d",
"preserveMemberOrder": false,
"members": [
{
"kind": "Property",
"canonicalReference": "@tldraw/editor!Edge2d#_length:member",
"docComment": "",
"excerptTokens": [
{
"kind": "Content",
"text": "_length?: "
},
{
"kind": "Content",
"text": "number"
},
{
"kind": "Content",
"text": ";"
}
],
"isReadonly": false,
"isOptional": true,
"releaseTag": "Public",
"name": "_length",
"propertyTypeTokenRange": {
"startIndex": 1,
"endIndex": 2
},
"isStatic": false,
"isProtected": false,
"isAbstract": false
},
{
"kind": "Constructor",
"canonicalReference": "@tldraw/editor!Edge2d:constructor(1)",
@ -6393,7 +6423,7 @@
"excerptTokens": [
{
"kind": "Content",
"text": "length: "
"text": "get length(): "
},
{
"kind": "Content",
@ -6404,7 +6434,7 @@
"text": ";"
}
],
"isReadonly": false,
"isReadonly": true,
"isOptional": false,
"releaseTag": "Public",
"name": "length",
@ -6559,6 +6589,36 @@
"isStatic": false,
"isProtected": false,
"isAbstract": false
},
{
"kind": "Property",
"canonicalReference": "@tldraw/editor!Edge2d#ul:member",
"docComment": "",
"excerptTokens": [
{
"kind": "Content",
"text": "ul: "
},
{
"kind": "Content",
"text": "number"
},
{
"kind": "Content",
"text": ";"
}
],
"isReadonly": false,
"isOptional": false,
"releaseTag": "Public",
"name": "ul",
"propertyTypeTokenRange": {
"startIndex": 1,
"endIndex": 2
},
"isStatic": false,
"isProtected": false,
"isAbstract": false
}
],
"extendsTokenRange": {
@ -8037,6 +8097,41 @@
"isProtected": false,
"isAbstract": false
},
{
"kind": "Property",
"canonicalReference": "@tldraw/editor!Editor#currentPageRenderingShapesSorted:member",
"docComment": "/**\n * An array containing all of the rendering shapes in the current page, sorted in z-index order (accounting for nested shapes): e.g. A, B, BA, BB, C.\n *\n * @example\n * ```ts\n * editor.currentPageShapesSorted\n * ```\n *\n * @readonly @public\n */\n",
"excerptTokens": [
{
"kind": "Content",
"text": "get currentPageRenderingShapesSorted(): "
},
{
"kind": "Reference",
"text": "TLShape",
"canonicalReference": "@tldraw/tlschema!TLShape:type"
},
{
"kind": "Content",
"text": "[]"
},
{
"kind": "Content",
"text": ";"
}
],
"isReadonly": true,
"isOptional": false,
"releaseTag": "Public",
"name": "currentPageRenderingShapesSorted",
"propertyTypeTokenRange": {
"startIndex": 1,
"endIndex": 3
},
"isStatic": false,
"isProtected": false,
"isAbstract": false
},
{
"kind": "Property",
"canonicalReference": "@tldraw/editor!Editor#currentPageShapeIds:member",
@ -10851,7 +10946,7 @@
},
{
"kind": "Content",
"text": "{\n margin?: number | undefined;\n hitInside?: boolean | undefined;\n hitLabels?: boolean | undefined;\n hitFrameInside?: boolean | undefined;\n filter?: ((shape: "
"text": "{\n renderingOnly?: boolean | undefined;\n margin?: number | undefined;\n hitInside?: boolean | undefined;\n hitLabels?: boolean | undefined;\n hitFrameInside?: boolean | undefined;\n filter?: ((shape: "
},
{
"kind": "Reference",

View file

@ -4222,6 +4222,7 @@ export class Editor extends EventEmitter<TLEventMap> {
getShapeAtPoint(
point: VecLike,
opts = {} as {
renderingOnly?: boolean
margin?: number
hitInside?: boolean
hitLabels?: boolean
@ -4229,11 +4230,7 @@ export class Editor extends EventEmitter<TLEventMap> {
filter?: (shape: TLShape) => boolean
}
): TLShape | undefined {
const {
viewportPageBounds,
zoomLevel,
currentPageShapesSorted: sortedShapesOnCurrentPage,
} = this
const { viewportPageBounds, zoomLevel } = this
const {
filter,
margin = 0,
@ -4248,7 +4245,9 @@ export class Editor extends EventEmitter<TLEventMap> {
let inMarginClosestToEdgeDistance = Infinity
let inMarginClosestToEdgeHit: TLShape | null = null
const shapesToCheck = sortedShapesOnCurrentPage.filter((shape) => {
const shapesToCheck = (
opts.renderingOnly ? this.currentPageRenderingShapesSorted : this.currentPageShapesSorted
).filter((shape) => {
if (this.isShapeOfType(shape, 'group')) return false
const pageMask = this.getShapeMask(shape)
if (pageMask && !pointInPolygon(point, pageMask)) return false
@ -4320,7 +4319,9 @@ export class Editor extends EventEmitter<TLEventMap> {
distance = minDistance
} else {
distance = geometry.distanceToPoint(pointInShapeSpace, hitInside)
distance = geometry.bounds.containsPoint(pointInShapeSpace, margin)
? geometry.distanceToPoint(pointInShapeSpace, hitInside)
: Infinity
}
if (geometry.isClosed) {
@ -4537,6 +4538,26 @@ export class Editor extends EventEmitter<TLEventMap> {
return results
}
/**
* An array containing all of the rendering shapes in the current page, sorted in z-index order (accounting
* for nested shapes): e.g. A, B, BA, BB, C.
*
* @example
* ```ts
* editor.currentPageShapesSorted
* ```
*
* @readonly
*
* @public
*/
@computed get currentPageRenderingShapesSorted(): TLShape[] {
return this.renderingShapes
.filter(({ isCulled }) => !isCulled)
.sort((a, b) => a.index - b.index)
.map(({ shape }) => shape)
}
/**
* Get whether a shape matches the type of a TLShapeUtil.
*
@ -6446,7 +6467,7 @@ export class Editor extends EventEmitter<TLEventMap> {
// Make sure that each partial will become the child of either the
// page or another shape that exists (or that will exist) in this page.
const { currentPageShapesSorted: sortedShapesOnCurrentPage } = this
const { currentPageShapesSorted } = this
partials = partials.map((partial) => {
// If the partial does not provide the parentId OR if the provided
// parentId is NOT in the store AND NOT among the other shapes being
@ -6460,7 +6481,7 @@ export class Editor extends EventEmitter<TLEventMap> {
partial = { ...partial }
const parentId =
sortedShapesOnCurrentPage.findLast(
currentPageShapesSorted.findLast(
(parent) =>
// parent.type === 'frame'
this.getShapeUtil(parent).canReceiveNewChildrenOfType(parent, partial.type) &&

View file

@ -334,7 +334,7 @@ export class Vec2d {
}
static Len(A: VecLike): number {
return Math.sqrt(Vec2d.Len2(A))
return Math.hypot(A.x, A.y)
}
static Pry(A: VecLike, B: VecLike): number {

View file

@ -45,6 +45,7 @@ export class Circle2d extends Geometry2d {
nearestPoint(point: Vec2d): Vec2d {
const { _center, radius } = this
if (_center.equals(point)) return Vec2d.AddXY(_center, radius, 0)
return _center.clone().add(point.clone().sub(_center).uni().mul(radius))
}

View file

@ -8,7 +8,7 @@ export class Edge2d extends Geometry2d {
end: Vec2d
d: Vec2d
u: Vec2d
length: number
ul: number
constructor(config: { start: Vec2d; end: Vec2d; isSnappable?: boolean }) {
super({ ...config, isClosed: false, isFilled: false })
@ -17,9 +17,18 @@ export class Edge2d extends Geometry2d {
this.start = start
this.end = end
this.d = start.clone().sub(end)
this.u = this.d.clone().uni()
this.length = this.d.len()
this.d = start.clone().sub(end) // the delta from start to end
this.u = this.d.clone().uni() // the unit vector of the edge
this.ul = this.u.len() // the length of the unit vector
}
_length?: number
get length() {
if (!this._length) {
return this.d.len()
}
return this._length
}
midPoint(): Vec2d {
@ -31,14 +40,16 @@ export class Edge2d extends Geometry2d {
}
override nearestPoint(point: Vec2d): Vec2d {
const { start, end, u } = this
if (start.equals(end)) return start.clone()
const C = start.clone().add(u.clone().mul(point.clone().sub(start).pry(u)))
if (C.x < Math.min(start.x, end.x)) return start.x < end.x ? start : end
if (C.x > Math.max(start.x, end.x)) return start.x > end.x ? start : end
if (C.y < Math.min(start.y, end.y)) return start.y < end.y ? start : end
if (C.y > Math.max(start.y, end.y)) return start.y > end.y ? start : end
return C
const { start, end, u, ul: l } = this
if (l === 0) return start // no length in the unit vector
const k = Vec2d.Sub(point, start).dpr(u) / l
const cx = start.x + u.x * k
if (cx < Math.min(start.x, end.x)) return start.x < end.x ? start : end
if (cx > Math.max(start.x, end.x)) return start.x > end.x ? start : end
const cy = start.y + u.y * k
if (cy < Math.min(start.y, end.y)) return start.y < end.y ? start : end
if (cy > Math.max(start.y, end.y)) return start.y > end.y ? start : end
return new Vec2d(cx, cy)
}
override hitTestLineSegment(A: Vec2d, B: Vec2d, _zoom: number): boolean {

View file

@ -28,12 +28,13 @@ export abstract class Geometry2d {
abstract nearestPoint(point: Vec2d): Vec2d
hitTestPoint(point: Vec2d, margin = 0, hitInside = false) {
if (!this.bounds.containsPoint(point, margin)) return false
// We've removed the broad phase here; that should be done outside of the call
return this.distanceToPoint(point, hitInside) <= margin
}
distanceToPoint(point: Vec2d, hitInside = false) {
const dist = point.dist(this.nearestPoint(point))
if (this.isClosed && (this.isFilled || hitInside) && pointInPolygon(point, this.vertices)) {
return -dist
}

View file

@ -46,23 +46,21 @@ export class Polyline2d extends Geometry2d {
}
nearestPoint(A: Vec2d): Vec2d {
let nearest: Vec2d | undefined
const { segments } = this
let nearest = this.points[0]
let dist = Infinity
if (this.points.length === 1) {
return this.points[0]
}
for (const edge of this.segments) {
const p = edge.nearestPoint(A)
const d = p.dist(A)
let p: Vec2d // current point on segment
let d: number // distance from A to p
for (let i = 0; i < segments.length; i++) {
p = segments[i].nearestPoint(A)
d = p.dist(A)
if (d < dist) {
nearest = p
dist = d
}
}
if (!nearest) throw Error('nearest point not found')
return nearest
}

View file

@ -16,6 +16,7 @@ export class Pointing extends StateNode {
},
margin: 0,
hitInside: true,
renderingOnly: true,
})
if (!target) {

View file

@ -330,6 +330,7 @@ export class Idle extends StateNode {
hitInside: false,
hitLabels: true,
hitFrameInside: false,
renderingOnly: true,
})
if (hitShape) {

View file

@ -38,6 +38,7 @@ export class PointingSelection extends StateNode {
: this.editor.getShapeAtPoint(this.editor.inputs.currentPagePoint, {
hitInside: true,
margin: 0,
renderingOnly: true,
})
if (hitShape) {

View file

@ -71,6 +71,7 @@ export class PointingShape extends StateNode {
this.editor.getShapeAtPoint(currentPagePoint, {
margin: HIT_TEST_MARGIN / zoomLevel,
hitInside: true,
renderingOnly: true,
}) ?? this.hitShape
const selectingShape = hitShape
@ -141,7 +142,10 @@ export class PointingShape extends StateNode {
currentPagePoint
)
if (labelGeometry.hitTestPoint(pointInShapeSpace)) {
if (
labelGeometry.bounds.containsPoint(pointInShapeSpace, 0) &&
labelGeometry.hitTestPoint(pointInShapeSpace)
) {
this.editor.batch(() => {
this.editor.mark('editing on pointer up')
this.editor.select(selectingShape.id)

View file

@ -125,20 +125,23 @@ export class ScribbleBrushing extends StateNode {
shape = shapes[i]
geometry = this.editor.getShapeGeometry(shape)
// If the shape is a group or is already selected or locked, don't select it
if (
this.editor.isShapeOfType<TLGroupShape>(shape, 'group') ||
newlySelectedShapeIds.has(shape.id) ||
(this.editor.isShapeOfType<TLFrameShape>(shape, 'frame') &&
geometry.hitTestPoint(
this.editor.getPointInShapeSpace(shape, originPagePoint),
0,
false
)) ||
this.editor.isShapeOrAncestorLocked(shape)
) {
continue
}
// If the scribble started inside of the frame, don't select it
if (this.editor.isShapeOfType<TLFrameShape>(shape, 'frame')) {
const point = this.editor.getPointInShapeSpace(shape, originPagePoint)
if (geometry.bounds.containsPoint(point)) {
continue
}
}
A = this.editor.getPointInShapeSpace(shape, previousPagePoint)
B = this.editor.getPointInShapeSpace(shape, currentPagePoint)
if (geometry.hitTestLineSegment(A, B, HIT_TEST_MARGIN / zoomLevel)) {

View file

@ -12,6 +12,7 @@ export function getHitShapeOnCanvasPointerDown(editor: Editor): TLShape | undefi
hitInside: false,
hitLabels: false,
margin: HIT_TEST_MARGIN / zoomLevel,
renderingOnly: true,
}) ??
// selected shape at point
editor.getSelectedShapeAtPoint(currentPagePoint)

View file

@ -8,6 +8,7 @@ export function selectOnCanvasPointerUp(editor: Editor) {
hitInside: false,
margin: HIT_TEST_MARGIN / editor.zoomLevel,
hitLabels: true,
renderingOnly: true,
})
// Note at the start: if we select a shape that is inside of a group,

View file

@ -6,6 +6,7 @@ export function updateHoveredId(editor: Editor) {
hitInside: false,
hitLabels: false,
margin: HIT_TEST_MARGIN / editor.zoomLevel,
renderingOnly: true,
})
if (!hitShape) return editor.setHoveredShape(null)

View file

@ -413,7 +413,7 @@ describe('When in readonly mode', () => {
x: 100,
y: 100,
opacity: 1,
props: { w: 100, h: 100, url: '' },
props: { w: 100, h: 100, url: 'https://tldraw.com' },
},
])
editor.updateInstanceState({ isReadonly: true })

View file

@ -17,6 +17,7 @@ const ids = {
beforeEach(() => {
editor = new TestEditor()
editor.setScreenBounds({ w: 3000, h: 3000, x: 0, y: 0 })
})
it('lists a sorted shapes array correctly', () => {