[culling] Improve setting of display none. (#3376)

Small improvement for culling shapes. We now use reactor to do it. .

Before:

![image](https://github.com/tldraw/tldraw/assets/2523721/7f791cdd-c0e2-4b92-84d1-8b071540de10)

After:

![image](https://github.com/tldraw/tldraw/assets/2523721/ca2e2a9e-f9f6-48a8-936f-05a402c1e7a2)


### Change Type

<!--  Please select a 'Scope' label ️ -->

- [ ] `sdk` — Changes the tldraw SDK
- [ ] `dotcom` — Changes the tldraw.com web app
- [ ] `docs` — Changes to the documentation, examples, or templates.
- [ ] `vs code` — Changes to the vscode plugin
- [x] `internal` — Does not affect user-facing stuff

<!--  Please select a 'Type' label ️ -->

- [ ] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [x] `improvement` — Improving existing features
- [ ] `chore` — Updating dependencies, other boring stuff
- [ ] `galaxy brain` — Architectural changes
- [ ] `tests` — Changes to any test code
- [ ] `tools` — Changes to infrastructure, CI, internal scripts,
debugging tools, etc.
- [ ] `dunno` — I don't know
This commit is contained in:
Mitja Bezenšek 2024-04-08 13:36:12 +02:00 committed by GitHub
parent 86403c1b0d
commit 947f7b1d76
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 167 additions and 83 deletions

View file

@ -36,7 +36,8 @@ export function CustomRenderer() {
const theme = getDefaultColorTheme({ isDarkMode: editor.user.getIsDarkMode() })
const currentPageId = editor.getCurrentPageId()
for (const { shape, maskedPageBounds, opacity } of renderingShapes) {
for (const { shape, opacity } of renderingShapes) {
const maskedPageBounds = editor.getShapeMaskedPageBounds(shape)
if (!maskedPageBounds) continue
ctx.save()

View file

@ -22,9 +22,11 @@ export function useChangedShapesReactor(
if (!beforeInfo) {
continue
} else {
if (afterInfo.isCulled && !beforeInfo.isCulled) {
const isAfterCulled = editor.isShapeCulled(afterInfo.id)
const isBeforeCulled = editor.isShapeCulled(beforeInfo.id)
if (isAfterCulled && !isBeforeCulled) {
culled.push(afterInfo.shape)
} else if (!afterInfo.isCulled && beforeInfo.isCulled) {
} else if (!isAfterCulled && isBeforeCulled) {
restored.push(afterInfo.shape)
}
beforeToVisit.delete(beforeInfo)

View file

@ -720,8 +720,6 @@ export class Editor extends EventEmitter<TLEventMap> {
index: number;
backgroundIndex: number;
opacity: number;
isCulled: boolean;
maskedPageBounds: Box | undefined;
}[];
getSelectedShapeAtPoint(point: VecLike): TLShape | undefined;
getSelectedShapeIds(): TLShapeId[];
@ -782,8 +780,6 @@ export class Editor extends EventEmitter<TLEventMap> {
index: number;
backgroundIndex: number;
opacity: number;
isCulled: boolean;
maskedPageBounds: Box | undefined;
}[];
getViewportPageBounds(): Box;
getViewportPageCenter(): Vec;
@ -821,6 +817,7 @@ export class Editor extends EventEmitter<TLEventMap> {
margin?: number | undefined;
hitInside?: boolean | undefined;
}): boolean;
isShapeCulled(shape: TLShape | TLShapeId): boolean;
isShapeInPage(shape: TLShape | TLShapeId, pageId?: TLPageId): boolean;
isShapeOfType<T extends TLUnknownShape>(shape: TLUnknownShape, type: T['type']): shape is T;
// (undocumented)

View file

@ -11955,16 +11955,7 @@
},
{
"kind": "Content",
"text": ">;\n index: number;\n backgroundIndex: number;\n opacity: number;\n isCulled: boolean;\n maskedPageBounds: "
},
{
"kind": "Reference",
"text": "Box",
"canonicalReference": "@tldraw/editor!Box:class"
},
{
"kind": "Content",
"text": " | undefined;\n }[]"
"text": ">;\n index: number;\n backgroundIndex: number;\n opacity: number;\n }[]"
},
{
"kind": "Content",
@ -11974,7 +11965,7 @@
"isStatic": false,
"returnTypeTokenRange": {
"startIndex": 1,
"endIndex": 12
"endIndex": 10
},
"releaseTag": "Public",
"isProtected": false,
@ -14823,6 +14814,64 @@
"isAbstract": false,
"name": "isPointInShape"
},
{
"kind": "Method",
"canonicalReference": "@tldraw/editor!Editor#isShapeCulled:member(1)",
"docComment": "/**\n * Get whether the shape is culled or not.\n *\n * @param shape - The shape (or shape id) to get the culled info for.\n *\n * @example\n * ```ts\n * editor.isShapeCulled(myShape)\n * editor.isShapeCulled(myShapeId)\n * ```\n *\n * @public\n */\n",
"excerptTokens": [
{
"kind": "Content",
"text": "isShapeCulled(shape: "
},
{
"kind": "Reference",
"text": "TLShape",
"canonicalReference": "@tldraw/tlschema!TLShape:type"
},
{
"kind": "Content",
"text": " | "
},
{
"kind": "Reference",
"text": "TLShapeId",
"canonicalReference": "@tldraw/tlschema!TLShapeId:type"
},
{
"kind": "Content",
"text": "): "
},
{
"kind": "Content",
"text": "boolean"
},
{
"kind": "Content",
"text": ";"
}
],
"isStatic": false,
"returnTypeTokenRange": {
"startIndex": 5,
"endIndex": 6
},
"releaseTag": "Public",
"isProtected": false,
"overloadIndex": 1,
"parameters": [
{
"parameterName": "shape",
"parameterTypeTokenRange": {
"startIndex": 1,
"endIndex": 4
},
"isOptional": false
}
],
"isOptional": false,
"isAbstract": false,
"name": "isShapeCulled"
},
{
"kind": "Method",
"canonicalReference": "@tldraw/editor!Editor#isShapeInPage:member(1)",

View file

@ -112,8 +112,9 @@ export function CulledShapes() {
const shapeVertices = computed('shape vertices', function calculateCulledShapeVertices() {
const results: number[] = []
for (const { isCulled, maskedPageBounds } of editor.getRenderingShapes()) {
if (isCulled && maskedPageBounds) {
for (const { id } of editor.getUnorderedRenderingShapes(true)) {
const maskedPageBounds = editor.getShapeMaskedPageBounds(id)
if (editor.isShapeCulled(id) && maskedPageBounds) {
results.push(
// triangle 1
maskedPageBounds.minX,

View file

@ -1,6 +1,6 @@
import { useQuickReactor, useStateTracking } from '@tldraw/state'
import { TLShape, TLShapeId } from '@tldraw/tlschema'
import { memo, useCallback, useLayoutEffect, useRef } from 'react'
import { memo, useCallback, useRef } from 'react'
import { ShapeUtil } from '../editor/shapes/ShapeUtil'
import { useEditor } from '../hooks/useEditor'
import { useEditorComponents } from '../hooks/useEditorComponents'
@ -26,7 +26,6 @@ export const Shape = memo(function Shape({
index,
backgroundIndex,
opacity,
isCulled,
dprMultiple,
}: {
id: TLShapeId
@ -35,7 +34,6 @@ export const Shape = memo(function Shape({
index: number
backgroundIndex: number
opacity: number
isCulled: boolean
dprMultiple: number
}) {
const editor = useEditor()
@ -120,13 +118,18 @@ export const Shape = memo(function Shape({
[opacity, index, backgroundIndex]
)
useLayoutEffect(() => {
const container = containerRef.current
const bgContainer = bgContainerRef.current
setStyleProperty(container, 'display', isCulled ? 'none' : 'block')
setStyleProperty(bgContainer, 'display', isCulled ? 'none' : 'block')
}, [isCulled])
useQuickReactor(
'set display',
() => {
const shape = editor.getShape(id)
if (!shape) return // probably the shape was just deleted
const isCulled = editor.isShapeCulled(shape)
setStyleProperty(containerRef.current, 'display', isCulled ? 'none' : 'block')
setStyleProperty(bgContainerRef.current, 'display', isCulled ? 'none' : 'block')
},
[editor]
)
const annotateError = useCallback(
(error: any) => editor.annotateError(error, { origin: 'shape', willCrashApp: false }),
[editor]

View file

@ -3126,48 +3126,26 @@ export class Editor extends EventEmitter<TLEventMap> {
index: number
backgroundIndex: number
opacity: number
isCulled: boolean
maskedPageBounds: Box | undefined
}[] = []
let nextIndex = MAX_SHAPES_PER_PAGE * 2
let nextBackgroundIndex = MAX_SHAPES_PER_PAGE
// We only really need these if we're using editor state, but that's ok
const editingShapeId = this.getEditingShapeId()
const selectedShapeIds = this.getSelectedShapeIds()
const erasingShapeIds = this.getErasingShapeIds()
const renderingBoundsExpanded = this.getRenderingBoundsExpanded()
// If renderingBoundsMargin is set to Infinity, then we won't cull offscreen shapes
const isCullingOffScreenShapes = Number.isFinite(this.renderingBoundsMargin)
const addShapeById = (id: TLShapeId, opacity: number, isAncestorErasing: boolean) => {
const shape = this.getShape(id)
if (!shape) return
opacity *= shape.opacity
let isCulled = false
let isShapeErasing = false
const util = this.getShapeUtil(shape)
const maskedPageBounds = this.getShapeMaskedPageBounds(id)
if (useEditorState) {
isShapeErasing = !isAncestorErasing && erasingShapeIds.includes(id)
if (isShapeErasing) {
opacity *= 0.32
}
isCulled =
isCullingOffScreenShapes &&
// never cull editingg shapes
editingShapeId !== id &&
// if the shape is fully outside of its parent's clipping bounds...
(maskedPageBounds === undefined ||
// ...or if the shape is outside of the expanded viewport bounds...
(!renderingBoundsExpanded.includes(maskedPageBounds) &&
// ...and if it's not selected... then cull it
!selectedShapeIds.includes(id)))
}
renderingShapes.push({
@ -3177,8 +3155,6 @@ export class Editor extends EventEmitter<TLEventMap> {
index: nextIndex,
backgroundIndex: nextBackgroundIndex,
opacity,
isCulled,
maskedPageBounds,
})
nextIndex += 1
@ -4266,6 +4242,51 @@ export class Editor extends EventEmitter<TLEventMap> {
return this.isShapeOrAncestorLocked(this.getShapeParent(shape))
}
@computed
private _getShapeCullingInfoCache(): ComputedCache<boolean, TLShape> {
return this.store.createComputedCache(
'shapeCullingInfo',
({ id }) => {
// We don't cull shapes that are being edited
if (this.getEditingShapeId() === id) return false
const maskedPageBounds = this.getShapeMaskedPageBounds(id)
// if the shape is fully outside of its parent's clipping bounds...
if (maskedPageBounds === undefined) return true
// We don't cull selected shapes
if (this.getSelectedShapeIds().includes(id)) return false
const renderingBoundsExpanded = this.getRenderingBoundsExpanded()
// the shape is outside of the expanded viewport bounds...
return !renderingBoundsExpanded.includes(maskedPageBounds)
},
(a, b) => this.getShapeMaskedPageBounds(a) === this.getShapeMaskedPageBounds(b)
)
}
/**
* Get whether the shape is culled or not.
*
* @example
* ```ts
* editor.isShapeCulled(myShape)
* editor.isShapeCulled(myShapeId)
* ```
*
* @param shape - The shape (or shape id) to get the culled info for.
*
* @public
*/
isShapeCulled(shape: TLShape | TLShapeId): boolean {
// If renderingBoundsMargin is set to Infinity, then we won't cull offscreen shapes
const isCullingOffScreenShapes = Number.isFinite(this.renderingBoundsMargin)
if (!isCullingOffScreenShapes) return false
const id = typeof shape === 'string' ? shape : shape.id
return this._getShapeCullingInfoCache().get(id)! as boolean
}
/**
* The bounds of the current page (the common bounds of all of the shapes on the page).
*
@ -4637,8 +4658,8 @@ export class Editor extends EventEmitter<TLEventMap> {
* @public
*/
@computed getCurrentPageRenderingShapesSorted(): TLShape[] {
return this.getRenderingShapes()
.filter(({ isCulled }) => !isCulled)
return this.getUnorderedRenderingShapes(true)
.filter(({ id }) => !this.isShapeCulled(id))
.sort((a, b) => a.index - b.index)
.map(({ shape }) => shape)
}

View file

@ -38,7 +38,8 @@ export async function getSvgJsx(
if (opts.bounds) {
bbox = opts.bounds
} else {
for (const { maskedPageBounds } of renderingShapes) {
for (const { id } of renderingShapes) {
const maskedPageBounds = editor.getShapeMaskedPageBounds(id)
if (!maskedPageBounds) continue
if (bbox) {
bbox.union(maskedPageBounds)

View file

@ -19,9 +19,10 @@ export function BackToContent() {
// Rendering shapes includes all the shapes in the current page.
// We have to filter them down to just the shapes that are inside the renderingBounds.
const visibleShapes = renderingShapes.filter(
(s) => s.maskedPageBounds && renderingBounds.includes(s.maskedPageBounds)
)
const visibleShapes = renderingShapes.filter((s) => {
const maskedPageBounds = editor.getShapeMaskedPageBounds(s.id)
return maskedPageBounds && renderingBounds.includes(maskedPageBounds)
})
const showBackToContentNow =
visibleShapes.length === 0 && editor.getCurrentPageShapes().length > 0

View file

@ -63,42 +63,50 @@ it('updates the rendering viewport when the camera stops moving', () => {
it('lists shapes in viewport', () => {
const ids = createShapes()
editor.selectNone()
expect(editor.getRenderingShapes().map(({ id, isCulled }) => [id, isCulled])).toStrictEqual([
[ids.A, false], // A is within the expanded rendering bounds, so should not be culled; and it's in the regular viewport too, so it's on screen.
[ids.B, false],
[ids.C, false],
[ids.D, true], // D is clipped and so should always be culled / outside of viewport
])
expect(editor.getRenderingShapes().map(({ id }) => [id, editor.isShapeCulled(id)])).toStrictEqual(
[
[ids.A, false], // A is within the expanded rendering bounds, so should not be culled; and it's in the regular viewport too, so it's on screen.
[ids.B, false],
[ids.C, false],
[ids.D, true], // D is clipped and so should always be culled / outside of viewport
]
)
// Move the camera 201 pixels to the right and 201 pixels down
editor.pan({ x: -201, y: -201 })
jest.advanceTimersByTime(500)
expect(editor.getRenderingShapes().map(({ id, isCulled }) => [id, isCulled])).toStrictEqual([
[ids.A, false], // A should not be culled, even though it's no longer in the viewport (because it's still in the EXPANDED viewport)
[ids.B, false],
[ids.C, false],
[ids.D, true], // D is clipped and so should always be culled / outside of viewport
])
expect(editor.getRenderingShapes().map(({ id }) => [id, editor.isShapeCulled(id)])).toStrictEqual(
[
[ids.A, false], // A should not be culled, even though it's no longer in the viewport (because it's still in the EXPANDED viewport)
[ids.B, false],
[ids.C, false],
[ids.D, true], // D is clipped and so should always be culled / outside of viewport
]
)
editor.pan({ x: -100, y: -100 })
jest.advanceTimersByTime(500)
expect(editor.getRenderingShapes().map(({ id, isCulled }) => [id, isCulled])).toStrictEqual([
[ids.A, true], // A should be culled now that it's outside of the expanded viewport too
[ids.B, false],
[ids.C, false],
[ids.D, true], // D is clipped and so should always be culled, even if it's in the viewport
])
expect(editor.getRenderingShapes().map(({ id }) => [id, editor.isShapeCulled(id)])).toStrictEqual(
[
[ids.A, true], // A should be culled now that it's outside of the expanded viewport too
[ids.B, false],
[ids.C, false],
[ids.D, true], // D is clipped and so should always be culled, even if it's in the viewport
]
)
editor.pan({ x: -900, y: -900 })
jest.advanceTimersByTime(500)
expect(editor.getRenderingShapes().map(({ id, isCulled }) => [id, isCulled])).toStrictEqual([
[ids.A, true],
[ids.B, true],
[ids.C, true],
[ids.D, true],
])
expect(editor.getRenderingShapes().map(({ id }) => [id, editor.isShapeCulled(id)])).toStrictEqual(
[
[ids.A, true],
[ids.B, true],
[ids.C, true],
[ids.D, true],
]
)
})
it('lists shapes in viewport sorted by id with correct indexes & background indexes', () => {