steveruizok commented 2 months ago

This PR implements a camera options API.

[x] Initial PR
[x] Updated unit tests
[x] Feedback / review
[x] New unit tests
[x] Update use-case examples
[x] Ship?

Public API

A user can provide camera options to the Tldraw component via the cameraOptions prop. The prop is also available on the TldrawEditor component and the constructor parameters of the Editor class.

export default function CameraOptionsExample() {
    return (
        <div className="tldraw__editor">
            <Tldraw cameraOptions={CAMERA_OPTIONS} />
        </div>
    )
}

At runtime, a user can:

get the current camera options with Editor.getCameraOptions
update the camera options with Editor.setCameraOptions

Setting the camera options automatically applies them to the current camera.

editor.setCameraOptions({...editor.getCameraOptions(), isLocked: true })

A user can get the "camera fit zoom" via editor.getCameraFitZoom().

Interface

The camera options themselves can look a few different ways depending on the type provided.

export type TLCameraOptions = {
    /** Whether the camera is locked. */
    isLocked: boolean
    /** The speed of a scroll wheel / trackpad pan. Default is 1. */
    panSpeed: number
    /** The speed of a scroll wheel / trackpad zoom. Default is 1. */
    zoomSpeed: number
    /** The steps that a user can zoom between with zoom in / zoom out. The first and last value will determine the min and max zoom. */
    zoomSteps: number[]
    /** Controls whether the wheel pans or zooms.
     *
     * - `zoom`: The wheel will zoom in and out.
     * - `pan`: The wheel will pan the camera.
     * - `none`: The wheel will do nothing.
     */
    wheelBehavior: 'zoom' | 'pan' | 'none'
    /** The camera constraints. */
    constraints?: {
        /** The bounds (in page space) of the constrained space */
        bounds: BoxModel
        /** The padding inside of the viewport (in screen space) */
        padding: VecLike
        /** The origin for placement. Used to position the bounds within the viewport when an axis is fixed or contained and zoom is below the axis fit. */
        origin: VecLike
        /** The camera's initial zoom, used also when the camera is reset.
         *
         * - `default`: Sets the initial zoom to 100%.
         * - `fit-x`: The x axis will completely fill the viewport bounds.
         * - `fit-y`: The y axis will completely fill the viewport bounds.
         * - `fit-min`: The smaller axis will completely fill the viewport bounds.
         * - `fit-max`: The larger axis will completely fill the viewport bounds.
         * - `fit-x-100`: The x axis will completely fill the viewport bounds, or 100% zoom, whichever is smaller.
         * - `fit-y-100`: The y axis will completely fill the viewport bounds, or 100% zoom, whichever is smaller.
         * - `fit-min-100`: The smaller axis will completely fill the viewport bounds, or 100% zoom, whichever is smaller.
         * - `fit-max-100`: The larger axis will completely fill the viewport bounds, or 100% zoom, whichever is smaller.
         */
        initialZoom:
            | 'fit-min'
            | 'fit-max'
            | 'fit-x'
            | 'fit-y'
            | 'fit-min-100'
            | 'fit-max-100'
            | 'fit-x-100'
            | 'fit-y-100'
            | 'default'
        /** The camera's base for its zoom steps.
         *
         * - `default`: Sets the initial zoom to 100%.
         * - `fit-x`: The x axis will completely fill the viewport bounds.
         * - `fit-y`: The y axis will completely fill the viewport bounds.
         * - `fit-min`: The smaller axis will completely fill the viewport bounds.
         * - `fit-max`: The larger axis will completely fill the viewport bounds.
         * - `fit-x-100`: The x axis will completely fill the viewport bounds, or 100% zoom, whichever is smaller.
         * - `fit-y-100`: The y axis will completely fill the viewport bounds, or 100% zoom, whichever is smaller.
         * - `fit-min-100`: The smaller axis will completely fill the viewport bounds, or 100% zoom, whichever is smaller.
         * - `fit-max-100`: The larger axis will completely fill the viewport bounds, or 100% zoom, whichever is smaller.
         */
        baseZoom:
            | 'fit-min'
            | 'fit-max'
            | 'fit-x'
            | 'fit-y'
            | 'fit-min-100'
            | 'fit-max-100'
            | 'fit-x-100'
            | 'fit-y-100'
            | 'default'
        /** The behavior for the constraints for both axes or each axis individually.
         *
         * - `free`: The bounds are ignored when moving the camera.
         * - 'fixed': The bounds will be positioned within the viewport based on the origin
         * - `contain`: The 'fixed' behavior will be used when the zoom is below the zoom level at which the bounds would fill the viewport; and when above this zoom, the bounds will use the 'inside' behavior.
         * - `inside`: The bounds will stay completely within the viewport.
         * - `outside`: The bounds will stay touching the viewport.
         */
        behavior:
            | 'free'
            | 'fixed'
            | 'inside'
            | 'outside'
            | 'contain'
            | {
                    x: 'free' | 'fixed' | 'inside' | 'outside' | 'contain'
                    y: 'free' | 'fixed' | 'inside' | 'outside' | 'contain'
              }
    }
}

