Commit Graph

2 Commits

Author SHA1 Message Date
Terry Jia
4a3098f1f2 performance fix: prevent gcd infinite loop with floating-point step values (#7258)
## Summary

report and fix https://github.com/Comfy-Org/ComfyUI_frontend/issues/3919

- Convert recursive gcd to iterative to avoid stack overflow
- Add epsilon tolerance (1e-10) for floating-point precision issues

This fixes workflow loading hangs when node trying merge values like
0.01 and 0.001, which caused the original recursive gcd to run
indefinitely due to floating-point modulo never reaching exactly zero.

please notice, we need both iterative and epsilon together to fix this
gcd issue

Call Chain

PrimitiveNode.onAfterGraphConfigured
  → #mergeWidgetConfig
    → #isValidConnection
      → mergeIfValid
        → mergeInputSpec
          → mergeNumericInputSpec
            → lcm(step1, step2)
              → gcd(a, b)  ← Problem here

Why It Happened
When some nodes connect to multiple nodes, it may merge values using
LCM, which internally calls GCD.

Original recursive implementation:
```
export const gcd = (a: number, b: number): number => {
   return b === 0 ? a : gcd(b, a % b)
}
```

Issues:
1. Stack Overflow: Recursive calls with many nodes exhausted the call
stack.
2. Floating-Point Precision: For values like gcd(0.01, 0.001):
 ` 0.01 % 0.001 = 0.0009999999999999994  // Not exactly 0!`
3. Due to Ifloating-point representation, the modulo never reaches
exactly zero, causing hundreds or thousands of iterations.

## Screenshots
### before


https://github.com/user-attachments/assets/cca4342c-a882-4590-a8d4-1e0bea19e5b7

### fix with only iterative, without epsilon


https://github.com/user-attachments/assets/1dc52aa4-a86a-40b5-8bac-904094c4c36b


### final fix with iterative and epsilon

https://github.com/user-attachments/assets/7b868b50-c3c9-4be4-8594-27cecbc08a26

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7258-performance-fix-prevent-gcd-infinite-loop-with-floating-point-step-values-2c46d73d3650818cbe8cf455c934a114)
by [Unito](https://www.unito.io)
2025-12-08 20:41:23 -05:00
Christian Byrne
b72e22f6be Add Centralized Vue Node Size/Pos Tracking (#5442)
* add dom element resize observer registry for vue node components

* Update src/renderer/extensions/vueNodes/composables/useVueNodeResizeTracking.ts

Co-authored-by: AustinMroz <austin@comfy.org>

* refactor(vue-nodes): typed TransformState InjectionKey, safer ResizeObserver sizing, centralized slot tracking, and small readability updates

* chore: make TransformState interface non-exported to satisfy knip pre-push

* Revert "chore: make TransformState interface non-exported to satisfy knip pre-push"

This reverts commit 110ecf31da.

* Revert "refactor(vue-nodes): typed TransformState InjectionKey, safer ResizeObserver sizing, centralized slot tracking, and small readability updates"

This reverts commit 428752619c.

* [refactor] Improve resize tracking composable documentation and test utilities

- Rename parameters in useVueElementTracking for clarity (appIdentifier, trackingType)
- Add comprehensive docstring with examples to prevent DOM attribute confusion
- Extract mountLGraphNode test utility to eliminate repetitive mock setup
- Add technical implementation notes documenting optimization decisions

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* remove typo comment

* convert to functional bounds collection

* remove inline import

* add interfaces for bounds mutations

* remove change log

* fix bounds collection when vue nodes turned off

* fix title offset on y

* move from resize observer to selection toolbox bounds

---------

Co-authored-by: AustinMroz <austin@comfy.org>
Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
2025-09-10 22:38:49 -07:00