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.)
This commit is contained in:
parent
f748d97702
commit
cbb2eaa729
2 changed files with 82 additions and 4 deletions
|
|
@ -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))]
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue