diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 48adc9c..1960a6a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,6 +6,9 @@ on: - main pull_request: +permissions: + contents: read + jobs: pytest: name: pytest (Python ${{ matrix.python-version }}) diff --git a/AGENTS.md b/AGENTS.md index 6fa7904..aba33a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,11 +4,11 @@ A ComfyUI node that resizes any image to fit your model's resolution rules: a mi ## Current state -_Last verified: 2026-08-08_ +_Last verified: 2026-08-19_ -- **Status:** released v2.3.2 (`pyproject.toml`). Stable and feature-complete for what it sets out to do. Published to the Comfy Registry — `.github/workflows/publish_action.yml` fires on a `pyproject.toml` version change on `main`, so a functional change needs a version bump or it never ships. +- **Status:** released v2.4.0 (`pyproject.toml`). Stable and feature-complete for what it sets out to do. Published to the Comfy Registry — `.github/workflows/publish_action.yml` fires on a `pyproject.toml` version change on `main`, so a functional change needs a version bump or it never ships. - **Works:** both constraint modes (Prioritize Min Resolution, Prioritize Max Resolution strict); divisibility by 2/8/16/32/64 enforced together with the min/max limits so reported width/height always match the output tensor; minimal smart cropping with a configurable crop position; all five resize methods; the full output set (resized image, original passthrough, width, height, final and original aspect ratios). `.github/workflows/test.yml` runs the pytest suite. -- **In progress:** nothing — the last release hardened `execute()` edge cases and added CI, closing out the v2.3 line. +- **In progress:** nothing — v2.4.0 closed out a full audit: a memory-exhaustion guard on extreme-aspect-ratio upscales, fixes for configurations that silently no-opped, a `validate_inputs` signature that was disabling ComfyUI's own input validation, and removal of a duplicate resize. - **Known gaps / next steps:** no batch-specific handling beyond what ComfyUI's own resizer provides; behaviour under extreme aspect ratios is covered by tests but has had little real-world exercise; the README screenshots track v2.3.1 and would need refreshing alongside any visible change. - **Deep docs:** none — `README.md` is the user-facing reference and `nodes.py` is the whole implementation. @@ -53,6 +53,8 @@ pytest - Single-file node — `nodes.py` is the entire feature. Keep it self-contained. - Zero pip dependencies. Drops into `custom_nodes/` — no pip install needed. - Divisibility, min/max, and crop dimensions are enforced together — reported width/height always match the actual output tensor. +- Unsatisfiable or memory-exhausting settings raise `ValueError` rather than returning a zero dimension; `calculate_optimal_dimensions` never returns 0 for a non-empty input. +- `validate_inputs` must NOT take `**kwargs`: ComfyUI skips its own range and combo-option validation for every input on the node if it does (see `validate_inputs` in ComfyUI's `execution.py`). - `resize_method` defaults to `lanczos` (sharpest). Options: lanczos, bicubic, bilinear, nearest-exact (pixel art/masks), area (big downscales). - When `crop_as_required=False` and `multiple_of=1`, no cropping occurs — keeps every pixel. - The node uses ComfyUI v3 schema (`comfy_api.latest`). @@ -61,7 +63,7 @@ pytest This file is **public-safe by default**. Never add local paths, credentials, API keys, personal data, infrastructure details, or subscription info. -Before pushing: `pwsh scripts/check-agents-md.ps1 AGENTS.md CLAUDE.md` — must exit 0. +Before pushing, re-read this file and confirm it contains no local paths, credentials, or personal data. ## Maintenance diff --git a/README.md b/README.md index 7578ab0..44d755e 100644 --- a/README.md +++ b/README.md @@ -24,16 +24,18 @@ This node applies all three rules in one step and **guarantees divisibility on e |---|---|---| | `image` | — | Image (or batch) to resize | | `min_res` | 704 | Neither dimension will end up below this | -| `max_res` | 1280 | Neither dimension will end up above this | +| `max_res` | 1280 | Bounds the longest side. In the default mode the long side may exceed it on extreme aspect ratios — see constraint modes below | | `multiple_of` | 2 | Output width and height are always divisible by this (1 disables) | | `resize_method` | lanczos | Interpolation: `lanczos` (sharpest), `bicubic`, `bilinear`, `nearest-exact` (pixel art/masks), `area` (big downscales) | | `constraint_mode` | Prioritize Min Resolution | What wins when an extreme aspect ratio can't satisfy both limits (see below) | | `crop_as_required` | True | Crop minimally to hit exact dimensions instead of distorting the aspect ratio | | `crop_position` | center | Which part to keep when cropping: center / top / bottom / left / right | +Every image is rescaled so its longest side lands on `max_res` — images already inside the range are resized too, not passed through. This is what makes the output size predictable for batching. + **Constraint modes** — with a very wide or very tall image, min and max can conflict: -- **Prioritize Min Resolution**: neither dimension goes below `min_res`, even if the long side must exceed `max_res`. Best default. +- **Prioritize Min Resolution**: neither dimension goes below `min_res`, even if the long side must exceed `max_res`. Best default. Beyond roughly a 52:1 aspect ratio the required output would be large enough to exhaust memory, so the node reports an error instead of trying — switch to Strict mode for those. - **Prioritize Max Resolution (Strict)**: output always fits in a `max_res × max_res` box (hard VRAM cap), even if the short side lands below `min_res`. ## Outputs @@ -68,9 +70,11 @@ Restart ComfyUI. No dependencies beyond ComfyUI itself (Python 3.10+). - Resizing uses ComfyUI's own resizer (`comfy.utils.common_upscale`), so results match core resize nodes; bicubic/lanczos output is clamped to the valid range to avoid overshoot artifacts. - Divisibility, min/max, and crop dimensions are enforced together — the reported `width`/`height` always match the actual output tensor. - Built on the ComfyUI v3 node API (`comfy_api.latest`). +- Impossible settings fail with a clear error rather than silently passing the image through — e.g. `multiple_of` larger than `max_res` in Strict mode, where no valid output size exists. ## Version history +- **v2.4.0**: Refuse extreme-aspect-ratio upscales that would exhaust memory instead of attempting them (a 1×690 input at default settings previously asked for 704×485760, roughly 4 GB per batch item); fixed settings that silently passed the image through untouched (`multiple_of` above `max_res` in Strict mode, and thin strips at `multiple_of=1`); restored ComfyUI's built-in range and combo-option validation, which the previous `validate_inputs` signature disabled for every input; ~1.85× faster when cropping by dropping a resize whose result was discarded; crop decisions now use exact aspect ratios instead of 4-decimal rounded ones - **v2.3.2**: Added CI test workflow (pytest on Python 3.10–3.12); warn on very large upscales in "Prioritize Min Resolution" mode; pass malformed zero-dimension inputs through unchanged instead of erroring - **v2.3.1**: Rewrote README with current screenshots; enforce `min_res` even when rounding to a large `multiple_of` would dip below it - **v2.3.0**: Added `resize_method` selection (lanczos default); fixed a rare off-by-one in cropped output size; removed unused numpy dependency; Python 3.10 compatibility; proper logging diff --git a/__init__.py b/__init__.py index 127cc37..0f1cd28 100644 --- a/__init__.py +++ b/__init__.py @@ -1,5 +1,12 @@ +import logging + +logger = logging.getLogger(__name__) + try: from .nodes import comfy_entrypoint __all__ = ['comfy_entrypoint'] except (ImportError, SystemError): - pass + # Without this log the pack just silently fails to appear in ComfyUI, + # leaving nothing to diagnose. + logger.exception("ComfyUI-ConstrainResolution failed to load; its node will be unavailable.") + __all__ = [] diff --git a/nodes.py b/nodes.py index 50b4ec6..3586174 100644 --- a/nodes.py +++ b/nodes.py @@ -9,6 +9,16 @@ from comfy_api.latest import ComfyExtension, io logger = logging.getLogger(__name__) +# "Prioritize Min Resolution" has no natural upper bound: holding both sides at +# or above min_res on an extreme aspect ratio scales the long side without +# limit. A 1x690 input at the default settings asks for 704x485760 — over 4 GB +# per batch item, enough to OOM the whole ComfyUI process. Refuse to allocate +# more than this many max_res-sized boxes and tell the user to switch modes. +MAX_OUTPUT_PIXEL_BUDGET_FACTOR = 16 + +# Upscales beyond this factor are legal but worth flagging in the log. +UPSCALE_WARN_FACTOR = 4 + # (str, Enum) instead of StrEnum keeps the pack importable on Python 3.10, # which ComfyUI still supports. @@ -74,14 +84,22 @@ class ConstrainResolution(io.ComfyNode): default=704, min=1, max=65536, - tooltip="Minimum resolution in pixels for width and height. Images smaller than this will be upscaled." + tooltip=( + "Minimum resolution in pixels for width and height. " + "Neither output dimension will fall below this." + ) ), io.Int.Input( "max_res", default=1280, min=1, max=65536, - tooltip="Maximum resolution in pixels for width and height. Images larger than this will be downscaled." + tooltip=( + "Maximum resolution in pixels. Every image is rescaled so its longest side " + "lands here, so images already in range are resized too. In " + "'Prioritize Min Resolution' mode the long side may exceed this on extreme " + "aspect ratios." + ) ), io.Int.Input( "multiple_of", @@ -175,8 +193,15 @@ class ConstrainResolution(io.ComfyNode): ) @classmethod - def validate_inputs(cls, min_res, max_res, multiple_of, **kwargs): - """Validate input parameters""" + def validate_inputs(cls, min_res, max_res, multiple_of): + """Validate input parameters. + + The signature deliberately declares only the values checked here and + takes no **kwargs: ComfyUI skips its own range and combo-option + validation for every input once this function accepts **kwargs + (see ``validate_inputs`` in ComfyUI's execution.py). Listing just these + three leaves the combo inputs to ComfyUI's built-in checks. + """ if max_res < min_res: return f"max_res ({max_res}) must be greater than or equal to min_res ({min_res})" @@ -197,9 +222,11 @@ class ConstrainResolution(io.ComfyNode): @staticmethod def round_to_multiple(value: int, multiple: int) -> int: - """Round a value to the nearest multiple of the specified number""" + """Round a value to the nearest multiple, never dropping below one pixel""" + if multiple < 1: + raise ValueError(f"multiple_of must be at least 1, got {multiple}") if multiple == 1: - return value + return max(1, value) return max(multiple, multiple * round(value / multiple)) @staticmethod @@ -212,6 +239,12 @@ class ConstrainResolution(io.ComfyNode): constraint_mode: str ) -> Tuple[int, int]: """Calculate optimal dimensions based on constraints""" + if multiple_of < 1: + raise ValueError(f"multiple_of must be at least 1, got {multiple_of}") + if constraint_mode not in (ConstraintMode.MIN_RES.value, ConstraintMode.MAX_RES_STRICT.value): + # Silently falling through would apply neither the min_res floor nor + # the max_res clamp, quietly violating both limits. + raise ValueError(f"Unknown constraint_mode: {constraint_mode!r}") if height == 0 or width == 0: return 0, 0 @@ -258,9 +291,33 @@ class ConstrainResolution(io.ComfyNode): # Clamp to the largest valid multiple of multiple_of that is <= max_res. if constraint_mode == ConstraintMode.MAX_RES_STRICT.value: max_allowed = (max_res // multiple_of) * multiple_of + if max_allowed == 0: + # No positive multiple of multiple_of fits inside max_res, so the + # request is unsatisfiable. Clamping anyway would return 0 and + # silently pass the image through untouched. + raise ValueError( + f"multiple_of ({multiple_of}) is larger than max_res ({max_res}), so no valid " + f"output size exists in '{ConstraintMode.MAX_RES_STRICT.value}' mode. " + f"Lower multiple_of or raise max_res." + ) final_width = min(final_width, max_allowed) final_height = min(final_height, max_allowed) + # 5. Refuse absurd allocations rather than letting the resize OOM the + # process. Only reachable in min-res mode, which is the only mode + # without an upper bound. + if constraint_mode == ConstraintMode.MIN_RES.value: + budget = max(min_res, max_res) ** 2 * MAX_OUTPUT_PIXEL_BUDGET_FACTOR + if final_width * final_height > budget: + raise ValueError( + f"'{ConstraintMode.MIN_RES.value}' needs {final_width}x{final_height} " + f"({final_width * final_height / 1e6:.1f} MP) to hold both sides at or above " + f"min_res ({min_res}) for a {width}x{height} input. That exceeds the " + f"{budget / 1e6:.1f} MP safety limit and would likely exhaust memory. " + f"Switch to '{ConstraintMode.MAX_RES_STRICT.value}' to cap the output size, " + f"or lower min_res." + ) + return final_width, final_height @staticmethod @@ -268,7 +325,7 @@ class ConstrainResolution(io.ComfyNode): image: torch.Tensor, target_width: int, target_height: int, - method: str = ResizeMethod.BILINEAR.value + method: str = ResizeMethod.LANCZOS.value ) -> torch.Tensor: """ Resize image tensor to target dimensions. @@ -329,6 +386,14 @@ class ConstrainResolution(io.ComfyNode): """ batch, height, width, channels = image.shape + if target_width > width or target_height > height: + # Slicing past the end silently yields a smaller tensor than the + # width/height this node reports, so refuse instead. + raise ValueError( + f"crop target {target_width}x{target_height} is larger than the source " + f"image {width}x{height}; crop_image cannot grow an image." + ) + # Calculate crop amounts width_diff = width - target_width height_diff = height - target_height @@ -409,51 +474,43 @@ class ConstrainResolution(io.ComfyNode): # ratios can blow up the long side and risk OOM). Strict mode caps this instead. if constraint_mode == ConstraintMode.MIN_RES.value: upscale_factor = max(target_width / width, target_height / height) - if upscale_factor > 4: + if upscale_factor > UPSCALE_WARN_FACTOR: logger.warning( "Upscaling by %.1fx to %dx%d to satisfy min_res on an extreme aspect ratio. " "Use 'Prioritize Max Resolution (Strict)' to cap output size and avoid large upscales.", upscale_factor, target_width, target_height ) - # Resize image to target dimensions - resized_image = cls.resize_image(image, target_width, target_height, resize_method) - - # Calculate aspect ratio after resize final_aspect_ratio = cls.calculate_aspect_ratio(target_width, target_height) - # Check if cropping is needed and enabled - if crop_as_required and original_aspect_ratio > 0: - # Determine if we need to crop - # After rounding to multiples, the aspect ratio might have changed slightly - # We'll resize to maintain aspect ratio on the larger dimension, then crop + # Decide on cropping before resizing, so only one resize is ever done. + # Compare exact ratios rather than the 4-decimal rounded outputs: a very + # tall image rounds to 0.0, which would skip the crop and distort it. + exact_original_ratio = width / height + exact_final_ratio = target_width / target_height + aspect_ratio_deviation = abs( + (exact_final_ratio - exact_original_ratio) / exact_original_ratio * 100 + ) - aspect_ratio_deviation = abs((final_aspect_ratio - original_aspect_ratio) / original_aspect_ratio * 100) + if crop_as_required and aspect_ratio_deviation > 0.1: + # Resize preserving aspect ratio so one side overshoots, then crop back. + # ceil + max keep the intermediate dimension at or above the target, + # so the crop below never has to grow the image. + if exact_original_ratio > exact_final_ratio: + # Original is wider: match height, overshoot and crop width + intermediate_width = max(target_width, math.ceil(target_height * width / height)) + resized_image = cls.resize_image(image, intermediate_width, target_height, resize_method) + else: + # Original is taller: match width, overshoot and crop height + intermediate_height = max(target_height, math.ceil(target_width * height / width)) + resized_image = cls.resize_image(image, target_width, intermediate_height, resize_method) - if aspect_ratio_deviation > 0.1: # If more than 0.1% deviation - # Resize to preserve aspect ratio, making one dimension larger than target, then crop - # ceil + max keep the intermediate dimension at or above the target, - # so the crop below never has to grow the image - if width / height > target_width / target_height: - # Original is wider (more landscape), resize based on height and crop width - # This ensures height matches target, width will be larger and cropped - intermediate_width = max(target_width, math.ceil(target_height * width / height)) - resized_image = cls.resize_image(image, intermediate_width, target_height, resize_method) - else: - # Original is taller (more portrait), resize based on width and crop height - # This ensures width matches target, height will be larger and cropped - intermediate_height = max(target_height, math.ceil(target_width * height / width)) - resized_image = cls.resize_image(image, target_width, intermediate_height, resize_method) + resized_image = cls.crop_image(resized_image, target_width, target_height, crop_position) + logger.debug("Image cropped to achieve exact dimensions %dx%d", target_width, target_height) + else: + resized_image = cls.resize_image(image, target_width, target_height, resize_method) - # Crop to exact target dimensions - resized_image = cls.crop_image(resized_image, target_width, target_height, crop_position) - - logger.debug("Image cropped to achieve exact dimensions %dx%d", target_width, target_height) - - # Log aspect ratio deviation warning if significant - if original_aspect_ratio > 0 and not crop_as_required: - aspect_ratio_deviation = abs((final_aspect_ratio - original_aspect_ratio) / original_aspect_ratio * 100) - if aspect_ratio_deviation > 1: # 1% tolerance for rounding + if not crop_as_required and aspect_ratio_deviation > 1: # 1% tolerance for rounding logger.info( "Aspect ratio changed by %.2f%% due to rounding. " "Enable 'Crop as Required' to preserve exact aspect ratio.", diff --git a/pyproject.toml b/pyproject.toml index c08ebc9..36d844a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [project] name = "constrainresolution" description = "Resize images to fit min/max resolution limits with dimensions guaranteed divisible by 2/8/16/32/64. Aspect-ratio-preserving resize with smart cropping — ideal for image-to-image and image-to-video workflows." -version = "2.3.2" +version = "2.4.0" license = {file = "LICENSE"} requires-python = ">=3.10" dependencies = [] diff --git a/tests/test_nodes.py b/tests/test_nodes.py index 126ccb7..f877f36 100644 --- a/tests/test_nodes.py +++ b/tests/test_nodes.py @@ -204,3 +204,147 @@ class TestExecuteGuards: with caplog.at_level("WARNING"): run_node(image, constraint_mode=MIN) assert not any("Upscaling by" in r.message for r in caplog.records) + + +class TestUpscaleBudget: + """A tiny sliver of an image must not be blown up into a multi-GB tensor.""" + + def test_extreme_ratio_raises_instead_of_exhausting_memory(self): + # 1x690 at the stock defaults previously asked for 704x485760 (~4.1 GB + # per batch item), which OOM-killed the ComfyUI process. + with pytest.raises(ValueError, match="safety limit"): + calc(1, 690, 704, 1280, 2, MIN) + + def test_error_names_the_escape_hatch(self): + with pytest.raises(ValueError, match="Prioritize Max Resolution"): + calc(8, 4000, 704, 1280, 2, MIN) + + def test_strict_mode_has_no_budget_error(self): + """Strict mode is already bounded, so the same input must succeed.""" + w, h = calc(1, 690, 704, 1280, 2, STRICT) + assert w <= 1280 and h <= 1280 + + @pytest.mark.parametrize("size", [(1000, 100), (100, 1000), (3000, 1000), (1920, 1080)]) + def test_reasonable_ratios_still_allowed(self, size): + """Ordinary and moderately wide images must not trip the budget.""" + w, h = calc(*size, 704, 1280, 2, MIN) + assert min(w, h) >= 704 + + def test_budget_scales_with_max_res(self): + """A larger max_res raises the ceiling rather than being a fixed cap.""" + with pytest.raises(ValueError): + calc(1, 400, 704, 1280, 2, MIN) + w, h = calc(1, 400, 704, 8192, 2, MIN) + assert min(w, h) >= 704 + + +class TestDegenerateConfigs: + """Configurations that used to yield a zero dimension and a silent no-op.""" + + def test_multiple_of_larger_than_max_res_raises(self): + # Previously returned (0, 0), which execute() turned into a silent + # passthrough logged as if the *input* had a zero dimension. + with pytest.raises(ValueError, match="larger than max_res"): + calc(512, 512, 8, 200, 256, STRICT) + + def test_thin_strip_does_not_collapse_to_zero_width(self): + # int() truncation drove this to a width of 0 when multiple_of == 1. + w, h = calc(1, 677, 8, 65, 1, STRICT) + assert w >= 1 and h >= 1 + + @pytest.mark.parametrize("mode", [MIN, STRICT]) + def test_multiple_of_zero_raises_not_zero_division(self, mode): + with pytest.raises(ValueError, match="multiple_of"): + calc(500, 333, 704, 1280, 0, mode) + + def test_round_to_multiple_rejects_zero(self): + with pytest.raises(ValueError, match="multiple_of"): + ConstrainResolution.round_to_multiple(100, 0) + + def test_round_to_multiple_never_returns_zero(self): + assert ConstrainResolution.round_to_multiple(0, 1) == 1 + assert ConstrainResolution.round_to_multiple(0, 32) == 32 + + def test_unknown_constraint_mode_raises(self): + # Falling through applied neither the min floor nor the max clamp, + # silently violating both limits. + with pytest.raises(ValueError, match="Unknown constraint_mode"): + calc(1000, 100, 704, 1280, 32, "TypoMode") + + +class TestCropGuards: + def test_crop_larger_than_source_raises(self): + image = torch.rand(1, 50, 50, 3) + with pytest.raises(ValueError, match="cannot grow"): + ConstrainResolution.crop_image(image, 100, 100, "center") + + def test_partial_overflow_also_raises(self): + """Silently returned 50x30 instead of the requested 100x30.""" + image = torch.rand(1, 50, 50, 3) + with pytest.raises(ValueError, match="cannot grow"): + ConstrainResolution.crop_image(image, 100, 30, "center") + + def test_exact_size_is_a_no_op(self): + image = torch.rand(1, 50, 60, 3) + assert ConstrainResolution.crop_image(image, 60, 50, "center") is image + + +class TestSingleResize: + def test_crop_path_resizes_only_once(self, monkeypatch): + """The crop path used to compute a full resize and throw it away.""" + calls = [] + real = ConstrainResolution.resize_image + + def counting(image, w, h, method=None): + calls.append((w, h)) + return real(image, w, h, method) + + monkeypatch.setattr(ConstrainResolution, "resize_image", staticmethod(counting)) + image = torch.rand(1, 1013, 1800, 3) + resized, _, out_w, out_h, _, _ = run_node(image, multiple_of=32) + assert len(calls) == 1, f"expected a single resize, got {calls}" + assert resized.shape == (1, out_h, out_w, 3) + + def test_no_crop_path_resizes_only_once(self, monkeypatch): + calls = [] + real = ConstrainResolution.resize_image + + def counting(image, w, h, method=None): + calls.append((w, h)) + return real(image, w, h, method) + + monkeypatch.setattr(ConstrainResolution, "resize_image", staticmethod(counting)) + image = torch.rand(1, 1000, 1000, 3) + run_node(image, crop_as_required=False, multiple_of=32) + assert len(calls) == 1 + + +class TestValidateInputs: + def test_signature_has_no_kwargs(self): + """**kwargs makes ComfyUI skip its own range and combo validation for + every input on this node (execution.py: validate_has_kwargs).""" + import inspect + spec = inspect.getfullargspec(ConstrainResolution.validate_inputs.__func__) + assert spec.varkw is None + assert spec.varargs is None + assert spec.args == ["cls", "min_res", "max_res", "multiple_of"] + + def test_accepts_valid_inputs(self): + assert ConstrainResolution.validate_inputs(704, 1280, 2) is True + + def test_rejects_max_below_min(self): + assert isinstance(ConstrainResolution.validate_inputs(1280, 704, 2), str) + + def test_rejects_bad_multiple_and_min(self): + assert isinstance(ConstrainResolution.validate_inputs(704, 1280, 0), str) + assert isinstance(ConstrainResolution.validate_inputs(0, 1280, 2), str) + + +class TestExtremeRatioCropping: + def test_very_tall_image_is_cropped_not_distorted(self): + """A ratio that rounds to 0.0 used to skip the crop branch entirely.""" + image = torch.rand(1, 3000, 1, 3) + resized, _, out_w, out_h, _, _ = run_node( + image, multiple_of=32, constraint_mode=STRICT + ) + assert resized.shape == (1, out_h, out_w, 3)