Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds curve types and a node. Introduces 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_extras/nodes_custom_sampler.py`:
- Around line 1069-1089: In execute (nodes_custom_sampler.execute) clamp the
interpolated y to [0.0,1.0] before converting to a sigma and avoid calling
model_sampling.percent_to_sigma with a 0.0 sentinel: compute percent = 1.0 -
clamp(y,0.0,1.0) and if percent <= 0.0 use model_sampling.sigma_max.item() (or
the equivalent sigma_max from the model) instead of percent_to_sigma(0.0);
otherwise call model_sampling.percent_to_sigma(percent) and append that float to
sigmas, keeping the existing fallback (y * sigma_max) when model_sampling is
None and preserving the final sigmas[-1] = 0.0 behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f409527-bbae-4232-99ed-5da206a7f411
📒 Files selected for processing (2)
comfy_extras/nodes_custom_sampler.pynodes.py
|
@christian-byrne I merged your changes and follow your pattern to add one more type linear curve FE change is Comfy-Org/ComfyUI_frontend#10118 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_api/latest/_input/basic_types.py`:
- Around line 226-227: LinearCurve.interp_array currently calls np.interp
directly and raises ValueError for empty control-point lists; update
interp_array to mirror interp()'s behavior by checking if self._xs/self._ys are
empty (e.g., if not self._xs or len(self._xs)==0) and in that case return an
array of zeros shaped like xs_in (use np.zeros_like(xs_in)), otherwise call
np.interp as before; reference: LinearCurve.interp_array, LinearCurve.interp(),
and to_lut().
In `@nodes.py`:
- Around line 2039-2058: The node's execute method assumes curve is a dict with
"points" but INPUT_TYPES advertises a bare point list; update execute to accept
both shapes: if curve is a CurveInput return it, elif isinstance(curve, (list,
tuple)) treat it as the raw point list, else if it's a dict read points =
curve.get("points") and interpolation =
curve.get("interpolation","monotone_cubic"); then build points =
[(float(x),float(y))...] and return LinearCurve(points) or
MonotoneCubicCurve(points) as before (referencing INPUT_TYPES, execute,
CurveInput, LinearCurve, MonotoneCubicCurve).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87c6f8cb-67ef-4888-b880-9dc0595ec77e
📒 Files selected for processing (4)
comfy_api/input/__init__.pycomfy_api/latest/_input/__init__.pycomfy_api/latest/_input/basic_types.pynodes.py
🚧 Files skipped from review as they are similar to previous changes (1)
- comfy_api/input/init.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_api/latest/_input/basic_types.py`:
- Around line 172-175: The edge-case branches in Monotone.interp_array (when n
== 0 or n == 1) currently use np.zeros_like(xs_in) and np.full_like(xs_in,
ys[0]), which inherit xs_in's dtype and can truncate floats when xs_in is
integer; change these to explicitly return float arrays (e.g. use
dtype=np.float64) so outputs match LinearCurve.interp_array's behavior; update
the branches referencing xs_in and ys in the Monotone.interp_array method to
construct results with dtype=np.float64.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d9511f24-c1aa-4bea-ad53-bd4cf84925b9
📒 Files selected for processing (4)
comfy_api/input/__init__.pycomfy_api/latest/_input/__init__.pycomfy_api/latest/_input/basic_types.pynodes.py
🚧 Files skipped from review as they are similar to previous changes (1)
- comfy_api/input/init.py
## Summary
Change the CURVE widget value from CurvePoint[] to CurveData ({ points,
interpolation }) to support multiple interpolation types. Add a Select
dropdown in the widget UI for switching between Smooth (monotone cubic)
and Linear interpolation, with the SVG preview updating accordingly.
- Add CurveData type with CURVE_INTERPOLATIONS const enum
- Add createLinearInterpolator with piecewise linear + binary search
- Add createInterpolator factory dispatching by interpolation type
- Add isCurveData type guard in curveUtils
- Update ICurveWidget value type to CurveData
- Add interpolation prop to CurveEditor and useCurveEditor composable
- Linear mode generates direct M...L... SVG path (no sampling)
- Add i18n entries for interpolation labels
- Add unit tests for createLinearInterpolator
BE change is Comfy-Org/ComfyUI#12757
## Screenshots (if applicable)
<img width="1437" height="670" alt="image"
src="https://github.com/user-attachments/assets/550aedec-e5da-425b-8233-86a4f28067fa"
/>
<img width="1445" height="648" alt="image"
src="https://github.com/user-attachments/assets/0a8dc654-3f92-4ca2-9fa2-c1fef3be6d66"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10118-feat-add-linear-interpolation-type-to-CURVE-widget-3256d73d36508185a86edf73bb555c51)
by [Unito](https://www.unito.io)
## Summary
Change the CURVE widget value from CurvePoint[] to CurveData ({ points,
interpolation }) to support multiple interpolation types. Add a Select
dropdown in the widget UI for switching between Smooth (monotone cubic)
and Linear interpolation, with the SVG preview updating accordingly.
- Add CurveData type with CURVE_INTERPOLATIONS const enum
- Add createLinearInterpolator with piecewise linear + binary search
- Add createInterpolator factory dispatching by interpolation type
- Add isCurveData type guard in curveUtils
- Update ICurveWidget value type to CurveData
- Add interpolation prop to CurveEditor and useCurveEditor composable
- Linear mode generates direct M...L... SVG path (no sampling)
- Add i18n entries for interpolation labels
- Add unit tests for createLinearInterpolator
BE change is Comfy-Org/ComfyUI#12757
## Screenshots (if applicable)
<img width="1437" height="670" alt="image"
src="https://github.com/user-attachments/assets/550aedec-e5da-425b-8233-86a4f28067fa"
/>
<img width="1445" height="648" alt="image"
src="https://github.com/user-attachments/assets/0a8dc654-3f92-4ca2-9fa2-c1fef3be6d66"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10118-feat-add-linear-interpolation-type-to-CURVE-widget-3256d73d36508185a86edf73bb555c51)
by [Unito](https://www.unito.io)
## Summary
Change the CURVE widget value from CurvePoint[] to CurveData ({ points,
interpolation }) to support multiple interpolation types. Add a Select
dropdown in the widget UI for switching between Smooth (monotone cubic)
and Linear interpolation, with the SVG preview updating accordingly.
- Add CurveData type with CURVE_INTERPOLATIONS const enum
- Add createLinearInterpolator with piecewise linear + binary search
- Add createInterpolator factory dispatching by interpolation type
- Add isCurveData type guard in curveUtils
- Update ICurveWidget value type to CurveData
- Add interpolation prop to CurveEditor and useCurveEditor composable
- Linear mode generates direct M...L... SVG path (no sampling)
- Add i18n entries for interpolation labels
- Add unit tests for createLinearInterpolator
BE change is Comfy-Org/ComfyUI#12757
## Screenshots (if applicable)
<img width="1437" height="670" alt="image"
src="https://github.com/user-attachments/assets/550aedec-e5da-425b-8233-86a4f28067fa"
/>
<img width="1445" height="648" alt="image"
src="https://github.com/user-attachments/assets/0a8dc654-3f92-4ca2-9fa2-c1fef3be6d66"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10118-feat-add-linear-interpolation-type-to-CURVE-widget-3256d73d36508185a86edf73bb555c51)
by [Unito](https://www.unito.io)
## Summary
Change the CURVE widget value from CurvePoint[] to CurveData ({ points,
interpolation }) to support multiple interpolation types. Add a Select
dropdown in the widget UI for switching between Smooth (monotone cubic)
and Linear interpolation, with the SVG preview updating accordingly.
- Add CurveData type with CURVE_INTERPOLATIONS const enum
- Add createLinearInterpolator with piecewise linear + binary search
- Add createInterpolator factory dispatching by interpolation type
- Add isCurveData type guard in curveUtils
- Update ICurveWidget value type to CurveData
- Add interpolation prop to CurveEditor and useCurveEditor composable
- Linear mode generates direct M...L... SVG path (no sampling)
- Add i18n entries for interpolation labels
- Add unit tests for createLinearInterpolator
BE change is Comfy-Org/ComfyUI#12757
## Screenshots (if applicable)
<img width="1437" height="670" alt="image"
src="https://github.com/user-attachments/assets/550aedec-e5da-425b-8233-86a4f28067fa"
/>
<img width="1445" height="648" alt="image"
src="https://github.com/user-attachments/assets/0a8dc654-3f92-4ca2-9fa2-c1fef3be6d66"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10118-feat-add-linear-interpolation-type-to-CURVE-widget-3256d73d36508185a86edf73bb555c51)
by [Unito](https://www.unito.io)
## Summary
Change the CURVE widget value from CurvePoint[] to CurveData ({ points,
interpolation }) to support multiple interpolation types. Add a Select
dropdown in the widget UI for switching between Smooth (monotone cubic)
and Linear interpolation, with the SVG preview updating accordingly.
- Add CurveData type with CURVE_INTERPOLATIONS const enum
- Add createLinearInterpolator with piecewise linear + binary search
- Add createInterpolator factory dispatching by interpolation type
- Add isCurveData type guard in curveUtils
- Update ICurveWidget value type to CurveData
- Add interpolation prop to CurveEditor and useCurveEditor composable
- Linear mode generates direct M...L... SVG path (no sampling)
- Add i18n entries for interpolation labels
- Add unit tests for createLinearInterpolator
BE change is Comfy-Org/ComfyUI#12757
## Screenshots (if applicable)
<img width="1437" height="670" alt="image"
src="https://github.com/user-attachments/assets/550aedec-e5da-425b-8233-86a4f28067fa"
/>
<img width="1445" height="648" alt="image"
src="https://github.com/user-attachments/assets/0a8dc654-3f92-4ca2-9fa2-c1fef3be6d66"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10118-feat-add-linear-interpolation-type-to-CURVE-widget-3256d73d36508185a86edf73bb555c51)
by [Unito](https://www.unito.io)
There was a problem hiding this comment.
The curve code is fine in the PR, but this should be restructured to not add code to nodes.py; that file is already cluttered with a bunch of legacy code so it would be best not to add new things into it. The Curve editor node can be added to its own nodes_extras file and be written in V3. Extending the typing of v1 is not really worthwhile.
8e19305 to
040e219
Compare
CurveInput is an abstract base class so future curve representations (bezier, LUT-based, analytical functions) can be added without breaking downstream nodes that type-check against CurveInput. MonotoneCubicCurve is the concrete implementation that: - Mirrors frontend createMonotoneInterpolator (curveUtils.ts) exactly - Pre-computes slopes as numpy arrays at construction time - Provides vectorised interp_array() using numpy for batch evaluation - interp() for single-value evaluation - to_lut() for generating lookup tables CurveEditor node wraps raw widget points in MonotoneCubicCurve.
|
Add Histogram to support in curve editor
used in FE change in Comfy-Org/ComfyUI_frontend#10365
but I don't include image to histogram node in this PR, will add in follow up PR |
116b18d to
d7a4f7c
Compare
## Summary
Change the CURVE widget value from CurvePoint[] to CurveData ({ points,
interpolation }) to support multiple interpolation types. Add a Select
dropdown in the widget UI for switching between Smooth (monotone cubic)
and Linear interpolation, with the SVG preview updating accordingly.
- Add CurveData type with CURVE_INTERPOLATIONS const enum
- Add createLinearInterpolator with piecewise linear + binary search
- Add createInterpolator factory dispatching by interpolation type
- Add isCurveData type guard in curveUtils
- Update ICurveWidget value type to CurveData
- Add interpolation prop to CurveEditor and useCurveEditor composable
- Linear mode generates direct M...L... SVG path (no sampling)
- Add i18n entries for interpolation labels
- Add unit tests for createLinearInterpolator
BE change is Comfy-Org/ComfyUI#12757
## Screenshots (if applicable)
<img width="1437" height="670" alt="image"
src="https://github.com/user-attachments/assets/550aedec-e5da-425b-8233-86a4f28067fa"
/>
<img width="1445" height="648" alt="image"
src="https://github.com/user-attachments/assets/0a8dc654-3f92-4ca2-9fa2-c1fef3be6d66"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10118-feat-add-linear-interpolation-type-to-CURVE-widget-3256d73d36508185a86edf73bb555c51)
by [Unito](https://www.unito.io)



CurveEditornode as a general-purpose utility for editing curves