Fix legacy SAM save and input handling

This commit is contained in:
Dr.Lt.Data
2026-11-01 03:22:13 +09:00
parent 011776c293
commit fe338c5b09
8 changed files with 254 additions and 34 deletions
+19 -1
View File
@@ -129,14 +129,32 @@ python -m pytest tests/test_e2e_legacy_sam.py -v
```
Install the model configured by `sam_editor_model` on the isolated server.
The detection tests skip only when `IMPACT_E2E_SAM_IMAGE` is unset. They forward
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
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.
Detection starts from both an uploaded input and a SaveImage output in a nested
subfolder, with both native restoration modes. 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.
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.
`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
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.
The all-node catalog requires **Full Cache mode** because the trusted snapshot
includes wildcard picker values displaying `Select Wildcard 🟢 Full Cache`.
In the test copy's `impact-pack.ini`, set `wildcard_cache_limit_mb` above the
+4 -6
View File
@@ -65,8 +65,8 @@ async function uploadMask(filepath, formData) {
ComfyApp.clipspace.imgs[ComfyApp.clipspace['selectedIndex']] = new Image();
ComfyApp.clipspace.imgs[ComfyApp.clipspace['selectedIndex']].src = `view?filename=${filepath.filename}&type=${filepath.type}`;
if(ComfyApp.clipspace.images)
ComfyApp.clipspace.images[ComfyApp.clipspace['selectedIndex']] = filepath;
ComfyApp.clipspace.images ??= [];
ComfyApp.clipspace.images[ComfyApp.clipspace['selectedIndex']] = filepath;
ComfyApp.clipspace_invalidate_handler?.();
}
@@ -421,6 +421,7 @@ class ImpactSamEditorDialog extends ComfyDialog {
static handleKeyDown(event) {
const self = ImpactSamEditorDialog.instance;
const brush = self.brush;
if (event.key === '=') { // positive
brush.style.backgroundColor = "blue";
brush.style.outline = "2px solid pink";
@@ -588,14 +589,11 @@ class ImpactSamEditorDialog extends ComfyDialog {
"type": "temp",
};
if(ComfyApp.clipspace.images)
ComfyApp.clipspace.images[0] = item;
if(ComfyApp.clipspace.widgets) {
const index = ComfyApp.clipspace.widgets.findIndex(obj => obj.name === 'image');
if(index >= 0)
ComfyApp.clipspace.widgets[index].value = `${filename} [temp]`;
ComfyApp.clipspace.widgets[index].value = item;
}
const dataURL = save_canvas.toDataURL();
+1 -2
View File
@@ -67,8 +67,6 @@ async def sam_prepare(request):
# already loaded: skip -- prevent redundant loading
return web.Response(status=200)
last_prepare_data = data
model_name = 'sam_vit_b_01ec64.pth'
if data['sam_model_name'] == 'auto':
model_name = impact.config.get_config()['sam_editor_model']
@@ -95,6 +93,7 @@ async def sam_prepare(request):
except ValueError:
return web.Response(status=400)
last_prepare_data = data
thread = threading.Thread(target=async_prepare_sam, args=(model_path, filename,))
thread.start()
+8 -1
View File
@@ -36,7 +36,14 @@ a running test server and are separate from the curl-based API integration tests
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.
in both widget restoration modes, starting from input and nested output images.
The suite also checks keyboard shortcuts, cancellation, resizing, changing the
image, and repeated invalid-path rejection. Run
`node --experimental-vm-modules --test tests/legacy_sam.test.mjs` for current-image
resizing and saved image references 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.
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.
+2 -1
View File
@@ -46,7 +46,8 @@ class ComfyUI:
self.page.mouse.dblclick(x, y, delay=100)
search = self.page.get_by_placeholder("Add a node...", exact=True)
search.fill(node_type)
label = self.page.locator("span.truncate").filter(has_text=re.compile(rf"^\s*{re.escape(title)}\s*$"))
label = self.page.locator("span.truncate, .option-container .font-semibold > span").filter(
has_text=re.compile(rf"^\s*{re.escape(title)}\s*$"))
self.page.get_by_role("option").filter(has=label).click()
expect(search).not_to_be_visible()
self.page.get_by_role("dialog").wait_for(state="hidden")
+96
View File
@@ -0,0 +1,96 @@
import assert from 'node:assert/strict';
import { readFileSync } from 'node:fs';
import test from 'node:test';
import vm from 'node:vm';
const source = readFileSync(new URL('../js/impact-sam-editor.js', import.meta.url), 'utf8');
async function loadSam(legacyUI = false) {
const window = new EventTarget();
Object.assign(window, { innerWidth: 1600, innerHeight: 1000 });
const ui = { ComfyDialog: class {}, $el: () => {} };
if (!legacyUI) window.comfyAPI = { ui };
const registered = [];
const context = vm.createContext({
window, Event, URL,
Image: class {
set src(value) {
this.url = String(value);
[this.width, this.height] = this.url.includes('portrait') ? [700, 1300] : [1400, 800];
}
get src() { return this.url; }
},
});
const ComfyApp = {};
const imports = {
'../../scripts/app.js': { app: { registerExtension: extension => registered.push(extension) }, ComfyApp },
'../../scripts/api.js': { api: { fetchApi: async () => ({ ok: true }) } },
'../../scripts/ui.js': ui,
};
const requests = [];
async function load(specifier) {
requests.push(specifier);
assert.ok(imports[specifier], `Unexpected import: ${specifier}`);
const values = imports[specifier];
const module = new vm.SyntheticModule(Object.keys(values), function () {
for (const [key, value] of Object.entries(values)) this.setExport(key, value);
}, { context });
await module.link(load);
await module.evaluate();
return module;
}
const module = new vm.SourceTextModule(`${source}\nexport { ImpactSamEditorDialog, uploadMask };`, {
context, importModuleDynamically: load,
});
await module.link(load);
await module.evaluate();
assert.deepEqual(registered.map(extension => extension.name), ['Comfy.Impact.SAMEditor']);
assert.equal(requests.includes('../../scripts/ui.js'), legacyUI);
return { module, window, ComfyApp };
}
for (const legacyUI of [false, true]) {
test(`resize redraws only the current image (${legacyUI ? 'UI module fallback' : 'global UI exports'})`, async () => {
const { module, window } = await loadSam(legacyUI);
const draws = [];
const imageCanvas = {
width: 0, height: 0, offsetTop: 0, offsetLeft: 0,
get clientWidth() { return this.width; },
getContext: () => ({ drawImage: image => draws.push(image.src) }),
};
const dialog = Object.create(module.namespace.ImpactSamEditorDialog.prototype);
Object.assign(dialog, {
maskCanvas: { style: {} }, maskCtx: {},
pointsCanvas: { style: {} }, pointsCtx: { canvas: {} },
prompt_points: [], saveButton: {},
});
for (const name of ['landscape', 'portrait']) {
dialog.setImages(`http://localhost/${name}.png`, imageCanvas, dialog.pointsCanvas);
dialog.image.onload();
}
draws.length = 0;
window.innerWidth = 1100;
window.dispatchEvent(new Event('resize'));
assert.equal(draws.length, 1, 'Resize redrew images from previous editing sessions');
assert.match(draws[0], /portrait\.png/);
});
}
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);
});
}
+122 -21
View File
@@ -45,21 +45,28 @@ def node_menu(browser, node_id, name):
def source_image(browser, path):
source = browser.add_node("Load Image", "LoadImage", 220, 200)
upload_image(browser, source, path)
return source
def upload_image(browser, source, path):
button = browser.app.evaluate("""(app, id) =>
app.graph.getNodeById(id).widgets.find(widget => widget.type === 'button').name
""", source)
with browser.page.expect_file_chooser() as chooser:
browser.page.mouse.click(*browser.point("LoadImage", widget=button))
chooser.value.set_files(path)
browser.page.wait_for_function("""(app) => {
with browser.page.expect_response(lambda response: response.url.endswith("/upload/image")) as uploaded:
chooser.value.set_files(path)
assert uploaded.value.ok
filename = uploaded.value.json()["name"]
browser.page.wait_for_function("""({app, filename}) => {
const image = app.graph._nodes.find(node => node.type === 'LoadImage').imgs?.[0];
return image?.complete && image.naturalWidth > 0;
}""", arg=browser.app)
return source
return image?.complete && image.naturalWidth > 0 && new URL(image.src).searchParams.get('filename') === filename;
}""", arg={"app": browser.app, "filename": filename})
def clipspace_menu(browser):
browser.page.get_by_test_id("comfy-menu-button").click()
browser.page.locator('[data-testid="comfy-menu-button"], .comfy-menu-button-wrapper').click()
browser.page.get_by_role("menuitem", name="Edit", exact=True).hover()
browser.page.get_by_role("menuitem", name="Clipspace", exact=True).click()
@@ -81,8 +88,15 @@ def widgets(browser):
}))""")
def supports_named_restore(browser):
return browser.app.evaluate("app => 'Comfy.Workflow.NamedValuesRestore' in app.ui.settings.settingsLookup")
def reload_native(browser, artifacts, named):
setting(browser, "Restore widget values", "Comfy.Workflow.NamedValuesRestore", named)
if supports_named_restore(browser):
setting(browser, "Restore widget values", "Comfy.Workflow.NamedValuesRestore", named)
else:
assert not named, "This frontend only provides positional widget restoration"
before = widgets(browser)
path = artifacts / "edited-workflow.json"
export_workflow(browser, path)
@@ -105,6 +119,9 @@ def test_legacy_sam_context_menu_and_clipspace_reopen(ui):
expect(empty_dialog).to_be_visible()
expect(browser.page.get_by_role("button", name="Impact SAM Detector", exact=True)).not_to_be_visible()
empty_dialog.get_by_role("button", name="Close", exact=True).click()
expect(empty_dialog).not_to_be_visible()
browser.page.mouse.click(600, 400)
expect(browser.page.get_by_role("menuitem", name="Edit", exact=True)).not_to_be_visible()
path = artifacts / "source.png"
Image.new("RGB", (256, 256), (64, 128, 192)).save(path)
source = source_image(browser, path)
@@ -121,28 +138,107 @@ 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):
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):
browser, artifacts = ui
assert_sam_loaded(browser)
first = artifacts / "landscape.png"
second = artifacts / "portrait.png"
Image.new("RGB", (1400, 800), (64, 128, 192)).save(first)
Image.new("RGB", (700, 1300), (192, 64, 128)).save(second)
source = source_image(browser, first)
for path, size, color in [(first, (1400, 800), [64, 128, 192]),
(second, (700, 1300), [192, 64, 128])]:
if path == second:
upload_image(browser, source, path)
before = widgets(browser)
for _ in range(2):
node_menu(browser, source, "Open in SAM Detector")
canvas = browser.page.locator("#pointsCanvas")
expect(canvas).to_be_visible()
expect(browser.page.get_by_role("button", name="Save to node", exact=True)).to_be_enabled()
assert canvas.evaluate("canvas => [...canvas.getContext('2d').getImageData(0, 0, canvas.width, canvas.height).data].every(value => value === 0)"), "Reopening retained prompt points"
brush = browser.page.locator("#sam-brush")
browser.page.keyboard.press("-")
expect(brush).to_have_css("background-color", "rgb(255, 0, 0)")
browser.page.keyboard.press("=")
expect(brush).to_have_css("background-color", "rgb(0, 0, 255)")
for width, height in [(1100, 900), (2200, 1300)]:
browser.page.set_viewport_size({"width": width, "height": height})
browser.page.wait_for_function("""({width, height}) => {
const image = document.getElementById('imageCanvas');
return image.width === width - 250 && image.height === height - 200;
}""", arg={"width": width, "height": height})
box = canvas.bounding_box()
assert box["width"] / box["height"] == pytest.approx(size[0] / size[1], abs=0.005)
pixel = browser.page.locator("#imageCanvas").evaluate("canvas => [...canvas.getContext('2d').getImageData(10, 10, 1, 1).data].slice(0, 3)")
assert pixel == color, "Resize drew a previously edited image"
canvas.click(position={"x": 60, "y": 60})
assert canvas.evaluate("canvas => canvas.getContext('2d').getImageData(60, 60, 1, 1).data[3]") > 0
browser.page.get_by_role("button", name="Cancel", exact=True).click()
expect(canvas).not_to_be_visible()
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"),
reason="Set IMPACT_E2E_SAM_IMAGE and install the configured SAM model")
@pytest.mark.parametrize("named", [False, True], ids=["positional", "named"])
def test_legacy_sam_detect_save_and_native_reload(ui, named):
@pytest.mark.parametrize("origin", ["input", "output_subfolder"])
def test_legacy_sam_detect_save_and_native_reload(ui, named, origin):
browser, artifacts = ui
assert_sam_loaded(browser)
if named and not supports_named_restore(browser):
pytest.skip("This frontend does not provide named widget restoration")
source = source_image(browser, os.environ["IMPACT_E2E_SAM_IMAGE"])
output = browser.add_node("Preview Image", "PreviewImage", 750, 200)
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)
convert = browser.add_node("Convert Mask to Image", "MaskToImage", 220, 800)
mask_output = browser.add_node("Save Image", "SaveImage", 750, 800)
browser.connect("LoadImage", "PreviewImage")
mask_output = browser.add_node(titles[mask_type], mask_type, 750, 800)
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)
browser.page.mouse.up()
browser.page.wait_for_function("""(app) => app.graph._nodes.find(node =>
node.type === 'MaskToImage').inputs[0].link !== null""", arg=browser.app)
browser.connect("MaskToImage", "SaveImage")
browser.connect("MaskToImage", mask_type)
if origin == "output_subfolder":
browser.page.mouse.dblclick(*browser.point("SaveImage", widget="filename_prefix"), delay=100)
editor = browser.page.locator("input.value:visible, textarea.value:visible")
editor.fill("impact-legacy-sam/nested/source")
editor.press("Enter")
original = browser.images(browser.run())[output][0]
responses = []
@@ -154,15 +250,20 @@ def test_legacy_sam_detect_save_and_native_reload(ui, named):
browser.page.route("**/sam/detect", capture)
previous = None
for index in range(2):
if index == 0:
node_menu(browser, source, "Open in SAM Detector")
else:
node_menu(browser, source, "Copy (Clipspace)")
content = open_clipspace(browser)
content.get_by_role("button", name="Impact SAM Detector", exact=True).click()
direct = index == 0 and origin == "input"
with browser.page.expect_request(lambda request: request.url.endswith("/sam/prepare")) as prepared:
if direct:
node_menu(browser, source, "Open in SAM Detector")
else:
node_menu(browser, output if index == 0 else source, "Copy (Clipspace)")
content = open_clipspace(browser)
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"
canvas = browser.page.locator("#pointsCanvas")
expect(canvas).to_be_visible()
save = browser.page.get_by_role("button", name="Save to node" if index == 0 else "Save", exact=True)
save = browser.page.get_by_role("button", name="Save to node" if direct else "Save", exact=True)
expect(save).to_be_enabled()
box = canvas.bounding_box()
browser.page.mouse.click(box["x"] + box["width"] * 0.5, box["y"] + box["height"] * (0.5 if index == 0 else 0.25))
@@ -185,7 +286,7 @@ def test_legacy_sam_detect_save_and_native_reload(ui, named):
save.click()
assert uploaded.value.ok
expect(canvas).not_to_be_visible()
if index:
if not direct:
browser.page.wait_for_function("""() => {
const image = document.getElementById('clipspace_preview');
return image.complete && image.naturalWidth > 0 &&
+2 -2
View File
@@ -112,8 +112,8 @@ def widget_state(ui, node_id):
def export_workflow(ui, path):
page = ui.page
page.get_by_role("button", name="Graph mode, workflow actions", exact=True).click()
page.get_by_role("menuitem", name="Export", exact=True).click()
page.locator('button[aria-label$="workflow actions" i]').click()
page.get_by_text("Export", exact=True).filter(visible=True).click()
dialog = page.get_by_role("dialog")
dialog.get_by_role("textbox").fill(path.stem)
with page.expect_download() as download: