Cleanup: Vue <--> Litegraph scaling logic. #6745
Conversation
dosubot
bot
added
the
size:L
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/22/2025, 01:44:06 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/22/2025, 01:54:15 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.13 MB (baseline 3.13 MB) • 🟢 -704 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 940 kB (baseline 940 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 7.97 kB (baseline 7.97 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 306 kB (baseline 306 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 141 kB (baseline 141 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 5.32 MB (baseline 5.32 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.87 MB (baseline 3.87 MB) • ⚪ 0 BBundles that do not match a named category
Status: 18 added / 18 removed |
DrJKL
changed the title
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
LGTM seems like just refactoring
DrJKL
force-pushed
the
drjkl/map-territory
branch
from
f423f42 to
f05201e
Compare
DrJKL
force-pushed
the
drjkl/map-territory
branch
from
f05201e to
6d2d50a
Compare
DrJKL
force-pushed
the
drjkl/map-territory
branch
from
6d2d50a to
5f964b9
Compare
DrJKL
force-pushed
the
drjkl/map-territory
branch
2 times, most recently
from
09ff304 to
405fcb3
Compare
📝 WalkthroughWalkthroughThis PR renames the exported type Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ensureCorrectLayoutScale
participant Graph
participant LayoutStore
Caller->>ensureCorrectLayoutScale: ensureCorrectLayoutScale(renderer='LG', targetGraph?)
activate ensureCorrectLayoutScale
ensureCorrectLayoutScale->>ensureCorrectLayoutScale: Check auto-scale flag (early return if disabled)
ensureCorrectLayoutScale->>Graph: Validate graph & gather nodes/groups/reroutes/IO
ensureCorrectLayoutScale->>ensureCorrectLayoutScale: Compute needsUpscale / needsDownscale and scaleFactor
ensureCorrectLayoutScale->>ensureCorrectLayoutScale: Determine onActiveGraph (is active graph?)
Note over ensureCorrectLayoutScale,Graph: For each element: compute origin-relative positions\nand sizes, apply scaleFactor
ensureCorrectLayoutScale->>Graph: Apply in-memory layout mutations
alt onActiveGraph
ensureCorrectLayoutScale->>LayoutStore: Emit batched layout updates
end
ensureCorrectLayoutScale->>Graph: Set graph.extra.workflowRendererVersion
ensureCorrectLayoutScale->>Caller: Return
deactivate ensureCorrectLayoutScale
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/litegraph/src/LGraph.ts(2 hunks)src/renderer/extensions/vueNodes/layout/ensureCorrectLayoutScale.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/renderer/extensions/vueNodes/layout/ensureCorrectLayoutScale.ts (5)
src/lib/litegraph/src/LGraph.ts (2)
RendererType(82-82)LGraph(123-2389)src/composables/useVueFeatureFlags.ts (1)
useVueFeatureFlags(37-39)src/renderer/core/layout/types.ts (1)
NodeBoundsUpdate(34-37)src/lib/litegraph/src/litegraph.ts (1)
LiteGraph(17-17)src/renderer/core/layout/store/layoutStore.ts (1)
layoutStore(1458-1458)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
- GitHub Check: setup
src/renderer/extensions/vueNodes/layout/ensureCorrectLayoutScale.ts
Outdated
Show resolved
Hide resolved
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
DrJKL
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
bot
removed
the
New Browser Test Expectations
DrJKL
force-pushed
the
drjkl/map-territory
branch
from
405fcb3 to
0c88396
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/renderer/extensions/vueNodes/layout/ensureCorrectLayoutScale.ts (1)
102-121: Reroute scaling is now origin‑relative; consider hoistinguseLayoutMutationsout of the loopThe reroute transform matches the node logic and correctly scales around the graph origin via:
const scaledX = originX + relativeX * scaleFactor const scaledY = originY + relativeY * scaleFactorwhich should prevent the origin‑drift issue seen before. Updating
reroute.posdirectly and then emitting amoveRerouteonly whenonActiveGraph && shouldRenderVueNodes.valuealso makes sense.You can avoid repeatedly creating the composable inside the loop by hoisting it:
- for (const reroute of graph.reroutes.values()) { + const layoutMutations = + onActiveGraph && shouldRenderVueNodes.value + ? useLayoutMutations() + : null + + for (const reroute of graph.reroutes.values()) { const [oldX, oldY] = reroute.pos @@ - reroute.pos = [scaledX, scaledY] - - if (onActiveGraph && shouldRenderVueNodes.value) { - const layoutMutations = useLayoutMutations() - layoutMutations.moveReroute( + reroute.pos = [scaledX, scaledY] + + if (layoutMutations) { + layoutMutations.moveReroute( reroute.id, { x: scaledX, y: scaledY }, { x: oldX, y: oldY } ) } }This keeps behavior identical while reducing per‑reroute setup work.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/extensions/vueNodes/layout/ensureCorrectLayoutScale.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/renderer/extensions/vueNodes/layout/ensureCorrectLayoutScale.ts (5)
src/lib/litegraph/src/LGraph.ts (2)
RendererType(82-82)LGraph(123-2389)src/composables/useVueFeatureFlags.ts (1)
useVueFeatureFlags(37-39)src/renderer/core/layout/types.ts (1)
NodeBoundsUpdate(34-37)src/renderer/core/layout/store/layoutStore.ts (1)
layoutStore(1458-1458)src/renderer/core/layout/operations/layoutMutations.ts (1)
useLayoutMutations(67-340)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
🔇 Additional comments (4)
src/renderer/extensions/vueNodes/layout/ensureCorrectLayoutScale.ts (4)
2-3: RendererType usage, guards, and active-graph gating look consistentImporting
RendererTypeand defaultingrendererto'LG'keeps the API explicit while still allowing callers to omit the argument. TheautoScaleLayoutSettingand!needsUpscale && !needsDownscaleguards correctly avoid unnecessary work when scaling isn’t required, andonActiveGraphensures layout mutations and canvas scaling only touch the active graph. TheworkflowRendererVersion ??= rendererin the no-scale path also safely initializes the version without overwriting an existing value.Also applies to: 15-24, 28-38, 51-53
45-76: Node scaling is origin‑relative; verify that height transform won’t drift on repeated LG↔Vue togglesThe node transform around
[originX, originY](including theNODE_TITLE_HEIGHTY-offset and the batchedlayoutStore.batchUpdateNodeBoundsupdates) looks consistent and keeps positions anchored to the graph bounds.One subtle point: the height formula
const scaledHeight = lgNode.height * scaleFactor - (needsUpscale ? 0 : LiteGraph.NODE_TITLE_HEIGHT)only subtracts
NODE_TITLE_HEIGHTon the downscale path. Algebraically, anLG → Vue → LGround‑trip will reducelgNode.size[1]byLiteGraph.NODE_TITLE_HEIGHT, and repeating that cycle would continue shrinking heights.If
ensureCorrectLayoutScalecan be invoked multiple times for the same graph as the renderer is toggled back and forth, this will cause cumulative height drift; if it’s guaranteed to run only once per graph per direction (e.g. a one‑time migration), it’s fine. Please confirm the intended call pattern and, if repeated toggles are possible, consider adjusting the height mapping (or explicitly tracking conversion state) to make the transform involutive.Also applies to: 79-82, 85-92, 98-100
123-143: IO node scaling matches the shared origin‑relative patternSubgraph input/output nodes use the same origin‑relative position transform and simple size scaling as reroutes, without extra title‑height adjustments. That keeps their layout consistent across LG↔Vue transitions and avoids additional drift.
35-38: Group and canvas scaling look correct; confirmworkflowRendererVersioncontract at call sitesThe group transform mirrors the node logic (including
NODE_TITLE_HEIGHTY-offsets) but keeps height scaling purely multiplicative, which makes the LG↔Vue↔LG mapping for groups mathematically involutive—no cumulative drift there. TheonActiveGraph && canvasguard aroundchangeScaleand pivoting atoriginScreenalso ensure only the active graph’s canvas is rescaled and that the visual layout stays fixed while coordinates are renormalized.For
graph.extra.workflowRendererVersion:
- In the no-scale path you now set
workflowRendererVersion ??= renderer, so an existing value is preserved.- When scaling occurs you unconditionally set it to
'Vue'or'LG'based on the direction.This is sound assuming callers always pass the renderer that produced the current layout (i.e., the “previous renderer”) and only invoke
ensureCorrectLayoutScalewhen that value disagrees with the currentshouldRenderVueNodesflag. Please double-check the call sites follow that contract; otherwise, layouts might be left unscaled or scaled more than once.Also applies to: 146-165, 167-173
DrJKL
changed the title
DrJKL
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
Consistent names and order of operations
DrJKL
force-pushed
the
drjkl/map-territory
branch
from
0c88396 to
18322e0
Compare
Consistent names and order of operations. This does not fix the resizing on load.
┆Issue is synchronized with this Notion page by Unito