Change Type

[x] sdk — Changes the tldraw SDK
[x] feature — New feature

Test Plan

These features combine in different ways, so we'll want to write some more tests to find surprises.

Add a step-by-step description of how to test your PR here.

[ ] Unit Tests

Release Notes

SDK: Adds camera options.

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
examples	✅ Ready (Inspect)	Visit Preview	May 4, 2024 5:37pm
tldraw-docs	✅ Ready (Inspect)	Visit Preview	May 4, 2024 5:37pm

OrionReed commented 2 months ago

This is great, it's what I've been wanting since starting to mess with tldraw and opens up a whole world of new UX/features/good stuff. So I'm certainly invested in this!

Initial thoughts

I like the shape of the API, it fits with everything else and is easy to work with
Is this assuming infrequent changes to camera constraints? If I were to be updating constraints 120 times a second, will that have any notable impact on performance?
I still don't quite understand the "natural" part of "natural zoom", I'd love to hear more on why that name was picked.

I will play around with this over the weekend, I have some use-cases from months ago which I abandoned because this API did not exist, so I'm looking forward to going back to those.

steveruizok commented 2 months ago

Thanks for the feedback @OrionReed .

I still don't quite understand the "natural" part of "natural zoom", I'd love to hear more on why that name was picked.

I'd like to improve that name! I couldn't come up with anything here. I'll try to describe it more.

A long name would be the zoom level at which the dimension (height or width) of the camera bounds would fill all available space in viewport. That is, viewportScreenBounds - paddings - (cameraBounds.width / camera.z). Unless the camera bounds are exactly proportional to the viewport bounds, this number will be different for the height or width.

More context

The "camera bounds" is in page space. The "viewport screen bounds" is in screen space. The "camera paddings" are in screen space.

As a quick refresher on the coordinate systems, the relationship between page space and screen space is determined by the camera's zoom. At 100% zoom, the two are equal. At 50% zoom, each screen unit is equal to two page units. At 200% zoom, each screen unit is equal to .5 page units.

Let's say the camera bounds and the viewport bounds were the same size. The zoom at which the camera bounds would fit perfectly in the viewport would be 100%. Easy!

If the camera bounds were exactly half the size of the viewport bounds, then the zoom at which the camera bounds would fit perfectly in the viewport would be 200%. Likewise, if the camera bounds were exactly twice the size of the viewport bounds, then the zoom at which the camera bounds would fit perfectly in the viewport would be 50%.

We use this zoom value to set the "zero" or "default" for the camera. Zooms are calculated as multiples / fractions of this number, and pressing "reset zoom" goes back to this number. When zoomed out below this number, the camera should be centered.

Different aspect ratios

In reality, its very unlikely that the aspect ratio of the bounds is going to be equal to the aspect ratio of the viewport.

