image: follow-up fixes for LOD (#3934)

couple fixes and improvements for the LOD work.

- add `format=auto` for Cloudflare to send back more modern image
formats
- fix the broken asset logic that regressed (should not have looked at
`url`)
- fix stray parenthesis, omg
- rm the `useValueDebounced` function in lieu of just debouncing the
resolver. the problem was that the initial load in a multiplayer room
has a zoom of 1 but then the real zoom comes in (via the url) and so we
would double load all images 😬. this switches the debouncing to the
resolving stage, not making it tied to the zoom specifically.


### Change Type

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

- [x] `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
- [ ] `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
This commit is contained in:
Mime Čuvalo 2024-06-14 11:01:50 +01:00 committed by GitHub
parent fba82ed924
commit 73c2b1088a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 30 additions and 69 deletions

View file

@ -128,7 +128,7 @@ describe('resolveAsset', () => {
networkEffectiveType: null,
})
).toBe(
'https://localhost:8788/cdn-cgi/image/width=50,dpr=2,fit=scale-down,quality=92/http://example.com/image.jpg'
'https://localhost:8788/cdn-cgi/image/format=auto,width=50,dpr=2,fit=scale-down,quality=92/http://example.com/image.jpg'
)
})
@ -145,7 +145,7 @@ describe('resolveAsset', () => {
networkEffectiveType: '3g',
})
).toBe(
'https://localhost:8788/cdn-cgi/image/width=25,dpr=2,fit=scale-down,quality=92/http://example.com/image.jpg'
'https://localhost:8788/cdn-cgi/image/format=auto,width=25,dpr=2,fit=scale-down,quality=92/http://example.com/image.jpg'
)
})
@ -162,7 +162,7 @@ describe('resolveAsset', () => {
networkEffectiveType: '4g',
})
).toBe(
'https://localhost:8788/cdn-cgi/image/width=400,dpr=1,fit=scale-down,quality=92/https://example.com/image.jpg'
'https://localhost:8788/cdn-cgi/image/format=auto,width=400,dpr=1,fit=scale-down,quality=92/https://example.com/image.jpg'
)
})
@ -179,7 +179,7 @@ describe('resolveAsset', () => {
networkEffectiveType: '2g',
})
).toBe(
'https://localhost:8788/cdn-cgi/image/width=25,dpr=1,fit=scale-down,quality=92/https://example.com/image.jpg'
'https://localhost:8788/cdn-cgi/image/format=auto,width=25,dpr=1,fit=scale-down,quality=92/https://example.com/image.jpg'
)
})
@ -196,7 +196,7 @@ describe('resolveAsset', () => {
networkEffectiveType: '4g',
})
).toBe(
'https://localhost:8788/cdn-cgi/image/width=25,dpr=1,fit=scale-down,quality=92/https://example.com/image.jpg'
'https://localhost:8788/cdn-cgi/image/format=auto,width=25,dpr=1,fit=scale-down,quality=92/https://example.com/image.jpg'
)
})
})

View file

@ -36,5 +36,5 @@ export async function resolveAsset(asset: TLAsset | null | undefined, context: A
// On preview, builds the origin for the asset won't be the right one for the Cloudflare transform.
const src = asset.props.src.replace(ASSET_UPLOADER_URL, ASSET_BUCKET_ORIGIN)
return `${ASSET_BUCKET_ORIGIN}/cdn-cgi/image/width=${width},dpr=${context.dpr},fit=scale-down,quality=92/${src}`
return `${ASSET_BUCKET_ORIGIN}/cdn-cgi/image/format=auto,width=${width},dpr=${context.dpr},fit=scale-down,quality=92/${src}`
}

View file

