diff --git a/MASK_NORMALIZATION.md b/MASK_NORMALIZATION.md index d440b37..9660230 100644 --- a/MASK_NORMALIZATION.md +++ b/MASK_NORMALIZATION.md @@ -1,57 +1,18 @@ -# Mask Normalization Implementation (Issue #106) +# Inpainting Mask Contract (#106) -## Berry Mask Convention +Berry masks encode edit coverage in alpha: opaque painted pixels are edited; transparent pixels are protected. Partial alpha preserves brush-edge coverage. Imported masks must have alpha; arbitrary grayscale uploads are not silently reinterpreted. -**Canonical Semantic**: Painted regions (opaque white, alpha=255) indicate areas TO BE EDITED by AI. Unpainted regions (transparent, alpha=0) indicate areas TO BE PROTECTED. +| Adapter | Submitted mask | Painted region | Protected region | +| --- | --- | --- | --- | +| ComfyUI LoadImage | RGBA PNG, inverse alpha | Alpha 0 -> MASK 1 | Alpha 255 -> MASK 0 | +| WebUI | Grayscale PNG from Berry alpha | White 255 | Black 0 | +| OpenAI edits | RGBA PNG, inverse alpha | Transparent | Opaque | +| Fal flux-general/inpainting | Grayscale PNG from Berry alpha | White 255 | Black 0 | -This convention is documented in: -- `backend/app/runners/mask_converter.py` module docstring -- Test suite in `backend/tests/test_mask_converter.py` +Fal uses an explicit grayscale mask rather than relying on transparency being interpreted consistently. Its documented [inpainting API](https://fal.ai/models/fal-ai/flux-general/inpainting/api) supplies a separate image and mask. [OpenAI mask guidance](https://developers.openai.com/api/docs/guides/image-generation) specifies transparent edit regions. ComfyUI LoadImage's mask remains inverse alpha. -## Provider-Specific Conversions +Every creative inpaint submission validates mask dimensions against the actual source before cache lookup or inference. Mismatches fail with guidance rather than stretching or cropping. Conversions preserve exact dimensions and coordinates. Temporary Comfy masks have unique filenames and are removed after upload. CPU mask conversion runs in a worker thread on creative cloud/Comfy paths. Cache runner version 0.4.0 invalidates older semantics. -### ComfyUI -- **Contract**: LoadImage MASK output = `1 - alpha` -- **Issue**: Painted regions (alpha=255) become 0 (protect), transparent (alpha=0) becomes 1 (edit) -- **Solution**: Invert alpha channel before upload -- **Implementation**: `normalize_mask_for_comfyui()` creates temporary inverted mask +Validation: 16 converter and cloud adapter payload tests pass. An 8x6 source with one painted pixel asserts the exact Fal/OpenAI payload meaning and unchanged coordinate alignment; mismatched dimensions prevent dispatch. Existing creative/upload/cancellation tests also pass (28 tests). -### WebUI -- **Contract**: Grayscale where white=edit, black=protect -- **Match**: Semantically matches Berry convention -- **Solution**: Convert RGBA alpha channel to grayscale L mode -- **Implementation**: `normalize_mask_for_webui()` extracts alpha to grayscale - -### OpenAI -- **Contract**: Alpha channel where transparent=edit, opaque=protect -- **Issue**: Inverted from Berry convention -- **Solution**: Invert alpha channel -- **Implementation**: `normalize_mask_for_openai()` inverts alpha - -### Fal.ai -- **Contract**: Alpha channel where opaque=edit, transparent=protect -- **Match**: Exactly matches Berry convention -- **Solution**: Validate format only, no conversion needed -- **Implementation**: `normalize_mask_for_fal_ai()` validates image - -## Integration Points - -1. **ComfyUI**: `creative_runner._run_comfy()` converts mask before upload, cleans up temporary file -2. **WebUI**: `webui_runner._run_inpaint()` converts mask before base64 encoding -3. **Cloud**: `creative_runner._run_cloud()` converts based on provider_id -4. **Validation**: All paths validate source/mask dimension alignment - -## Cache Invalidation - -Runner version bumped from `0.1.0` to `0.2.0` in `compute_creative_cache_hash()` to prevent reuse of results generated with incorrect mask semantics. - -## Test Coverage - -13 mask converter tests verify: -- Berry convention documentation -- Provider-specific conversions (ComfyUI, WebUI, OpenAI, Fal.ai) -- Dimension validation and preservation -- Edge cases: partial alpha, fully painted, fully transparent -- File handle management - -All tests passing. +These are deterministic adapter tests. Live engine/provider output preservation remains unverified; #106 stays open until a real known-region run records source, mask, output, engine/model/provider versions, and protected-area comparison. No paid inference was performed. diff --git a/backend/app/runners/creative_runner.py b/backend/app/runners/creative_runner.py index 726972b..8dbbcd2 100644 --- a/backend/app/runners/creative_runner.py +++ b/backend/app/runners/creative_runner.py @@ -205,7 +205,7 @@ def compute_creative_cache_hash( "model": req.model, "connection_id": connection_id, # Isolate by connection "provider_id": effective_provider, - "runner_version": "0.3.0", # Bumped from 0.2.0 for Issue #127 connection routing + "runner_version": "0.4.0", # Explicit grayscale cloud masks and dimension validation "width": req.width, "height": req.height, "steps": req.steps, @@ -449,6 +449,14 @@ async def execute(self, req: CreativeActionRequest) -> CreativeActionResult: height=req.height, ) + if req.action == CreativeActionType.INPAINT: + try: + if not source_width or not source_height: + raise ValueError("Source image dimensions could not be decoded.") + await asyncio.to_thread(validate_mask_dimensions, mask_file_path, source_width, source_height) + except (OSError, ValueError) as error: + return CreativeActionResult(success=False, task_id=task_id, error_message=str(error)) + try: # 1. Resolve unified execution plan before cache check & dispatch plan = resolve_execution_plan(req) @@ -664,7 +672,7 @@ async def _run_comfy( source_img = Image.open(input_file) validate_mask_dimensions(mask_file, source_img.width, source_img.height) - converted_mask_path = normalize_mask_for_comfyui(mask_file) + converted_mask_path = await asyncio.to_thread(normalize_mask_for_comfyui, mask_file) mask_result = await comfy_client.upload_mask(str(converted_mask_path), subfolder="berry_assets") uploaded_mask_name = mask_result["name"] if mask_result.get("subfolder"): @@ -826,14 +834,14 @@ async def _run_cloud( if not key: raise RuntimeError("OpenAI API key missing. Configure it in Cloud Providers (BYOK).") # Convert mask for OpenAI (Issue #106): transparent = edit - converted_mask_bytes = normalize_mask_for_openai(mask_bytes) + converted_mask_bytes = await asyncio.to_thread(normalize_mask_for_openai, mask_bytes) remote_url = await _call_openai_inpaint(req.prompt, image_bytes, converted_mask_bytes, key) elif plan.provider_id == "fal_ai": key = credentials_manager.get_key(CloudProviderId.FAL) if not key: raise RuntimeError("Fal.ai API key is required for cloud inpainting. Configure it in Cloud Settings (BYOK).") - # Convert mask for Fal.ai (Issue #106): opaque = edit (matches Berry, validate only) - converted_mask_bytes = normalize_mask_for_fal_ai(mask_bytes) + # flux-general/inpainting consumes grayscale white=edit, black=protect. + converted_mask_bytes = await asyncio.to_thread(normalize_mask_for_fal_ai, mask_bytes) converted_mask_b64 = base64.b64encode(converted_mask_bytes).decode("utf-8") remote_url = await _call_fal_ai_action( action="inpaint", diff --git a/backend/app/runners/mask_converter.py b/backend/app/runners/mask_converter.py index 47cbc87..1e7b86c 100644 --- a/backend/app/runners/mask_converter.py +++ b/backend/app/runners/mask_converter.py @@ -1,210 +1,69 @@ -""" -Mask Conversion and Normalization for Inpainting (Issue #106). - -Canonical Berry Mask Convention: -- Painted (opaque white, alpha=255) regions indicate areas TO BE EDITED by AI -- Unpainted (transparent, alpha=0) regions indicate areas TO BE PROTECTED (kept unchanged) +"""Normalize inpainting masks without changing their dimensions or coordinates. -This module converts Berry masks to provider-specific formats: -- ComfyUI: LoadImage MASK output computes 1-alpha, requiring inversion -- WebUI: Expects grayscale where white=edit, black=protect (matches Berry) -- OpenAI: Uses alpha channel where transparent=edit (inverted from Berry) -- Fal.ai: Uses alpha channel where opaque=edit (matches Berry) - -All conversions preserve source/mask dimensions and coordinate alignment. +Berry uses alpha as edit coverage: painted/opaque = edit, transparent = protect. +ComfyUI LoadImage and OpenAI use inverse alpha. WebUI and the supported Fal +flux-general/inpainting endpoint use grayscale white = edit, black = protect. """ from io import BytesIO from pathlib import Path -from typing import Optional, Tuple +from tempfile import NamedTemporaryFile -from PIL import Image +from PIL import Image, ImageOps -def normalize_mask_for_comfyui(mask_path: Path) -> Path: - """ - Convert Berry mask to ComfyUI-compatible format. - - ComfyUI's LoadImage MASK output = 1 - alpha, so painted regions (alpha=255) - become 0 (protect) instead of 1 (edit). We must invert the alpha channel. - - Args: - mask_path: Path to Berry mask (white painted = edit, transparent = protect) +def _mask_image(mask_bytes: bytes, inverse_alpha: bool) -> bytes: + """Encode edit coverage explicitly, preserving antialiased edges.""" + try: + with Image.open(BytesIO(mask_bytes)) as source: + if "A" not in source.getbands() and "transparency" not in source.info: + raise ValueError("Berry masks must include an alpha channel") + rgba = source.convert("RGBA") + alpha = rgba.getchannel("A") + if inverse_alpha: + result = Image.new("RGBA", rgba.size, "white") + result.putalpha(ImageOps.invert(alpha)) + else: + result = alpha + output = BytesIO() + result.save(output, "PNG") + return output.getvalue() + except (OSError, ValueError) as error: + raise ValueError(f"Invalid Berry mask: {error}") from error - Returns: - Path to converted mask suitable for ComfyUI LoadImage MASK channel - Raises: - FileNotFoundError: If mask_path doesn't exist - ValueError: If image cannot be processed - """ +def normalize_mask_for_comfyui(mask_path: Path) -> Path: + """Create a unique PNG with inverted alpha for ComfyUI's 1-alpha mask.""" if not mask_path.is_file(): raise FileNotFoundError(f"Mask file not found: {mask_path}") - - try: - img = Image.open(mask_path).convert("RGBA") - width, height = img.size - pixels = img.load() - - # Invert alpha channel: painted (255) -> 0, transparent (0) -> 255 - for y in range(height): - for x in range(width): - r, g, b, a = pixels[x, y] - pixels[x, y] = (255, 255, 255, 255 - a) - - # Save as temporary converted mask - converted_path = mask_path.with_name(f"{mask_path.stem}_comfy{mask_path.suffix}") - img.save(converted_path, "PNG") - return converted_path - - except Exception as e: - raise ValueError(f"Failed to convert mask for ComfyUI: {e}") from e + converted = _mask_image(mask_path.read_bytes(), inverse_alpha=True) + with NamedTemporaryFile(dir=mask_path.parent, prefix="berry-mask-", suffix=".png", delete=False) as output: + output.write(converted) + return Path(output.name) def normalize_mask_for_webui(mask_bytes: bytes) -> bytes: - """ - Convert Berry mask to WebUI-compatible grayscale format. - - WebUI expects grayscale where: - - White (255) = edit region - - Black (0) = protect region - - Berry convention already matches this (painted white = edit), so we just - convert alpha to grayscale: alpha channel -> single grayscale channel. - - Args: - mask_bytes: Berry mask PNG bytes - - Returns: - Converted grayscale mask PNG bytes - - Raises: - ValueError: If image cannot be processed - """ - try: - img = Image.open(BytesIO(mask_bytes)).convert("RGBA") - width, height = img.size - - # Create grayscale image where painted regions are white - grayscale = Image.new("L", (width, height), 0) - pixels_src = img.load() - pixels_dst = grayscale.load() - - for y in range(height): - for x in range(width): - _, _, _, a = pixels_src[x, y] - # Alpha 255 (painted) -> white 255 (edit) - # Alpha 0 (transparent) -> black 0 (protect) - pixels_dst[x, y] = a - - output = BytesIO() - grayscale.save(output, "PNG") - return output.getvalue() - - except Exception as e: - raise ValueError(f"Failed to convert mask for WebUI: {e}") from e + """Encode alpha coverage as grayscale white=edit, black=protect.""" + return _mask_image(mask_bytes, inverse_alpha=False) def normalize_mask_for_openai(mask_bytes: bytes) -> bytes: - """ - Convert Berry mask to OpenAI-compatible format. - - OpenAI expects alpha channel where: - - Transparent (alpha=0) = edit region - - Opaque (alpha=255) = protect region - - This is inverted from Berry convention, so we invert the alpha channel. - - Args: - mask_bytes: Berry mask PNG bytes - - Returns: - Converted mask PNG bytes with inverted alpha - - Raises: - ValueError: If image cannot be processed - """ - try: - img = Image.open(BytesIO(mask_bytes)).convert("RGBA") - width, height = img.size - pixels = img.load() - - # Invert alpha: painted (255) -> 0 (edit), transparent (0) -> 255 (protect) - for y in range(height): - for x in range(width): - r, g, b, a = pixels[x, y] - pixels[x, y] = (r, g, b, 255 - a) - - output = BytesIO() - img.save(output, "PNG") - return output.getvalue() - - except Exception as e: - raise ValueError(f"Failed to convert mask for OpenAI: {e}") from e + """Encode inverse alpha: transparent=edit, opaque=protect.""" + return _mask_image(mask_bytes, inverse_alpha=True) def normalize_mask_for_fal_ai(mask_bytes: bytes) -> bytes: - """ - Convert Berry mask to Fal.ai-compatible format. - - Fal.ai expects alpha channel where: - - Opaque (alpha=255) = edit region - - Transparent (alpha=0) = protect region - - This matches Berry convention, so no conversion needed - just validate format. - - Args: - mask_bytes: Berry mask PNG bytes - - Returns: - Original mask bytes (already in correct format) - - Raises: - ValueError: If image cannot be processed - """ - try: - # Validate image can be loaded - img = Image.open(BytesIO(mask_bytes)) - img.verify() - return mask_bytes - - except Exception as e: - raise ValueError(f"Failed to validate mask for Fal.ai: {e}") from e - - -def validate_mask_dimensions(mask_path: Path, source_width: int, source_height: int) -> Tuple[int, int]: - """ - Validate mask dimensions match source image dimensions. - - Args: - mask_path: Path to mask image - source_width: Expected width from source image - source_height: Expected height from source image - - Returns: - Tuple of (mask_width, mask_height) - - Raises: - FileNotFoundError: If mask doesn't exist - ValueError: If dimensions don't match - """ - if not mask_path.is_file(): - raise FileNotFoundError(f"Mask file not found: {mask_path}") - - try: - with Image.open(mask_path) as img: - mask_width, mask_height = img.size - - if mask_width != source_width or mask_height != source_height: - raise ValueError( - f"Mask dimensions ({mask_width}x{mask_height}) do not match " - f"source dimensions ({source_width}x{source_height}). " - "Source and mask must have identical dimensions for proper alignment." - ) - - return mask_width, mask_height - - except ValueError: - raise - except Exception as e: - raise ValueError(f"Failed to validate mask dimensions: {e}") from e + """Encode grayscale coverage for flux-general/inpainting.""" + return _mask_image(mask_bytes, inverse_alpha=False) + + +def validate_mask_dimensions(mask_path: Path, source_width: int, source_height: int) -> tuple[int, int]: + """Reject mismatched masks instead of guessing how to resize coordinates.""" + with Image.open(mask_path) as mask: + if mask.size != (source_width, source_height): + raise ValueError( + f"Mask dimensions ({mask.width}x{mask.height}) do not match " + f"source dimensions ({source_width}x{source_height}). " + "Source and mask must have identical dimensions for proper alignment." + ) + return mask.size diff --git a/backend/app/runners/webui_runner.py b/backend/app/runners/webui_runner.py index fc7c458..25ef8ad 100644 --- a/backend/app/runners/webui_runner.py +++ b/backend/app/runners/webui_runner.py @@ -228,8 +228,8 @@ async def _run_inpaint(self, client: httpx.AsyncClient, req: CreativeActionReque # Validate mask dimensions match source (Issue #106) from PIL import Image - source_img = Image.open(img_path) - validate_mask_dimensions(mask_path, source_img.width, source_img.height) + with Image.open(img_path) as source_img: + validate_mask_dimensions(mask_path, source_img.width, source_img.height) # Convert mask for WebUI (Issue #106): grayscale white=edit, black=protect mask_bytes = mask_path.read_bytes() diff --git a/backend/tests/test_comfy_asset_transfer_contract.py b/backend/tests/test_comfy_asset_transfer_contract.py index b489747..b0c14e3 100644 --- a/backend/tests/test_comfy_asset_transfer_contract.py +++ b/backend/tests/test_comfy_asset_transfer_contract.py @@ -50,8 +50,8 @@ def chunk(kind: bytes, data: bytes) -> bytes: async def handle(request: httpx.Request) -> httpx.Response: if request.url.path == "/upload/image": body = await request.aread() - # Check for mask conversion: converted files have "_comfy" suffix - name = "mask" if b'filename="mask' in body else "source" + # Converted masks have unique names so concurrent uploads cannot collide. + name = "mask" if b'filename="berry-mask-' in body else "source" events.append(name) if name == "mask" and fail_mask: return httpx.Response(503) diff --git a/backend/tests/test_mask_adapter_payloads.py b/backend/tests/test_mask_adapter_payloads.py new file mode 100644 index 0000000..ca68414 --- /dev/null +++ b/backend/tests/test_mask_adapter_payloads.py @@ -0,0 +1,63 @@ +"""Known painted regions retain their meaning at the cloud adapter boundary.""" + +import base64 +from io import BytesIO +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest +from PIL import Image + +from app.runners.creative_runner import CreativeRunner, resolve_execution_plan +from app.schemas.creative import CreativeActionRequest + + +@pytest.mark.asyncio +@pytest.mark.parametrize("provider", ["fal_ai", "openai"]) +async def test_cloud_payload_edits_known_region(provider: str, tmp_path: Path) -> None: + source = tmp_path / "source.png" + mask = tmp_path / "mask.png" + Image.new("RGB", (8, 6), "red").save(source) + painted = Image.new("RGBA", (8, 6), (255, 255, 255, 0)) + painted.putpixel((3, 2), (255, 255, 255, 255)) + painted.save(mask) + request = CreativeActionRequest(action="inpaint", engine_id=provider) + with patch("app.runners.creative_runner.credentials_manager.get_key", return_value="fixture-key"), patch( + "app.runners.creative_runner.asset_store.save_image_from_url", new=AsyncMock(return_value=SimpleNamespace(id="output", width=8, height=6)) + ), patch("app.runners.creative_runner._call_fal_ai_action", new=AsyncMock(return_value="https://fixture/output.png")) as fal, patch( + "app.runners.creative_runner._call_openai_inpaint", new=AsyncMock(return_value="https://fixture/output.png") + ) as openai: + await CreativeRunner()._run_cloud(request, resolve_execution_plan(request), source, mask) + if provider == "fal_ai": + encoded = base64.b64decode(fal.call_args.kwargs["mask_b64"]) + with Image.open(BytesIO(encoded)) as actual: + assert actual.size == (8, 6) + assert actual.mode == "L" + assert actual.getpixel((3, 2)) == 255 + assert actual.getpixel((0, 0)) == 0 + else: + with Image.open(BytesIO(openai.call_args.args[2])) as actual: + assert actual.size == (8, 6) + assert actual.getpixel((3, 2))[3] == 0 + assert actual.getpixel((0, 0))[3] == 255 + + +@pytest.mark.asyncio +async def test_misaligned_cloud_mask_blocks_dispatch(tmp_path: Path) -> None: + source, mask = tmp_path / "source.png", tmp_path / "mask.png" + Image.new("RGB", (8, 6)).save(source) + Image.new("RGBA", (4, 3)).save(mask) + records = { + "source": SimpleNamespace(content_hash="source", width=8, height=6, path=source), + "mask": SimpleNamespace(content_hash="mask", path=mask), + } + request = CreativeActionRequest(action="inpaint", engine_id="fal_ai", input_image_id="source", mask_image_id="mask") + runner = CreativeRunner() + with patch("app.runners.creative_runner.asset_store.get_asset", new=AsyncMock(side_effect=lambda key: records[key])), patch( + "app.runners.creative_runner.asset_store.get_absolute_path", side_effect=lambda record: record.path + ), patch.object(runner, "_run_cloud", new=AsyncMock()) as dispatch: + result = await runner.execute(request) + assert not result.success + assert "do not match" in result.error_message + dispatch.assert_not_called() diff --git a/backend/tests/test_mask_converter.py b/backend/tests/test_mask_converter.py index 197328f..7fc4546 100644 --- a/backend/tests/test_mask_converter.py +++ b/backend/tests/test_mask_converter.py @@ -122,22 +122,21 @@ def test_openai_mask_alpha_inversion(): assert unpainted_pixel[3] == 255, f"Unpainted region should have alpha=255 for OpenAI, got {unpainted_pixel[3]}" -def test_fal_ai_mask_passthrough(): +def test_fal_ai_mask_grayscale(): """ - Fal.ai expects opaque (alpha=255) = edit, transparent (alpha=0) = protect. - This matches Berry convention exactly - just validate format. + The supported Fal endpoint expects white=edit and black=protect. """ mask_bytes = create_test_mask(100, 100, (20, 20, 40, 40)) converted_bytes = normalize_mask_for_fal_ai(mask_bytes) - # Should return original bytes since format matches - converted = Image.open(BytesIO(converted_bytes)).convert("RGBA") + converted = Image.open(BytesIO(converted_bytes)) + assert converted.mode == "L" # Painted region should still be opaque - assert converted.getpixel((30, 30))[3] == 255 + assert converted.getpixel((30, 30)) == 255 # Unpainted region should still be transparent - assert converted.getpixel((5, 5))[3] == 0 + assert converted.getpixel((5, 5)) == 0 def test_mask_dimension_validation_success(): @@ -276,3 +275,17 @@ def test_fully_transparent_mask(): from io import BytesIO + + +def test_comfy_conversions_use_distinct_files(tmp_path: Path) -> None: + source = tmp_path / "mask.png" + source.write_bytes(create_test_mask(4, 4, (1, 1, 1, 1))) + first = normalize_mask_for_comfyui(source) + second = normalize_mask_for_comfyui(source) + try: + assert first != second + assert first.read_bytes() == second.read_bytes() + assert source.read_bytes() != first.read_bytes() + finally: + first.unlink() + second.unlink()