When the aspect ratios differ, then there are going to be two different numbers for the height or width.

The question is, which value do we want to use for the "zero" / "default" / "natural" / "resting" zoom? Do we want to fit the smaller of the two? Or the larger of the two? Or ignore these are use 100% as the number to use? You can configure that via the type value (another provisional name).

steveruizok commented 2 months ago

Is this assuming infrequent changes to camera constraints? If I were to be updating constraints 120 times a second, will that have any notable impact on performance?

It should be fine. You calling setCameraOptions sets the camera to its current state and applies the constraints.

OrionReed commented 2 months ago

Some names, not because I think they're good but maybe they'll prompt a thought, maybe Proportional Zoom or Viewport-Fill Zoom? or something to that effect?

Some more questions:

what if I wanted to constrain the camera to a non-xy axis like 30degrees?
How is vertical movement/scrolling being handled in the demo? Is that just using the browser scrolling (feels like it). Would love to hear more about this because part of my hope for this API is being able to create canvases that are indistinguishable from a regular webpage.

A thought on the type which does feel a bit odd, other than the fact it specifies a schema on the object similar to shape types.

Given the default/infinite setup:

const CAMERA_OPTIONS: TLCameraOptions = {
    type: 'infinite',
    zoomMax: 8,
    zoomMin: 0.1,
    zoomSteps: [0.1, 0.25, 0.5, 1, 2, 4, 8],
    zoomSpeed: 1,
    panSpeed: 1,
    isLocked: false,
}

Might it make sense to have a shape more like this?

const CAMERA_OPTIONS: TLCameraOptions = {
    zoomMax: 8,
    zoomMin: 0.1,
    zoomSteps: [0.1, 0.25, 0.5, 1, 2, 4, 8],
    zoomSpeed: 1,
    panSpeed: 1,
    isLocked: false,
    constraints: {
        type: 'contain',
            bounds: { x: 0, y: 0, w: 1200, h: 800 },
            padding: [0, 0],
            origin: [0.5, 0.5],
        }
}

where constraints: {} would be the same as type: 'infinite'. This way if you have more constraint types in the future their schematised type is localised to its own little part of the options.

Not sure I entirely like this yet, but it does feel like the camera options 'type' is a bit unnatural and that a options + constraints is a nice combination.

SomeHats commented 2 months ago

So the design here is pretty different from the one I came up with. Haven't done a proper code review on this, just responding to your write-up so far.

Natural zoom

The concept of a natural zoom that affects the zoom stops is one that I thought about for a while, but specifically wanted to avoid after I thought about it a bit more.

The main reason I wanted to avoid it is that I think that for cases where you're operating with camera constraints, you're much more likely to want a consistent concept of zoom levels across devices. Print & document editing is the obvious one here. There, I probably want to be able to have a fairly consistent concept of 100%, 200%, etc.

The concept of a natural zoom is also pretty variable across use-cases. For say an image editor, it's probably either 100%, or your natural zoom concept if the content doesn't fit on the screen at 100%. For a document annotator, it's probably 100% or the scale that fits the entire width of the document on-screen - because the bounds are likely to be very tall & thin.

The version I was experimenting with allowed using CSS style keywords or mayyybe functions in the place of certain zoom stops etc. It looked something like this:

const opts = {
    // keyword style: minimum zoom level is 1, or the right level to fit the width into the content
    zoomMin: [1, 'fit-width'],
    // keywords can be used within zoom stops - they'll get turned to numbers and sorted into place
    zoomStops: [0.1, 0.25, 1, 'fit-bounds', 2, 4, 8],
    // we could add something like your 'multiple reset zoom stops' like this:
    resetZoom: [1, 'fit-width'],
}

Not convinced this 👆 is the way to go either, but I definitely feel like the concept of natural zoom in this diff is too prescriptive. This is probably the most tricky and nuanced area of this problem though - maybe needs a bit more thought in general.

Origin

I thought about this and decided not to include it in my version as I couldn't think of a use-case for anything other than centered. Do you have one? Could we leave this off for now and see if there's need for it?

