ADD: Automatically move the viewport when dragging an item close to t… #517

LotzF · 2025-02-10T20:02:05Z

Refers to issue #516

…he canvas borders

webfiltered

Hi there, thanks for the PR! The idea seems sound.

I think this feature should be opt-in for users, and if it's not enabled, everything should be bypassed (so no overhead).

If you're interested in making these changes yourself, please feel free!

webfiltered · 2025-02-10T20:16:45Z

src/DragAndScale.ts

+  /**
+   * Calculates the clamped vector between two points.
+   * @param pointA The first point
+   * @param pointB The second point
+   * @returns Vector2 {@link Point} of pointA to pointB, clamped to the range [-1, 1]
+   * @remarks Calculates a pseudo "normalized" Vector2 by clamping it in range. Faster than using SQRT for truly normalized vectors. DO NOT use as a true unit vector.
+   * */
+  getVector2Clamped(pointA: Point, pointB: Point): Point {
+    const tempVector: Point = [
+      (pointA[0] - pointB[0]) / pointB[0],
+      (pointA[1] - pointB[1]) / pointB[1]
+    ]
+    return [
+      Math.max(-1, Math.min(1, tempVector[0])),
+      Math.max(-1, Math.min(1, tempVector[1]))
+    ]
+  }


This looks like it should live in measure.ts / some other utility function module.

I'd also recommend just using two consts instead of making a Point for one calculation; it'd actually be more concise.

I've seen the measure.ts and had the function placed there to begin with but got a bit unsure. Now I know. I'll move it.

webfiltered · 2025-02-10T20:25:41Z

src/DragAndScale.ts

+  /**
+   * Automatically moves the canvas to follow the mouse.
+   * @param deltaT Time difference since last frame
+   * @param mouse Current mouse position
+   * @param canvasCenter Center of the canvas
+   * @param selectedItems Items that should be moved with the canvas when moving
+   * @remarks Does not require items to be selected to work properly. Could add ramp up/down for smoother movement transition.
+   * */
+  autoMoveCanvas(deltaT: number, mouse: Point, canvasCenter: Point, selectedItems: Set<Positionable> = null): void {
+    const moveDirection = this.getVector2Clamped(mouse, canvasCenter)
+    if (Math.abs(moveDirection[0]) > this.move_threshold || Math.abs(moveDirection[1]) > this.move_threshold) {
+      const newOffset = [
+        moveDirection[0] * deltaT * this.move_factor,
+        moveDirection[1] * deltaT * this.move_factor,
+      ]
+      this.offset[0] -= newOffset[0] / this.scale
+      this.offset[1] -= newOffset[1] / this.scale
+
+      if (!selectedItems) return
+      for (const item of selectedItems) {
+        item.move(newOffset[0] / this.scale, newOffset[1] / this.scale, false)
+      }
+    }
+  }


The DragAndScale object is a subordinate object of the canvas - it is more "do what you're told" than "make decisions". Movement of items is generally performed by the canvas or graph.

I could adapt the function and move it to the LGraphCanvas.ts .
@webfiltered or would you prefer it in any other location? LGraphCanvas kinda makes sense.

Yeah, I am loathe to recommend adding more to LGraphCanvas, but it is definitely the only logical choice right now. Thank you!

webfiltered · 2025-02-10T20:28:33Z

src/LGraphCanvas.ts

+      if (this.isDragging || this.connecting_links) {
+        const canvasCenter: Point = [
+          this.canvas.clientWidth / 2,
+          this.canvas.clientHeight / 2
+        ]
+        const selected = this.connecting_links ? null : this.selectedItems // Prevent selected items from being moved when dragging a connector
+        this.ds.autoMoveCanvas(this.render_time, this.mouse, canvasCenter, selected)
+        this.#dirty()
+      }


I think this belongs in a CanvasPointer callback, like other movement calculations. If there's a reason it's in the measure pass, please let us know.

If I'm not mistaken most CanvasPointer events only trigger when actually doing something with the pointer/mouse like down, up, drag, etc. This way the auto move would become a bit janky. It'll work but not as expected.
Let's say I'll use a move event for updating the viewport position. It'll work for sure but you gotta wiggle around the mouse to keep it working otherwise it'll just stop moving. By using a tick event like the draw loop we'll make sure the viewport keeps moving as long as the Positionable is close enough to the bounds even when dragging but not moving the mouse anymore.

You are not mistaken at all! I just need to expand a little:

Use CanvasPointer events to monitor the mouse movement (performant and keeps concerns separate)

Keep state on the CanvasPointer object, e.g. when the pointer gets to the edge, is dragging stuff, and should start panning

