Fix culling. (#3504)

Fixes culling for cases when another user would drag shapes inside your
viewport. We weren't correctly calculating the culling status for arrows
that might be bound to those shapes and also for shapes within dragged
in groups / frames.


### 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 ️ -->

- [x] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [ ] `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


### Test Plan

1. Open the same room in two browsers / tabs.
2. Have some shapes that are visible in one browser, but not the other.
3. Drag these shapes so that they are visible in the other browser as
well.
4. They should correctly get unculled.
5. Do this by dragging shapes that have arrows bound to them (arrows
should uncull), groups (shapes within them should uncull), frames.

- [x] Unit Tests
- [ ] End to end tests

### Release Notes

- Fix culling.
This commit is contained in:
Mitja Bezenšek 2024-04-17 13:39:09 +02:00 committed by GitHub
parent 34ad856873
commit 0b44a8b47a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 62 additions and 61 deletions

View file

@ -1,5 +1,5 @@
import { RESET_VALUE, computed, isUninitialized } from '@tldraw/state' import { computed, isUninitialized } from '@tldraw/state'
import { TLPageId, TLShapeId, isShape, isShapeId } from '@tldraw/tlschema' import { TLShapeId } from '@tldraw/tlschema'
import { Box } from '../../primitives/Box' import { Box } from '../../primitives/Box'
import { Editor } from '../Editor' import { Editor } from '../Editor'
@ -21,15 +21,10 @@ function isShapeNotVisible(editor: Editor, id: TLShapeId, viewportPageBounds: Bo
*/ */
export const notVisibleShapes = (editor: Editor) => { export const notVisibleShapes = (editor: Editor) => {
const isCullingOffScreenShapes = Number.isFinite(editor.renderingBoundsMargin) const isCullingOffScreenShapes = Number.isFinite(editor.renderingBoundsMargin)
const shapeHistory = editor.store.query.filterHistory('shape')
let lastPageId: TLPageId | null = null
let prevViewportPageBounds: Box
function fromScratch(editor: Editor): Set<TLShapeId> { function fromScratch(editor: Editor): Set<TLShapeId> {
const shapes = editor.getCurrentPageShapeIds() const shapes = editor.getCurrentPageShapeIds()
lastPageId = editor.getCurrentPageId()
const viewportPageBounds = editor.getViewportPageBounds() const viewportPageBounds = editor.getViewportPageBounds()
prevViewportPageBounds = viewportPageBounds.clone()
const notVisibleShapes = new Set<TLShapeId>() const notVisibleShapes = new Set<TLShapeId>()
shapes.forEach((id) => { shapes.forEach((id) => {
if (isShapeNotVisible(editor, id, viewportPageBounds)) { if (isShapeNotVisible(editor, id, viewportPageBounds)) {
@ -38,68 +33,21 @@ export const notVisibleShapes = (editor: Editor) => {
}) })
return notVisibleShapes return notVisibleShapes
} }
return computed<Set<TLShapeId>>('getCulledShapes', (prevValue, lastComputedEpoch) => { return computed<Set<TLShapeId>>('getCulledShapes', (prevValue) => {
if (!isCullingOffScreenShapes) return new Set<TLShapeId>() if (!isCullingOffScreenShapes) return new Set<TLShapeId>()
if (isUninitialized(prevValue)) { if (isUninitialized(prevValue)) {
return fromScratch(editor) return fromScratch(editor)
} }
const diff = shapeHistory.getDiffSince(lastComputedEpoch)
if (diff === RESET_VALUE) { const nextValue = fromScratch(editor)
return fromScratch(editor)
}
const currentPageId = editor.getCurrentPageId() if (prevValue.size !== nextValue.size) return nextValue
if (lastPageId !== currentPageId) { for (const prev of prevValue) {
return fromScratch(editor) if (!nextValue.has(prev)) {
} return nextValue
const viewportPageBounds = editor.getViewportPageBounds()
if (!prevViewportPageBounds || !viewportPageBounds.equals(prevViewportPageBounds)) {
return fromScratch(editor)
}
let nextValue = null as null | Set<TLShapeId>
const addId = (id: TLShapeId) => {
// Already added
if (prevValue.has(id)) return
if (!nextValue) nextValue = new Set(prevValue)
nextValue.add(id)
}
const deleteId = (id: TLShapeId) => {
// No need to delete since it's not there
if (!prevValue.has(id)) return
if (!nextValue) nextValue = new Set(prevValue)
nextValue.delete(id)
}
for (const changes of diff) {
for (const record of Object.values(changes.added)) {
if (isShape(record)) {
const isCulled = isShapeNotVisible(editor, record.id, viewportPageBounds)
if (isCulled) {
addId(record.id)
} }
} }
} return prevValue
for (const [_from, to] of Object.values(changes.updated)) {
if (isShape(to)) {
const isCulled = isShapeNotVisible(editor, to.id, viewportPageBounds)
if (isCulled) {
addId(to.id)
} else {
deleteId(to.id)
}
}
}
for (const id of Object.keys(changes.removed)) {
if (isShapeId(id)) {
deleteId(id)
}
}
}
return nextValue ?? prevValue
}) })
} }

View file

@ -136,3 +136,56 @@ it('correctly calculates the culled shapes when adding and deleting shapes', ()
const culledShapeFromScratch = editor.getCulledShapes() const culledShapeFromScratch = editor.getCulledShapes()
expect(culledShapesIncremental).toEqual(culledShapeFromScratch) expect(culledShapesIncremental).toEqual(culledShapeFromScratch)
}) })
it('works for shapes that are outside of the viewport, but are then moved inside it', () => {
const box1Id = createShapeId()
const box2Id = createShapeId()
const arrowId = createShapeId()
editor.createShapes([
{
id: box1Id,
props: { w: 100, h: 100, geo: 'rectangle' },
type: 'geo',
x: -500,
y: 0,
},
{
id: box2Id,
type: 'geo',
x: -1000,
y: 200,
props: { w: 100, h: 100, geo: 'rectangle' },
},
{
id: arrowId,
type: 'arrow',
props: {
start: {
type: 'binding',
isExact: true,
boundShapeId: box1Id,
normalizedAnchor: { x: 0.5, y: 0.5 },
isPrecise: false,
},
end: {
type: 'binding',
isExact: true,
boundShapeId: box2Id,
normalizedAnchor: { x: 0.5, y: 0.5 },
isPrecise: false,
},
},
},
])
expect(editor.getCulledShapes()).toEqual(new Set([box1Id, box2Id, arrowId]))
// Move box1 and box2 inside the viewport
editor.updateShapes([
{ id: box1Id, type: 'geo', x: 100 },
{ id: box2Id, type: 'geo', x: 200 },
])
// Arrow should also not be culled
expect(editor.getCulledShapes()).toEqual(new Set())
})