Padding

I called this margin. Same thing! In your version, it's only on the infinite type, but I think we should have it (optionally) for both types - we have things like "zoom to fit" in infinite where right now we have a hard-coded padding at the moment.

I had the <Tldraw /> component (IE the thing that brings our UI) set a default for this so if you were using our UI, it would automatically give you padding that avoided our UI and responded to our break-points. Seems slightly annoying to get right on your own, so seems worth-while to me.

Type, cover, & contain

This is where I started with my version of this too, but eventually pared it down to something a fair bit simpler.

The first realisation is that I couldn't come up with a use-case for cover (although mine was a bit different from yours as it didn't have a natural zoom concept). Curious what the use-case you found for this was!

That had me down to just infinite & cover types, but without origin & with padding on both the infinite and constrained version, the only difference between those things was the bounds property, so I ditched type and made bounds an optional property instead. I think we should do that here if we can - it makes for a simpler API and much more readable reference docs. If we still want to keep origin, we could move it under a property like:

interface CameraOptions {
    // ...
    constrain?: {
        bounds: BoxModel,
        origin?: VecLike,
    },
}

Dynamic

I know you mentioned this on my rough notes so it's probably just missing from here, but it's pretty important that a lot of these options can take functions that are provided with an editor instance and that they'll behave reactively when those functions return different values. You want to be able to use the on-canvas geometry to determine your bounds, and having to do an onMount={() => setEditor} dance and push those updates through react is pretty annoying.

Coordinate spaces & naming

When I was working on this with some customers on Discord, I found that the different co-ordinate spaces (screen vs viewport vs page) were one of the biggest sources of confusion. Some of this is probably just about docs (we don't have an overview anywhere) but even with that, what do you think of having this API reflect the coordinate spaces in its naming? Mostly I'm thinking padding->viewportPadding and bounds->pageBounds.

OrionReed commented 2 months ago

what do you think of having this API reflect the coordinate spaces in its naming?

Oh this seems like a fantastic idea and would stop people having to hold this info in their heads.

OrionReed commented 1 month ago

I'm wondering about non axis-aligned constraints, would that be something that this API should support?

This would enable both angled movement and also facilitate constraints along paths, the creation of (in user-land) guide systems which constrain your view along a set of vertices, for example.

If this is something that could be made to work with this API as-is I'd love to know how, I am interested to see how this would affect the momentum of scrolling/movement.

OrionReed commented 1 month ago

These last few commits have calmed all my discomforts with the API design, thanks @steveruizok!

"Fit" and the other name changes are so much better, the options are much closer to self-explanatory now.

I need to try and implement an off-axis or follow-path constraint with this system to see if that can be done, but I'm really happy with how this API is turning out.

steveruizok commented 1 month ago

Thanks Orion!

I need to try and implement an off-axis or follow-path constraint with this system to see if that can be done, but I'm really happy with how this API is turning out.

You could do the "path following" by locking the camera and using the Editor.centerOnPoint command in a loop. Though actually no—locking the camera prevents all movement, not just "user-initiated" movement.

Maybe that's something we need to distinguish between?

OrionReed commented 1 month ago

Hmmm yeah, if I keep the camera unlocked and call editor.centerOnPoint in a loop maybe that will work? I suppose it will depend on the event ordering and whether that call will avoid jitter from other motion induced by scrolling, etc.

Failing that, do you have ideas around how user-initiated and non-user movement would be distinguished?

OrionReed commented 1 month ago

On the point about namespacing that @mimecuvalo raised, I don't have a definite opinion about the best direction but I do strongly agree that something should be done. The API surface area is really quite large and I regularly discover things by accident that I was previously looking for (and gave up on).

Going to the API reference doesn't solve this except for the cases where I get lucky, I'll often resort to a CMD+F and check every occurrence of a keyword (like "camera") to try and find what I need.

I'm not sure about the dual flat + namespaced solution, but it would certainly help with API discoverability. I would just suggest that if this route is taken, that all namespaced properties/functions are included in the flat map so it doesn't introduce the ambiguity of "is this a namespaced thing? or a flat thing?".

steveruizok commented 1 month ago

I'd gone back and forth on namespacing during the early phases of this project and eventually decided that a flat API + certain (private) managers is a better fit for tldraw.

Identifying and moving to name spacing would be a bigger project, I'd rather have that discussion be more thorough (i.e. to also include evaluating whether certain methods should be moved out of the editor, potentially up to the "ui / actions" API layer) and do it as part of a major breaking release.

Going to the API reference doesn't solve this except for the cases where I get lucky, I'll often resort to a CMD+F and check every occurrence of a keyword (like "camera") to try and find what I need.

This is exactly why I prefer the big flat surface area rather than name-spacing! I'm still burned by map-box and game engines that had me digging around looking for methods.

OrionReed commented 1 month ago

Agreed that API structure changes should be discussed thoroughly and outside of this specific PR. I also share your game engine trauma...

One note on API discoverability: If I hadn't been following this API closely, I might have missed the resetZoom function, if I had tried to find this later without knowing what to look for and did a CMD+F search for "camera" I would not have found this because "camera" is not mentioned anywhere in the reference for this function.

Perhaps a procedural/human check when committing JSDoc text that it "names the system(s) it concerns or affects" would be enough. Maybe a little PR bot that asks you to check this, or maybe even adding custom tags to JSDoc so you can pick these up in searches. The one perk of custom JSDoc tags would be that searching tldraw.dev for "camera" would be able to bring up editor.resetZoom() which it currently won't.

OrionReed commented 1 month ago

One small request: if you include those images for "camera fit zoom" diagrams in the docs, explicitly label what the dotted and solid boxes are for clarity.

steveruizok commented 1 month ago

Dynamic I know you mentioned this on my rough notes so it's probably just missing from here, but it's pretty important that a lot of these options can take functions that are provided with an editor instance and that they'll behave reactively when those functions return different values. You want to be able to use the on-canvas geometry to determine your bounds, and having to do an onMount={() => setEditor} dance and push those updates through react is pretty annoying.

The more I looked at this, the more it seemed to be impossible to get right. If camera options are dynamic (for example, based on the location of shapes on the page) then we can get into weird states while dragging shapes around. If we can figure this out, then I suppose we can figure it out later and add it on; otherwise, I'd want to trigger it on some kind of "all interactions have ended, now what?" event.

Coordinate spaces & naming When I was working on this with some customers on Discord, I found that the different co-ordinate spaces (screen vs viewport vs page) were one of the biggest sources of confusion. Some of this is probably just about docs (we don't have an overview anywhere) but even with that, what do you think of having this API reflect the coordinate spaces in its naming? Mostly I'm thinking padding->viewportPadding and bounds->pageBounds.

Playing with the names now. pageBounds seems a bit confusing as it isn't really configuring the bounds of the page, but rather the constrained area in page space. viewportPadding is okay, I'll go with that.

steveruizok commented 1 month ago

The main reason I wanted to avoid it is that I think that for cases where you're operating with camera constraints, you're much more likely to want a consistent concept of zoom levels across devices. Print & document editing is the obvious one here. There, I probably want to be able to have a fairly consistent concept of 100%, 200%, etc.

Makes sense. I've separated out defaultZoom, which can be set to use one of the dimensions, and zoomBehavior, which decides whether the zoom steps are based on multiples of the defaultZoom or not.

hossain666 commented 4 weeks ago

https://github.com/apple/swift/blob/main/utils/pygments/swift.py

hossain666 commented 4 weeks ago

https://github.com/apple/swift/blob/main/utils/pygments/swift.py

tldraw / tldraw

Camera options #3282

Public API

Interface

Change Type

Test Plan

Release Notes

More context

Different aspect ratios

Natural zoom

Origin

Padding

Type, cover, & contain

Dynamic

Coordinate spaces & naming