pointer.atEdgeOfScreenAndDraggingStuff = true (please don't use this name though haha)

I think a short interval may be better for movement at lower FPS, provided the thread isn't blocked elsewhere in the pipeline; changing the canvas offset between frames has almost no cost.

If point 3 doesn't work or seems like too much effort, you can insert a call to a function that does the above checks below the draw call here:

litegraph.js/src/LGraphCanvas.ts

Lines 1893 to 1896 in 4cee8ca

function renderFrame(this: LGraphCanvas) {

if (!this.pause_rendering) {

this.draw()

}

LotzF · 2025-02-10T20:46:03Z

Hey @webfiltered,

Thanks for the fast reply. I'll make the changes based on your feedback and get back to you.

… to const number

…terval, changed Point to const number for calculations, set up variables for UI (threshold, speed, enable)

…to_panning_event

LotzF · 2025-02-11T20:20:39Z

Hey @webfiltered,
Finally had some time to make the changes.

Here's a quick rundown:

Moved getVector2Clamped to measure.ts
getVector2Clamped now uses const number instead of a Point
Added auto_panning_enabled boolean to enable/disable the feature entirely (disabled by default)
Functionality uses an interval instead of the render loop for updates. (Interval should be at least 30ms/33fps to have a smooth movement)
Created a set function to create/clear the interval
Interval only exists whilst interacting with a node
Recycle LGraphCanvas.mouse isntead of creating another variable doing the same thing
- Variable already gets updated every down and move event
- Only using the data from a move event and storing its value could introduce a bug
  - Could result in a wrong panning behaviour when selecting a node within the threshold but not moving the mouse
    - Would move in the direction of the prev. stored location (Worst case would be the opposite direction)
Added offset to LGraphCanvas.graph_mouse when dragging a link
- Spline rendering of links is based on this value not updating it would introduce a visual glitch
- Link/connector would stay at its prev. position when not moving the mouse within the threshold

I totally understand why you loathe the LGraphCanvas.ts in its current state. It's a hot mess.
BTW! Are there any plans on streamlining the link connectors? At the moment it's a bit of a pain to detect if a link is being dragged in comparison to an item. With items you already have #startDraggingItems. Any plans on doing something similar with links?

Let me know what you think.
Cheers,
Leo

…ning as an interval to prevent speed from varying based on fps

… (fixed failing tests)

ADD: Automatically move the viewport when dragging an item close to t…

12454b2

…he canvas borders

webfiltered requested changes Feb 10, 2025

View reviewed changes

LotzF added 5 commits February 11, 2025 10:48

UPDATE: Moved Vector2Clamped function to measure.ts and changed Point…

fc02280

… to const number

UPDATE: Moved AutoPanCanvas to LGraphCanvas, switched from tick to in…

138d42a

…terval, changed Point to const number for calculations, set up variables for UI (threshold, speed, enable)

Merge branch 'Comfy-Org:master' into master

8367685

UPDATE: Removed unnecessary if statement, fixed typedefinition for au…

ff3a4e7

…to_panning_event

BUGFIX: SetAutoPanCanvas now works with disconnected input links

22824bf

LotzF requested a review from webfiltered February 12, 2025 08:59

LotzF added 2 commits February 12, 2025 12:40

BUGFIX: Use interval timing as deltaT instead of render_time when run…

31bb630

…ning as an interval to prevent speed from varying based on fps

BUGFIX: Added trailing comma, removed Positionables from DragAndScale…

c290207

… (fixed failing tests)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ADD: Automatically move the viewport when dragging an item close to t… #517

ADD: Automatically move the viewport when dragging an item close to t… #517

LotzF commented Feb 10, 2025

webfiltered left a comment

webfiltered Feb 10, 2025

LotzF Feb 10, 2025

webfiltered Feb 10, 2025 •

edited

Loading

LotzF Feb 10, 2025

webfiltered Feb 11, 2025

webfiltered Feb 10, 2025 •

edited

Loading

LotzF Feb 10, 2025

webfiltered Feb 10, 2025

LotzF commented Feb 10, 2025

LotzF commented Feb 11, 2025 •

edited

Loading

	function renderFrame(this: LGraphCanvas) {
	if (!this.pause_rendering) {
	this.draw()
	}

ADD: Automatically move the viewport when dragging an item close to t… #517

Are you sure you want to change the base?

ADD: Automatically move the viewport when dragging an item close to t… #517

Conversation

LotzF commented Feb 10, 2025

webfiltered left a comment

Choose a reason for hiding this comment

webfiltered Feb 10, 2025

Choose a reason for hiding this comment

LotzF Feb 10, 2025

Choose a reason for hiding this comment

webfiltered Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

LotzF Feb 10, 2025

Choose a reason for hiding this comment

webfiltered Feb 11, 2025

Choose a reason for hiding this comment

webfiltered Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

LotzF Feb 10, 2025

Choose a reason for hiding this comment

webfiltered Feb 10, 2025

Choose a reason for hiding this comment

LotzF commented Feb 10, 2025

LotzF commented Feb 11, 2025 • edited Loading

webfiltered Feb 10, 2025 •

edited

Loading

webfiltered Feb 10, 2025 •

edited

Loading

LotzF commented Feb 11, 2025 •

edited

Loading