@ -85,7 +85,6 @@ import { useComputed } from '@tldraw/state';
import { useQuickReactor } from '@tldraw/state';
import { useReactor } from '@tldraw/state';
import { useValue } from '@tldraw/state';
import { useValueDebounced } from '@tldraw/state';
import { VecModel } from '@tldraw/tlschema';
import { whyAmIRunning } from '@tldraw/state';
@ -3480,8 +3479,6 @@ export function useTransform(ref: React.RefObject<HTMLElement | SVGElement>, x?:
export { useValue }
export { useValueDebounced }
// @public (undocumented)
export class Vec {
constructor(x?: number, y?: number, z?: number);

View file

@ -28,7 +28,6 @@ export {
useQuickReactor,
useReactor,
useValue,
useValueDebounced,
whyAmIRunning,
type Atom,
type Signal,

View file

@ -207,9 +207,6 @@ export function useValue<Value>(value: Signal<Value>): Value;
// @public (undocumented)
export function useValue<Value>(name: string, fn: () => Value, deps: unknown[]): Value;
// @public
export function useValueDebounced<Value>(name: string, fn: () => Value, deps: unknown[], ms: number): Value;
// @public
export function whyAmIRunning(): void;

View file

@ -5,4 +5,3 @@ export { useQuickReactor } from './useQuickReactor'
export { useReactor } from './useReactor'
export { useStateTracking } from './useStateTracking'
export { useValue } from './useValue'
export { useValueDebounced } from './useValueDebounced'

View file

@ -1,33 +0,0 @@
/* eslint-disable prefer-rest-params */
import { useEffect, useState } from 'react'
import { useValue } from './useValue'
/**
* Extracts the value from a signal and subscribes to it, debouncing the value by the given number of milliseconds.
*
* @see [[useValue]] for more information.
*
* @public
*/
export function useValueDebounced<Value>(
name: string,
fn: () => Value,
deps: unknown[],
ms: number
): Value
/** @public */
export function useValueDebounced<Value>(): Value {
const args = [...arguments].slice(0, -1) as Parameters<typeof useValue>
const ms = arguments[arguments.length - 1] as number
const value = useValue(...args) as Value
const [debouncedValue, setDebouncedValue] = useState<Value>(value)
useEffect(() => {
const timer = setTimeout(() => {
setDebouncedValue(value)
}, ms)
return () => clearTimeout(timer)
}, [value, ms])
return debouncedValue
}

View file

@ -963,7 +963,7 @@ export class ImageShapeUtil extends BaseBoxShapeUtil<TLImageShape> {
// (undocumented)
canCrop: () => boolean;
// (undocumented)
component(shape: TLImageShape): JSX_2.Element;
component(shape: TLImageShape): JSX_2.Element | null;
// (undocumented)
getDefaultProps(): TLImageShape['props'];
// (undocumented)

View file

@ -73,7 +73,6 @@ export class ImageShapeUtil extends BaseBoxShapeUtil<TLImageShape> {
// cause visual flickering when the image is loaded.
if (url && !this.isAnimated(shape)) {
let cancelled = false
if (!url) return
const image = Image()
image.onload = () => {
@ -91,7 +90,6 @@ export class ImageShapeUtil extends BaseBoxShapeUtil<TLImageShape> {
useEffect(() => {
if (url && this.isAnimated(shape)) {
let cancelled = false
if (!url) return
const image = Image()
image.onload = () => {
@ -129,7 +127,8 @@ export class ImageShapeUtil extends BaseBoxShapeUtil<TLImageShape> {
const containerStyle = getCroppedContainerStyle(shape)
if (!url) {
// This is specifically `asset?.props.src` and not `url` because we're looking for broken assets.
if (!asset?.props.src) {
return (
<HTMLContainer
id={shape.id}
@ -145,7 +144,6 @@ export class ImageShapeUtil extends BaseBoxShapeUtil<TLImageShape> {
<div className="tl-image-container" style={containerStyle}>
{asset ? null : <BrokenAssetIcon />}
</div>
)
{'url' in shape.props && shape.props.url && (
<HyperlinkButton url={shape.props.url} zoomLevel={this.editor.getZoomLevel()} />
)}
@ -153,6 +151,8 @@ export class ImageShapeUtil extends BaseBoxShapeUtil<TLImageShape> {
)
}
if (!loadedSrc) return null
return (
<>
{showCropPreview && (
@ -189,7 +189,6 @@ export class ImageShapeUtil extends BaseBoxShapeUtil<TLImageShape> {
<div className="tl-image__tg">GIF</div>
)}
</div>
)
{shape.props.url && (
<HyperlinkButton url={shape.props.url} zoomLevel={this.editor.getZoomLevel()} />
)}

View file

@ -1,4 +1,4 @@
import { TLAssetId, useEditor, useValueDebounced } from '@tldraw/editor'
import { TLAssetId, useEditor, useValue } from '@tldraw/editor'
import { useEffect, useState } from 'react'
/** @internal */
@ -10,22 +10,25 @@ export function useAsset(assetId: TLAssetId | null, width: number) {
const shapeScale = asset && 'w' in asset.props ? width / asset.props.w : 1
// We debounce the zoom level to reduce the number of times we fetch a new image and,
// more importantly, to not cause zooming in and out to feel janky.
const debouncedScreenScale = useValueDebounced(
'zoom level',
() => editor.getZoomLevel() * shapeScale,
[editor, shapeScale],
500
)
const screenScale = useValue('zoom level', () => editor.getZoomLevel() * shapeScale, [
editor,
shapeScale,
])
useEffect(() => {
async function resolve() {
let isCancelled = false
const timer = editor.timers.setTimeout(async () => {
const resolvedUrl = await editor.resolveAssetUrl(assetId, {
screenScale: debouncedScreenScale,
screenScale,
})
setUrl(resolvedUrl)
if (!isCancelled) setUrl(resolvedUrl)
})
return () => {
clearTimeout(timer)
isCancelled = true
}
resolve()
}, [assetId, debouncedScreenScale, editor])
}, [assetId, screenScale, editor])
return { asset, url }
}

View file

@ -158,7 +158,9 @@ export class VideoShapeUtil extends BaseBoxShapeUtil<TLVideoShape> {
>
<div className="tl-counter-scaled">
<div className="tl-video-container">
{url ? (
{!asset?.props.src ? (
<BrokenAssetIcon />
) : url ? (
<video
ref={rVideo}
style={isEditing ? { pointerEvents: 'all' } : undefined}
@ -181,9 +183,7 @@ export class VideoShapeUtil extends BaseBoxShapeUtil<TLVideoShape> {
>
<source src={url} />
</video>
) : (
<BrokenAssetIcon />
)}
) : null}
</div>
</div>
</HTMLContainer>