Preserve legacy SAM selection after saving
This commit is contained in:
Dr.Lt.Data
+17
-11
@@ -16,6 +16,7 @@ that setup does not extend the supported import scope.
|
||||
| `tests/test_e2e_switches.py` | Create and wire nodes; import prepared graphs and UI-created JSON, reload Chrome, delete connected nodes, reconnect and click Run | Slot identities and typed links, list order, non-default selection and routing, fresh output after reconnecting, output preservation when a Reroute disconnects during loading |
|
||||
| `tests/test_e2e_widget_catalog.py` | Load the UI-created catalog and pan to each node at 100% zoom | Every registered node exactly once; live widget names, order, and values match the fixed baseline under positional and named restoration |
|
||||
| `tests/test_e2e_legacy_sam.py` | Open the legacy SAM dialog from a node and Clipspace, detect twice, save/paste, export and reload | No removed imports, one Clipspace button after reopening/refresh, real detector pixels in saved masks, unchanged RGB, widget names/order/values in both restoration modes |
|
||||
| `tests/test_sam_api.py` | Upload an image and send preparation requests over HTTP, without launching Chrome | Valid requests succeed before/after repeated invalid type/path rejection; rejected requests are not cached as successful |
|
||||
| `tests/e2e_tutorials.py` | Import the original Switch tutorial PNGs, change selection widgets, save and reimport JSON | Every selected branch and final output matches direct core-node image results; both selection modes retain the final preview |
|
||||
| `tests/e2e_regional.py` | Import the regional img2img tutorial and prepared variants, click Run | Exact color masks, output differs from a VAE roundtrip, each regional prompt affects its own region more than distant pixels, browser previews match server images |
|
||||
| `tests/e2e_conditionings.py` | Import conditioning and regional-prompt graphs, click Run, save and reimport through the file input | Combine/Concat results match core ComfyUI nodes; regional prompts retain both items in order; dynamic slots and links survive loading |
|
||||
@@ -130,27 +131,32 @@ python -m pytest tests/test_e2e_legacy_sam.py -v
|
||||
|
||||
Install the model configured by `sam_editor_model` on the isolated server.
|
||||
Detection tests skip when `IMPACT_E2E_SAM_IMAGE` is unset; named-restoration
|
||||
cases also skip on older frontends without that setting. Other native reloads
|
||||
use positional restoration on those frontends. The tests forward
|
||||
cases also skip on older frontends without that setting. Positional save/reload
|
||||
cases still run on those frontends. The tests forward
|
||||
the real detector response without substituting a mask, compare saved outputs
|
||||
and browser previews, and reload native exports in positional and named modes.
|
||||
The second edit starts from the first saved temporary image and checks that two
|
||||
clicks produce exactly one positive and one negative point. The separate menu
|
||||
test checks repeated opening and native loading without running detection.
|
||||
test checks repeated opening without running detection.
|
||||
Detection starts from both an uploaded input and a SaveImage output in a nested
|
||||
subfolder, with both native restoration modes. Event checks cover keyboard
|
||||
subfolder, with both native restoration modes. The output scenario creates a
|
||||
two-image batch through the UI and edits the second image. Saving must preserve
|
||||
the first image's URL and metadata, the selected index and its dropdown display.
|
||||
Native export/import checks are concentrated in these save scenarios.
|
||||
Event checks cover keyboard
|
||||
shortcuts, cancellation without changing the image widget, resizing and opening
|
||||
a different image. Invalid type/path requests are rejected twice, between valid
|
||||
preparation requests, so a rejected request cannot be cached as successful.
|
||||
No Vue MaskEditor extension API is required.
|
||||
a different image. No Vue MaskEditor extension API is required.
|
||||
|
||||
For the frontend 1.42.15 / backend 0.19.3 compatibility comparison, select
|
||||
`-k 'not rejects_invalid'`. Those three API checks target the current backend's
|
||||
path resolver; the older-pair comparison exercises the legacy UI and image flow.
|
||||
Run `python -m pytest tests/test_sam_api.py -v` against the current backend for
|
||||
the three HTTP checks. These share one uploaded image and require the configured
|
||||
SAM model, but do not launch a browser. Invalid type/path requests are rejected
|
||||
twice between valid preparation requests, so rejection cannot be cached as
|
||||
success. The frontend 1.42.15 / backend 0.19.3 comparison runs only
|
||||
`tests/test_e2e_legacy_sam.py`; its older path resolver is outside these API checks.
|
||||
|
||||
`node --experimental-vm-modules --test tests/legacy_sam.test.mjs` checks that
|
||||
resizing redraws only the current image after reopening and that mask upload
|
||||
sets the selected image reference without changing another image. It loads the shipped
|
||||
creates missing image metadata for core preview refresh. It loads the shipped
|
||||
SAM module with controlled UI exports and canvas/event objects, exercising both
|
||||
global UI exports and the dynamic UI-module fallback. This checks those import
|
||||
contracts; it does not substitute for browser testing an older frontend release.
|
||||
|
||||
@@ -6,6 +6,8 @@ const { ComfyDialog, $el } = window.comfyAPI?.ui ?? await import("../../scripts/
|
||||
function addClipspaceSamButton() {
|
||||
if (!ComfyApp.clipspace?.imgs?.length) return;
|
||||
const content = document.getElementById("clipspace_preview")?.closest(".comfy-modal-content");
|
||||
const selector = content?.querySelector("#clipspace_img_selector");
|
||||
if (selector) selector.value = String(ComfyApp.clipspace.selectedIndex);
|
||||
if (!content || content.querySelector("#impact-sam-clipspace")) return;
|
||||
const button = $el("button", {
|
||||
id: "impact-sam-clipspace",
|
||||
|
||||
+7
-3
@@ -37,13 +37,17 @@ and Clipspace button without importing the removed `ClipspaceDialog` module.
|
||||
Set `IMPACT_E2E_SAM_IMAGE` and install the configured SAM model to include real
|
||||
detection, repeated saving/pasting, RGB preservation, and native workflow reload
|
||||
in both widget restoration modes, starting from input and nested output images.
|
||||
The output case edits the second image of a batch and preserves the other image
|
||||
and the Clipspace selection after saving.
|
||||
The suite also checks keyboard shortcuts, cancellation, resizing, changing the
|
||||
image, and repeated invalid-path rejection. Run
|
||||
image. Run `python3 -m pytest tests/test_sam_api.py -v` against the current backend
|
||||
for repeated invalid-path rejection between valid preparation requests. It uses
|
||||
HTTP without launching Chrome and requires the configured SAM model. Run
|
||||
`node --experimental-vm-modules --test tests/legacy_sam.test.mjs` for current-image
|
||||
resizing and saved image references with controlled browser objects, including
|
||||
resizing and missing image metadata with controlled browser objects, including
|
||||
global and fallback UI exports.
|
||||
Older frontends without named widget restoration skip the two named detection
|
||||
cases; their other native reloads use positional restoration.
|
||||
cases; their positional save/reload cases still run.
|
||||
With Playwright and Chrome installed, `python3 -m pytest tests/test_e2e_ui.py`
|
||||
also checks failure cleanup and JavaScript errors during artifact saving using
|
||||
intercepted browser requests, without a server.
|
||||
|
||||
+11
-18
@@ -76,21 +76,14 @@ for (const legacyUI of [false, true]) {
|
||||
});
|
||||
}
|
||||
|
||||
for (const selectedIndex of [0, 1]) {
|
||||
test(`upload retains image references (selected ${selectedIndex}, metadata ${selectedIndex ? 'present' : 'absent'})`, async () => {
|
||||
const { module, ComfyApp } = await loadSam();
|
||||
const original = { filename: 'original.png', subfolder: '', type: 'input' };
|
||||
const otherImage = { src: 'http://localhost/other.png' };
|
||||
ComfyApp.clipspace = { selectedIndex, imgs: [otherImage, otherImage] };
|
||||
if (selectedIndex) ComfyApp.clipspace.images = [original, original];
|
||||
let refreshes = 0;
|
||||
ComfyApp.clipspace_invalidate_handler = () => { refreshes++; };
|
||||
const saved = { filename: 'saved.png', subfolder: '', type: 'temp' };
|
||||
await module.namespace.uploadMask(saved, {});
|
||||
assert.deepEqual(ComfyApp.clipspace.images?.[selectedIndex], saved,
|
||||
'Saved image metadata is required when the core refreshes node previews');
|
||||
assert.equal(ComfyApp.clipspace.imgs[1 - selectedIndex], otherImage);
|
||||
if (selectedIndex) assert.equal(ComfyApp.clipspace.images[0], original);
|
||||
assert.equal(refreshes, 1);
|
||||
});
|
||||
}
|
||||
test('upload creates missing image metadata for core preview refresh', async () => {
|
||||
const { module, ComfyApp } = await loadSam();
|
||||
ComfyApp.clipspace = { selectedIndex: 0, imgs: [{ src: 'http://localhost/original.png' }] };
|
||||
let refreshes = 0;
|
||||
ComfyApp.clipspace_invalidate_handler = () => { refreshes++; };
|
||||
const saved = { filename: 'saved.png', subfolder: '', type: 'temp' };
|
||||
await module.namespace.uploadMask(saved, {});
|
||||
assert.deepEqual(ComfyApp.clipspace.images?.[0], saved,
|
||||
'Saved image metadata is required when the core refreshes node previews');
|
||||
assert.equal(refreshes, 1);
|
||||
});
|
||||
|
||||
@@ -80,6 +80,14 @@ def open_clipspace(browser):
|
||||
return content
|
||||
|
||||
|
||||
def clipspace_state(browser):
|
||||
return browser.page.evaluate("""() => {
|
||||
const state = window.comfyAPI.app.ComfyApp.clipspace;
|
||||
return {selectedIndex: state.selectedIndex, images: state.images,
|
||||
urls: state.imgs.map(image => image.src)};
|
||||
}""")
|
||||
|
||||
|
||||
def widgets(browser):
|
||||
# SAM updates LoadImage. Output preview widgets are recreated only on execution.
|
||||
return browser.app.evaluate("""app => app.graph._nodes.filter(node => node.type === 'LoadImage').map(node => ({
|
||||
@@ -138,8 +146,6 @@ def test_legacy_sam_context_menu_and_clipspace_reopen(ui):
|
||||
expect(browser.page.locator("#pointsCanvas")).to_be_visible()
|
||||
browser.page.get_by_role("button", name="Cancel", exact=True).click()
|
||||
content.get_by_role("button", name="Close", exact=True).click()
|
||||
for named in ((False, True) if supports_named_restore(browser) else (False,)):
|
||||
reload_native(browser, artifacts, named)
|
||||
|
||||
|
||||
def test_legacy_sam_keyboard_cancel_resize_and_change_image(ui):
|
||||
@@ -183,32 +189,6 @@ def test_legacy_sam_keyboard_cancel_resize_and_change_image(ui):
|
||||
assert widgets(browser) == before, "Cancel changed the source image"
|
||||
browser.page.keyboard.press("-")
|
||||
expect(brush).to_have_css("background-color", "rgb(0, 0, 255)")
|
||||
reload_native(browser, artifacts, path == second and supports_named_restore(browser))
|
||||
|
||||
|
||||
@pytest.mark.parametrize("invalid", [
|
||||
{"type": "unknown"},
|
||||
{"filename": "../outside.png"},
|
||||
{"subfolder": "../../outside"},
|
||||
], ids=["unknown-type", "filename-escape", "subfolder-escape"])
|
||||
def test_legacy_sam_rejects_invalid_path_on_retry(ui, invalid):
|
||||
browser, artifacts = ui
|
||||
assert_sam_loaded(browser)
|
||||
path = artifacts / "valid-source.png"
|
||||
Image.new("RGB", (256, 256), (64, 128, 192)).save(path)
|
||||
source = source_image(browser, path)
|
||||
with browser.page.expect_response(lambda response: response.url.endswith("/sam/prepare")) as prepared:
|
||||
node_menu(browser, source, "Open in SAM Detector")
|
||||
assert prepared.value.status == 200, "Valid SAM preparation must succeed before testing rejection"
|
||||
valid = prepared.value.request.post_data_json
|
||||
browser.page.get_by_role("button", name="Cancel", exact=True).click()
|
||||
data = dict(valid)
|
||||
data.update(invalid)
|
||||
for _ in range(2):
|
||||
response = browser.page.request.post(f"{browser.base_url}/sam/prepare", data=data)
|
||||
assert response.status == 400, "Rejected SAM preparation was cached as successful"
|
||||
response = browser.page.request.post(f"{browser.base_url}/sam/prepare", data=valid)
|
||||
assert response.status == 200, "Rejected paths prevented valid preparation"
|
||||
|
||||
|
||||
@pytest.mark.skipif(not os.environ.get("IMPACT_E2E_SAM_IMAGE"),
|
||||
@@ -223,10 +203,20 @@ def test_legacy_sam_detect_save_and_native_reload(ui, named, origin):
|
||||
source = source_image(browser, os.environ["IMPACT_E2E_SAM_IMAGE"])
|
||||
output_type, mask_type = ("PreviewImage", "SaveImage") if origin == "input" else ("SaveImage", "PreviewImage")
|
||||
titles = {"PreviewImage": "Preview Image", "SaveImage": "Save Image"}
|
||||
output = browser.add_node(titles[output_type], output_type, 750, 200)
|
||||
output = browser.add_node(titles[output_type], output_type,
|
||||
1350 if origin == "output_subfolder" else 750, 200)
|
||||
convert = browser.add_node("Convert Mask to Image", "MaskToImage", 220, 800)
|
||||
mask_output = browser.add_node(titles[mask_type], mask_type, 750, 800)
|
||||
browser.connect("LoadImage", output_type)
|
||||
if origin == "output_subfolder":
|
||||
definition = browser.page.request.get(f"{browser.base_url}/object_info/RepeatImageBatch")
|
||||
assert definition.ok
|
||||
title = definition.json()["RepeatImageBatch"]["display_name"] or "RepeatImageBatch"
|
||||
browser.add_node(title, "RepeatImageBatch", 750, 200)
|
||||
browser.set_number("RepeatImageBatch", "amount", 2)
|
||||
browser.connect("LoadImage", "RepeatImageBatch")
|
||||
browser.connect("RepeatImageBatch", output_type)
|
||||
else:
|
||||
browser.connect("LoadImage", output_type)
|
||||
browser.page.mouse.move(*browser.point("LoadImage", slot=1))
|
||||
browser.page.mouse.down()
|
||||
browser.page.mouse.move(*browser.point("MaskToImage", is_input=True), steps=12)
|
||||
@@ -257,10 +247,17 @@ def test_legacy_sam_detect_save_and_native_reload(ui, named, origin):
|
||||
else:
|
||||
node_menu(browser, output if index == 0 else source, "Copy (Clipspace)")
|
||||
content = open_clipspace(browser)
|
||||
if index == 0:
|
||||
content.locator("#clipspace_img_selector").select_option("1")
|
||||
before = clipspace_state(browser)
|
||||
if index == 0:
|
||||
assert len(before["urls"]) == 2
|
||||
assert before["selectedIndex"] == 1
|
||||
content.get_by_role("button", name="Impact SAM Detector", exact=True).click()
|
||||
if index == 0 and origin == "output_subfolder":
|
||||
assert prepared.value.post_data_json["type"] == "output"
|
||||
assert prepared.value.post_data_json["subfolder"] == "impact-legacy-sam/nested"
|
||||
assert prepared.value.post_data_json["filename"] == before["images"][1]["filename"]
|
||||
canvas = browser.page.locator("#pointsCanvas")
|
||||
expect(canvas).to_be_visible()
|
||||
save = browser.page.get_by_role("button", name="Save to node" if direct else "Save", exact=True)
|
||||
@@ -290,8 +287,20 @@ def test_legacy_sam_detect_save_and_native_reload(ui, named, origin):
|
||||
browser.page.wait_for_function("""() => {
|
||||
const image = document.getElementById('clipspace_preview');
|
||||
return image.complete && image.naturalWidth > 0 &&
|
||||
image.src === window.comfyAPI.app.ComfyApp.clipspace.imgs[0].src;
|
||||
image.src === window.comfyAPI.app.ComfyApp.clipspace.imgs[
|
||||
window.comfyAPI.app.ComfyApp.clipspace.selectedIndex].src;
|
||||
}""")
|
||||
after = clipspace_state(browser)
|
||||
selected = before["selectedIndex"]
|
||||
assert after["selectedIndex"] == selected
|
||||
expect(content.locator("#clipspace_img_selector")).to_have_value(str(selected))
|
||||
assert len(after["urls"]) == len(before["urls"])
|
||||
assert after["urls"][selected] != before["urls"][selected]
|
||||
assert after["images"][selected]["type"] == "temp"
|
||||
for other in range(len(before["urls"])):
|
||||
if other != selected:
|
||||
assert after["urls"][other] == before["urls"][other]
|
||||
assert after["images"][other] == before["images"][other], "Save changed another image reference"
|
||||
expect(content.get_by_role("button", name="Impact SAM Detector", exact=True)).to_have_count(1)
|
||||
content.get_by_role("button", name="Close", exact=True).click()
|
||||
node_menu(browser, source, "Paste (Clipspace)")
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
"""SAM preparation HTTP contract; requires the current backend and configured model."""
|
||||
import io
|
||||
import os
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from PIL import Image
|
||||
from playwright.sync_api import sync_playwright
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def sam_api():
|
||||
with sync_playwright() as playwright:
|
||||
api = playwright.request.new_context(
|
||||
base_url=os.environ.get("IMPACT_E2E_BASE_URL", "http://127.0.0.1:18188"), timeout=180000)
|
||||
try:
|
||||
image = io.BytesIO()
|
||||
Image.new("RGB", (256, 256), (64, 128, 192)).save(image, format="PNG")
|
||||
response = api.post("/upload/image", multipart={
|
||||
"image": {"name": f"sam-prepare-{uuid4().hex}.png", "mimeType": "image/png", "buffer": image.getvalue()},
|
||||
"type": "input",
|
||||
})
|
||||
assert response.ok
|
||||
uploaded = response.json()
|
||||
yield api, {"sam_model_name": "auto", "filename": uploaded["name"],
|
||||
"type": uploaded["type"], "subfolder": uploaded["subfolder"]}
|
||||
finally:
|
||||
api.dispose()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("invalid", [
|
||||
{"type": "unknown"},
|
||||
{"filename": "../outside.png"},
|
||||
{"subfolder": "../../outside"},
|
||||
], ids=["unknown-type", "filename-escape", "subfolder-escape"])
|
||||
def test_sam_rejects_invalid_path_on_retry(sam_api, invalid):
|
||||
api, valid = sam_api
|
||||
assert api.post("/sam/prepare", data=valid).status == 200, "Valid SAM preparation must succeed before testing rejection"
|
||||
for _ in range(2):
|
||||
response = api.post("/sam/prepare", data={**valid, **invalid})
|
||||
assert response.status == 400, "Rejected SAM preparation was cached as successful"
|
||||
assert api.post("/sam/prepare", data=valid).status == 200, "Rejected paths prevented valid preparation"
|
||||
Reference in New Issue
Block a user