From cbb2eaa729aae1da6f79d28566fb00262227b3ab Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 29 Jun 2026 10:36:44 +0000 Subject: [PATCH] Studio diffusion (Phase 6) review round 2: img2img source dims + upscale repeats Codex review on the native engine arg builder: - build_sd_cpp_command emitted --width/--height unconditionally, so an img2img/inpaint/edit run that left dims unset forced a 1024x1024 resize/crop of the input. width/height are now Optional (None = unset): an image-conditioned run (init_img or ref_images) with unset dims omits the flags so sd.cpp derives the size from the input image (set_width_and_height_if_unset); a plain txt2img run with unset dims keeps the prior 1024x1024 default; explicit dims are always honored. width/height are read only by the builder, so the type change is local. - build_sd_cpp_upscale_command used a truthiness guard (params.repeats and ...) that silently swallowed repeats=0 into sd-cli's default of one pass, turning an explicit no-op into a real upscale. It now rejects repeats < 1 with ValueError and emits the flag for any explicit value != 1. Tests: img2img unset dims omit width/height (init_img and ref_images), explicit dims emitted, txt2img keeps 1024; upscale rejects repeats=0 and omits the flag at the default. (Two pre-existing binary-discovery tests fail only because a real sd-cli is installed in this dev environment; unrelated to this change.) --- studio/backend/core/inference/sd_cpp_args.py | 26 +++++++-- studio/backend/tests/test_sd_cpp_args.py | 60 ++++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/studio/backend/core/inference/sd_cpp_args.py b/studio/backend/core/inference/sd_cpp_args.py index b2df104b7c..44f57fb989 100644 --- a/studio/backend/core/inference/sd_cpp_args.py +++ b/studio/backend/core/inference/sd_cpp_args.py @@ -87,8 +87,11 @@ class SdCppGenParams: prompt: str negative_prompt: Optional[str] = None - width: int = 1024 - height: int = 1024 + # None = "unset": an image-conditioned run (img2img/inpaint/edit) then lets + # sd.cpp derive the size from the input image instead of forcing a resize; a + # plain txt2img run with unset dims falls back to 1024x1024 (see the builder). + width: Optional[int] = None + height: Optional[int] = None steps: Optional[int] = None cfg_scale: Optional[float] = None guidance: Optional[float] = None @@ -207,7 +210,17 @@ def build_sd_cpp_command( cmd += ["--lora-model-dir", params.lora_dir] if params.lora_apply_mode: cmd += ["--lora-apply-mode", params.lora_apply_mode] - cmd += ["--width", str(int(params.width)), "--height", str(int(params.height))] + # Emit explicit dims when given. For an image-conditioned run (img2img / + # inpaint / edit) that leaves them unset, omit the flags so sd.cpp derives the + # size from the input image (set_width_and_height_if_unset) rather than forcing + # a 1024x1024 resize/crop of the source. A plain txt2img run with unset dims + # keeps the prior 1024 default. + if params.width is not None or params.height is not None: + w = int(params.width) if params.width is not None else 1024 + h = int(params.height) if params.height is not None else 1024 + cmd += ["--width", str(w), "--height", str(h)] + elif not (params.init_img or params.ref_images): + cmd += ["--width", "1024", "--height", "1024"] if params.steps is not None: cmd += ["--steps", str(int(params.steps))] if params.cfg_scale is not None: @@ -251,6 +264,11 @@ def build_sd_cpp_upscale_command( raise ValueError("input_image is required for upscale") if not params.upscale_model: raise ValueError("upscale_model is required for upscale") + # A truthiness guard below would silently swallow repeats=0 and fall back to + # sd-cli's default of one pass, turning an explicit no-op into a real upscale. + # Reject it (and negatives) so the caller's intent isn't quietly changed. + if params.repeats < 1: + raise ValueError("repeats must be >= 1 for upscale") cmd: list[str] = [ binary, "--mode", @@ -260,7 +278,7 @@ def build_sd_cpp_upscale_command( "--upscale-model", params.upscale_model, ] - if params.repeats and params.repeats != 1: + if params.repeats != 1: cmd += ["--upscale-repeats", str(int(params.repeats))] if params.tile_size is not None: cmd += ["--upscale-tile-size", str(int(params.tile_size))] diff --git a/studio/backend/tests/test_sd_cpp_args.py b/studio/backend/tests/test_sd_cpp_args.py index 7e3a024f0e..c33a4a01f9 100644 --- a/studio/backend/tests/test_sd_cpp_args.py +++ b/studio/backend/tests/test_sd_cpp_args.py @@ -221,6 +221,47 @@ def test_build_edit_repeats_ref_image(): assert [cmd[i + 1] for i in idxs] == ["/r/a.png", "/r/b.png"] +def test_img2img_unset_dims_lets_sdcpp_derive_from_source(): + # img2img/inpaint/edit with dims left unset must NOT force --width/--height, + # so sd.cpp derives the size from the input image instead of resizing it to 1024. + files = SdCppModelFiles(diffusion_model = "/m/z.gguf") + cmd = build_sd_cpp_command( + "/bin/sd-cli", + files, + SdCppGenParams(prompt = "x", init_img = "/in/src.png"), + output_path = "/o.png", + ) + assert "--width" not in cmd and "--height" not in cmd + # an edit (ref-image) run derives its size too + cmd2 = build_sd_cpp_command( + "/bin/sd-cli", + files, + SdCppGenParams(prompt = "x", ref_images = ("/r/a.png",)), + output_path = "/o.png", + ) + assert "--width" not in cmd2 and "--height" not in cmd2 + + +def test_img2img_explicit_dims_are_emitted(): + files = SdCppModelFiles(diffusion_model = "/m/z.gguf") + cmd = build_sd_cpp_command( + "/bin/sd-cli", + files, + SdCppGenParams(prompt = "x", init_img = "/in/src.png", width = 768, height = 512), + output_path = "/o.png", + ) + assert _pair(cmd, "--width") == "768" and _pair(cmd, "--height") == "512" + + +def test_txt2img_unset_dims_keep_1024_default(): + # A plain txt2img run with no dims keeps the prior 1024x1024 default. + files = SdCppModelFiles(diffusion_model = "/m/z.gguf") + cmd = build_sd_cpp_command( + "/bin/sd-cli", files, SdCppGenParams(prompt = "x"), output_path = "/o.png" + ) + assert _pair(cmd, "--width") == "1024" and _pair(cmd, "--height") == "1024" + + def test_build_lora_dir_and_apply_mode(): files = SdCppModelFiles(diffusion_model = "/m/z.gguf") params = SdCppGenParams( @@ -261,6 +302,25 @@ def test_build_upscale_command(): assert "--prompt" not in cmd and "--llm" not in cmd +def test_build_upscale_rejects_non_positive_repeats(): + # repeats=0 must not be silently swallowed into sd-cli's default of one pass. + with pytest.raises(ValueError, match = "repeats"): + build_sd_cpp_upscale_command( + "/bin/sd-cli", + SdCppUpscaleParams(input_image = "/i.png", upscale_model = "/m/e.pth", repeats = 0), + output_path = "/o.png", + ) + + +def test_build_upscale_default_repeats_omits_flag(): + cmd = build_sd_cpp_upscale_command( + "/bin/sd-cli", + SdCppUpscaleParams(input_image = "/i.png", upscale_model = "/m/e.pth"), # repeats=1 + output_path = "/o.png", + ) + assert "--upscale-repeats" not in cmd + + def test_build_upscale_requires_input_and_model(): with pytest.raises(ValueError): build_sd_cpp_upscale_command(