Feature/magic eraser #697
Feature/magic eraser #697
Conversation
WalkthroughAdds a Magic Eraser feature: backend ONNX-based inpainting (Inpainter + new FastAPI route), frontend canvas overlay and OCR integrations, automated model download during setup, dependency updates, and tests—integrated into the ImageViewer UI. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FE as Frontend (ImageViewer)
participant Canvas as MagicEraserOverlay (Canvas)
participant API as FastAPI /edit/magic-eraser
participant Inpainter as Inpainter (ONNX)
participant FS as File System
User->>FE: Open image & enable Magic Eraser
FE->>Canvas: Render brush canvas over image
Note over User,Canvas: User paints mask
User->>Canvas: Draw strokes (paths)
Canvas->>Canvas: Build mask PNG (scaled to original)
User->>FE: Click "Remove Object"
FE->>API: POST { image_path, mask_data }
API->>FS: Load image from image_path
API->>Inpainter: inpaint(image, mask)
Inpainter->>Inpainter: Resize to 512x512, normalize, NCHW
Inpainter->>Inpainter: Run ONNX inference
Inpainter->>Inpainter: Postprocess & blend with mask
Inpainter-->>API: Return inpainted image (bytes/base64)
API-->>FE: MagicEraserResponse { success, image_data }
FE->>Canvas: Show preview
User->>FE: Save changes
FE->>FS: Write modified image to disk
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
frontend/src/components/Media/MagicEraserOverlay.tsx (2)
35-49: Canvas doesn't resize when container dimensions change.The canvas size is set once on mount but won't update if the container resizes (e.g., window resize). This could cause misaligned brush strokes.
Add a resize observer or window resize listener:
useEffect(() => { const canvas = canvasRef.current; const container = containerRef.current; - if (canvas && container) { - canvas.width = container.clientWidth; - canvas.height = container.clientHeight; - // ... - } - }, []); + if (!canvas || !container) return; + + const updateCanvasSize = () => { + canvas.width = container.clientWidth; + canvas.height = container.clientHeight; + const ctx = canvas.getContext('2d'); + if (ctx) { + ctx.lineCap = 'round'; + ctx.lineJoin = 'round'; + ctx.strokeStyle = 'rgba(255, 0, 0, 0.5)'; + } + }; + + const resizeObserver = new ResizeObserver(updateCanvasSize); + resizeObserver.observe(container); + updateCanvasSize(); + + return () => resizeObserver.disconnect(); + }, []);
225-229: Consider adding analtattribute with more context.The current
alt="Editing"is generic. For accessibility, consider including more context about what is being edited.frontend/src/components/Media/TextOverlay.tsx (1)
13-39: Support Cmd+C on macOS for copyThe global key handler only checks
e.ctrlKey && e.key.toLowerCase() === 'c'. On macOS users expect Cmd+C; with transparent text, the default browser copy might not behave as intended.Consider expanding the check to also accept
e.metaKey:if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'c') { … }This makes the overlay copy UX consistent across platforms.
frontend/src/components/Media/ImageViewer.tsx (1)
134-178: Avoid overlaying base viewer controls on top of Magic Eraser UIWhen
isMagicEraserActiveis true,MagicEraserOverlay(z-50) covers the viewer, but the base top-left controls (Text Detection/Magic Eraserbuttons,z-60) and OCR banners still render above it. That can visually clash with the Magic Eraser header and close button, and may confuse users about which controls are active.Two simple options:
- Hide or dim the base controls while
isMagicEraserActiveis true, or- Give the Magic Eraser overlay a higher z-index than the base controls and consider not rendering the banners when the overlay is active.
This keeps the editing UI focused and unambiguous.
Also applies to: 253-295
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.gitignore(1 hunks)backend/app/models/Inpainter.py(1 hunks)backend/app/routes/edit.py(1 hunks)backend/main.py(2 hunks)backend/requirements.txt(1 hunks)backend/tests/test_inpainter.py(1 hunks)docs/backend/backend_python/openapi.json(2 hunks)frontend/package.json(1 hunks)frontend/src/components/Media/ImageViewer.tsx(3 hunks)frontend/src/components/Media/MagicEraserOverlay.tsx(1 hunks)frontend/src/components/Media/TextOverlay.tsx(1 hunks)frontend/src/services/OCRService.ts(1 hunks)scripts/download_models.py(1 hunks)scripts/setup.ps1(1 hunks)scripts/setup.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/components/Media/ImageViewer.tsx (3)
frontend/src/services/OCRService.ts (1)
ocrService(59-59)frontend/src/components/Media/TextOverlay.tsx (1)
TextOverlay(10-134)frontend/src/components/Media/MagicEraserOverlay.tsx (1)
MagicEraserOverlay(13-325)
backend/app/routes/edit.py (1)
backend/app/models/Inpainter.py (2)
Inpainter(10-110)inpaint(40-110)
backend/tests/test_inpainter.py (1)
backend/app/models/Inpainter.py (2)
Inpainter(10-110)inpaint(40-110)
🔇 Additional comments (11)
backend/requirements.txt (2)
34-34: Verify numpy version constraint relaxation works with ONNX Runtime.The constraint changed from an exact pin (
numpy==1.26.4) to an upper-bounded range (numpy<2.0.0). This allows any numpy version <2.0 to be installed, not just 1.26.4. While the rationale is clear (blocking numpy 2.0 due to ONNX incompatibility), ensure this relaxation has been tested and won't introduce subtle compatibility issues with the LaMa ONNX model or other downstream dependencies.
35-36: New dependencies aligned with Magic Eraser requirements.The additions of
tqdm==4.66.4andrequests==2.31.0align well with the PR objectives: tqdm for progress tracking during model downloads and requests for HTTP operations in the download script. Both are stable, established libraries with reasonable versions..gitignore (1)
11-12: LGTM!The new ignore pattern for ONNX models is consistent with the existing pattern for image-generation models and correctly prevents large model files from being committed.
backend/app/models/Inpainter.py (2)
40-51: LGTM on the lazy re-initialization pattern.The retry logic to re-initialize the session if it failed previously (e.g., model download completed after initial load attempt) is a good defensive pattern for handling startup race conditions.
62-108: Well-structured image processing pipeline.The preprocessing (resize, normalize, transpose to NCHW), inference, and postprocessing (transpose back, resize to original, blend with original) are correctly implemented. The blending step properly preserves original image quality outside the masked region.
frontend/src/components/Media/MagicEraserOverlay.tsx (1)
190-202: LGTM on undo/redo implementation.The path-based history management correctly handles undo (move from paths to poppedPaths) and redo (move from poppedPaths to paths), with proper clearing of redo history when new strokes are added.
scripts/setup.ps1 (1)
143-147: LGTM - Model download integrated into setup.The model download step is correctly placed after dependencies are installed (ensuring
requestsis available) and before deactivating the virtual environment.Consider adding explicit error handling to distinguish download failures from other setup errors, but the current try-catch provides basic coverage.
frontend/src/services/OCRService.ts (1)
7-56: OCR worker lifecycle and concurrency look good; just confirm API compatibilityThe lazy singleton worker with
workerPromiseis a solid pattern, and error handling/termination logic are reasonable.Please just double‑check that your installed
tesseract.jsversion supportscreateWorker('eng', undefined),Worker.setParameters, andPSM.AUTOexactly as used here.backend/main.py (1)
24-36: Edit router integration is consistent and correctImporting
edit_routerand including it under/editwith its own"Edit"tag matches the existing routing pattern and ensures the Magic Eraser endpoint is exposed and included in generated OpenAPI.Also applies to: 127-136
scripts/download_models.py (1)
10-53: Tighten HTTP error handling and exit codes in model downloaderThe current implementation allows HTTP errors (404, 500, etc.) and truncated downloads to be silently treated as successful. Additionally, the script always exits with code 0 even on failure, preventing downstream setup scripts or CI/CD from detecting problems.
Add
response.raise_for_status()to catch HTTP errors, include atimeoutparameter on network calls, and callsys.exit(1)when downloads fail:-import os -import requests -from tqdm import tqdm +import os +import sys +import requests +from tqdm import tqdm @@ def download_file(url, filename): @@ - response = requests.get(url, stream=True) + response = requests.get(url, stream=True, timeout=30) + response.raise_for_status() @@ print(f"Downloading LaMa ONNX model from {MODEL_URL}...") try: success = download_file(MODEL_URL, MODEL_PATH) if success: print("Download completed successfully!") else: print("Download failed.") if os.path.exists(MODEL_PATH): os.remove(MODEL_PATH) + sys.exit(1) except Exception as e: print(f"An error occurred: {e}") if os.path.exists(MODEL_PATH): - os.remove(MODEL_PATH) + os.remove(MODEL_PATH) + sys.exit(1)This ensures setup fails immediately and clearly when the model cannot be fetched.
backend/app/routes/edit.py (1)
7-15: GlobalInpainterinstance and base64 helpers look solidUsing a single module-level
Inpainterto avoid repeated ONNX session loads is appropriate, and thebase64_to_cv2/cv2_to_base64helpers correctly handle data URL prefixes and PNG encoding for round‑tripping images.Also applies to: 25-37
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
backend/app/routes/edit.py (1)
47-52: Path validation is improved but still fairly ad‑hoc; consider rooting paths under a known directory.The
not os.path.isabs(...) and ".." in body.image_pathcheck plusos.path.existshelps, but a crafted relative path without..(or symlinks) can still escape any intended media directory.If you ever run this outside a strictly sandboxed desktop context, consider normalizing against a configured media root:
MEDIA_ROOT = os.path.abspath(settings.MEDIA_ROOT) candidate = os.path.normpath(os.path.join(MEDIA_ROOT, body.image_path)) if not candidate.startswith(MEDIA_ROOT + os.sep): return MagicEraserResponse(success=False, error="Invalid image path")This would give a stronger guarantee that only files under your media root are accessible.
🧹 Nitpick comments (7)
scripts/download_models.py (1)
31-56: Main function logic is sound.The error handling correctly cleans up partial files and exits non-zero on failure, which integrates well with setup.sh. The TODO about integrity checks is a reasonable deferral for V1.
Minor: Line 55 has inconsistent indentation (extra space).
except Exception as e: print(f"An error occurred: {e}") if os.path.exists(MODEL_PATH): - os.remove(MODEL_PATH) + os.remove(MODEL_PATH) sys.exit(1)frontend/src/components/Media/ImageViewer.tsx (1)
272-290: Consider adding image refresh mechanism.The comments acknowledge that the saved image won't display updated content until the user navigates away and back. For better UX, consider adding a cache-busting mechanism.
One approach: append a timestamp query param to force reload:
// Add state for cache busting const [imageVersion, setImageVersion] = useState(0); // In onSave after writeFile: setImageVersion(v => v + 1); // In img src: src={`${convertFileSrc(imagePath)}?v=${imageVersion}` || '/placeholder.svg'}frontend/src/components/Media/MagicEraserOverlay.tsx (4)
35-49: Canvas size is fixed at mount; consider handling container resize.The canvas dimensions are derived from
container.clientWidth/Heightonly once on mount. If the overlay can be resized (window resize, layout change), the drawing surface and pointer math may drift from the visible image and from theoriginalWidth/originalHeightscaling.If you expect resize, consider updating the canvas size on resize (or when
originalWidth/originalHeightor container size changes) and re-drawing paths accordingly.
51-83: Path model stores per-point size but rendering uses only the first point’s size.You’re storing
{ x, y, size }for every point, but both the committed and current path rendering only usepath[0].size/currentPath[0].sizeforlineWidth. If brush size never changes mid-stroke, you can simplify the state to storesizeonce per path; if you do want variable size within a stroke, you’d need to honor each point’s size when drawing.Right now this is just extra state with no effect on rendering.
129-194: Erase handler and API integration are solid; consider tightening a couple of edge cases.The mask generation, scaling to
originalWidth/originalHeight, env-based API URL, and user-facing error state are all well put together.Two optional robustness tweaks:
- Check
response.okbeforeresponse.json()and surface a clearer message for non‑2xx responses.- Optionally use an
AbortController(and/or an “isMounted” flag) so closing the overlay while a request is in flight doesn’t risk state updates after unmount.These don’t block correctness but would harden the async flow.
252-267: Nice error overlay; consider whether users need a more actionable recovery affordance.The inline error banner with
pointer-events-noneis a good way to show failures without blocking interaction. If errors from the backend can be long or repeated, you might eventually want either truncation or a dismiss/“Try again” affordance, but the current implementation is already a big UX improvement over console-only errors.backend/app/routes/edit.py (1)
58-65: Consider validating mask dimensions against the source image.Right now the code assumes the decoded mask matches the image’s spatial dimensions. If they ever drift (e.g., rounding differences in scaling, client bug),
inpainter.inpaintmay error or give unexpected results.You could add a simple guard (or resize) before inpainting, for example:
- mask = base64_to_cv2(body.mask_data) - if mask is None: - return MagicEraserResponse(success=False, error="Failed to decode mask data") + mask = base64_to_cv2(body.mask_data) + if mask is None: + return MagicEraserResponse(success=False, error="Failed to decode mask data") + + if mask.shape[:2] != image.shape[:2]: + # Option A: error out + return MagicEraserResponse(success=False, error="Mask size does not match image") + # Option B: resize + # mask = cv2.resize(mask, (image.shape[1], image.shape[0]), interpolation=cv2.INTER_NEAREST)This is defensive but can prevent hard‑to‑debug issues if client and server ever diverge.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/app/models/Inpainter.py(1 hunks)backend/app/routes/edit.py(1 hunks)backend/tests/test_inpainter.py(1 hunks)docs/backend/backend_python/openapi.json(2 hunks)frontend/package.json(2 hunks)frontend/src/components/Media/ImageViewer.tsx(3 hunks)frontend/src/components/Media/MagicEraserOverlay.tsx(1 hunks)scripts/download_models.py(1 hunks)scripts/setup.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/tests/test_inpainter.py
- frontend/package.json
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/components/Media/ImageViewer.tsx (3)
frontend/src/services/OCRService.ts (1)
ocrService(59-59)frontend/src/components/Media/TextOverlay.tsx (1)
TextOverlay(10-134)frontend/src/components/Media/MagicEraserOverlay.tsx (1)
MagicEraserOverlay(13-340)
scripts/download_models.py (1)
sync-microservice/app/utils/logger_writer.py (1)
write(31-53)
🔇 Additional comments (15)
scripts/setup.sh (1)
138-145: LGTM! Error handling for model download properly implemented.The setup script now correctly checks the exit status of the download script and aborts with a clear error message if the download fails. This addresses the previous review concern about silent failures.
backend/app/models/Inpainter.py (2)
10-38: Inpainter initialization and session loading look correct.The CUDA/CPU fallback logic is appropriate, and the error handling for missing models properly sets
session = Noneto allow deferred loading attempts.
97-110: Blending logic is well-implemented.The approach of blending the inpainted region with the original image using the mask ensures quality preservation outside the masked area. The float32 arithmetic prevents precision loss during blending.
docs/backend/backend_python/openapi.json (2)
1307-1348: Magic Eraser endpoint schema is well-defined.The endpoint accepts
image_pathandmask_dataas required fields, and the response correctly models nullableimage_dataanderrorfields with onlysuccessas required.
2310-2362: Response schema correctly addresses previous review concern.The
MagicEraserResponseschema now properly modelsimage_dataas nullable (anyOf: [string, null]), which aligns with the expected behavior whereimage_datais only populated on success.frontend/src/components/Media/ImageViewer.tsx (4)
1-9: Imports are appropriate for the new features.The new imports support OCR (tesseract.js Page type, OCRService), Magic Eraser overlay, and file saving capabilities via Tauri plugin-fs.
87-109: OCR trigger logic handles race conditions well.The use of
imagePathRefto guard against state updates when the image has changed mid-processing is a good pattern. The caching behavior (not re-running ifocrDataexists) is reasonable for V1.
197-251: TransformWrapper integration is correct.Disabling panning when OCR is active allows text selection to work properly. The TextOverlay positioning using
imageScaleensures OCR boxes align with the displayed image.
253-256: Guard against zero dimensions properly implemented.The condition now correctly requires
naturalWidth > 0 && naturalHeight > 0before renderingMagicEraserOverlay, addressing the previous review concern about 0×0 mask canvas issues.frontend/src/components/Media/MagicEraserOverlay.tsx (3)
85-125: Pointer math and drawing flow look sound.The shared
getPointerPoscorrectly normalizes mouse and touch coordinates relative to the canvas, and thestartDrawing/draw/stopDrawingsequence cleanly commits paths and resets redo history. This should behave well across input types given thetouch-noneclass on the canvas.
196-208: Undo/redo and control wiring look correct.Undo/redo correctly move paths between
pathsandpoppedPaths, and the buttons’ disabled states (paths.length === 0/poppedPaths.length === 0) match that logic. The brush size slider is also correctly bound tobrushSizewith immediate feedback.Also applies to: 272-307
319-334: Save / discard preview flow is clean and safe.Tying “Discard” to
setPreviewImage(null)and clearingpathsgives a predictable reset state, andonSave(previewImage!)is safe in this branch becausepreviewImageis guaranteed non‑null. This flow should integrate well with the parent’s persistence logic.backend/app/routes/edit.py (3)
16-23: Request/response models are straightforward and match the frontend contract.
MagicEraserRequestandMagicEraserResponsecleanly capture the API shape (path + base64 mask in, success flag + base64 image/error out) and align with the frontend usage ofdata.success,data.image_data, anddata.error.
25-37: Base64 ↔ cv2 helpers look correct for PNG flows.Stripping any
data:*;base64,prefix, decoding into a NumPy buffer, andcv2.imdecode(..., cv2.IMREAD_UNCHANGED)is appropriate here, as is encoding the result back to PNG and returning adata:image/png;base64,...URI.Any decode failures will raise and be caught by the route’s
except, which is acceptable for now.
66-76: Inpainting and error handling are well-structured.Using a module-level
Inpainterinstance avoids reloading the model on each request, and wrapping the body in atry/exceptwithlogger.exceptionplus a generic error string gives good observability without leaking internals to the client. Combined with thesuccessflag, this keeps the API contract consistent.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
backend/tests/test_inpainter.py (1)
11-57: Convert this into a real test with assertions (or move out oftests/)Several failure paths (
session is None, shape mismatch) only print andreturn, so under pytest this test still passes; others callsys.exit(1), which is also not ideal in a test suite. This remains more of a manual debug script than a unit test.-import sys @@ -def test_inpainter(): - print("Initializing Inpainter...") - try: - inpainter = Inpainter() - if inpainter.session is None: - print("FAILED: Model session not initialized. Model file might be missing.") - return +def test_inpainter_end_to_end(): + inpainter = Inpainter() + assert inpainter.session is not None, "Model session not initialized. Model file might be missing." @@ - print("Creating dummy image and mask...") - # Create a 512x512 gradient image - img = np.zeros((512, 512, 3), dtype=np.uint8) - for i in range(512): - img[i, :, :] = i // 2 + # Create a 512x512 gradient image + img = np.zeros((512, 512, 3), dtype=np.uint8) + img[:, :, :] = np.linspace(0, 255, 512, dtype=np.uint8).reshape(512, 1, 1) @@ - # Create a mask (white square in center) - mask = np.zeros((512, 512), dtype=np.uint8) - mask[200:300, 200:300] = 255 + # Create a mask (white square in center) + mask = np.zeros((512, 512), dtype=np.uint8) + mask[200:300, 200:300] = 255 @@ - print("Running inpaint...") - result = inpainter.inpaint(img, mask) + result = inpainter.inpaint(img, mask) @@ - print("Inpaint finished.") - print(f"Result shape: {result.shape}") - - if result.shape != img.shape: - print(f"FAILED: Shape mismatch. Expected {img.shape}, got {result.shape}") - return + assert result.shape == img.shape, f"Shape mismatch. Expected {img.shape}, got {result.shape}" @@ - center_pixel = result[250, 250] - print(f"Center pixel value: {center_pixel}") - - if np.all(center_pixel == 0): - print(f"FAILED: Center pixel is black (0). Model output likely [0, 1] but treated as [0, 255].") - sys.exit(1) - - print("SUCCESS: Inpainter verification passed.") - - except Exception as e: - print(f"FAILED: Exception occurred: {e}") - import traceback - traceback.print_exc() - sys.exit(1) - -if __name__ == "__main__": - test_inpainter() + center_pixel = result[250, 250] + assert not np.all(center_pixel == 0), "Center pixel is black (0); model output likely not scaled correctly."backend/app/routes/edit.py (1)
38-76: Hardenimage_pathvalidation to properly prevent path traversal / arbitrary file accessThe current check (
".." in body.image_path) is not sufficient andbase_diris unused. Clients can still supply absolute paths (e.g./etc/passwd) or tricky relatives without.., and you then passbody.image_pathdirectly tocv2.imread.Consider constraining requests to a known media root and always resolving against it, then using that resolved path for both existence check and
imread:-from fastapi import APIRouter, HTTPException, Body +from fastapi import APIRouter, Body @@ -inpainter = Inpainter() +inpainter = Inpainter() +MEDIA_ROOT = os.path.abspath(os.path.join(os.getcwd(), "media")) # adjust to your storage layout @@ - # Custom Validation: Prevent Path Traversal - # Ensure path is absolute and doesn't contain traversal sequences - abs_path = os.path.abspath(body.image_path) - base_dir = os.path.abspath(os.getcwd()) # Or a specific allowed media directory - - # Simple check for ".." usage which suggests traversal attempts - if ".." in body.image_path: - return MagicEraserResponse(success=False, error="Invalid image path: Path traversal detected") - - if not os.path.exists(abs_path): - return MagicEraserResponse(success=False, error="Image file not found") - - image = cv2.imread(body.image_path) + # Resolve image_path against MEDIA_ROOT and prevent traversal + candidate_path = os.path.normpath(os.path.join(MEDIA_ROOT, body.image_path)) + if not candidate_path.startswith(MEDIA_ROOT + os.sep): + return MagicEraserResponse(success=False, error="Invalid image path") + + if not os.path.exists(candidate_path): + return MagicEraserResponse(success=False, error="Image file not found") + + image = cv2.imread(candidate_path)(Adjust
MEDIA_ROOTto match however your app stores user images.)
🧹 Nitpick comments (3)
scripts/download_models.py (1)
15-30: Improve network robustness with timeout and safer progress-bar handlingThe download flow is correct, but a couple of small hardening tweaks would make it more robust: add a reasonable timeout to
requests.getand ensure the progress bar is closed even if an exception occurs while streaming.- response = requests.get(url, stream=True) + response = requests.get(url, stream=True, timeout=30) @@ - progress_bar = tqdm(total=total_size_in_bytes, unit='iB', unit_scale=True) - - with open(filename, 'wb') as file: - for data in response.iter_content(block_size): - progress_bar.update(len(data)) - file.write(data) - progress_bar.close() + progress_bar = tqdm(total=total_size_in_bytes, unit='iB', unit_scale=True) + + try: + with open(filename, 'wb') as file: + for data in response.iter_content(block_size): + if not data: + continue + progress_bar.update(len(data)) + file.write(data) + finally: + progress_bar.close()frontend/src/components/Media/MagicEraserOverlay.tsx (1)
127-208: Tighten erase flow and history reset semanticsFunctionally this works, but two small refinements would make behavior more predictable:
- Guard
handleEraseagainst concurrent calls (in case it’s ever invoked from elsewhere), not just via the disabled button.- When discarding a preview, also clear
poppedPathsso undo/redo history doesn’t carry over from a previous erase session.const handleErase = async () => { - if (paths.length === 0) return; + if (paths.length === 0 || isProcessing) return; @@ <button onClick={() => { setPreviewImage(null); - setPaths([]); // Clear paths or keep them? Usually clear if we want fresh edit. + setPaths([]); + setPoppedPaths([]); }}Also applies to: 271-336
frontend/src/components/Media/ImageViewer.tsx (1)
253-299: Consider forcing an image refresh after Magic Eraser saveThe save flow correctly overwrites the file, but the viewer may continue showing a cached image until the user reloads/navigates. A lightweight pattern is to track a local
imageVersionand include it in the renderedimgkey or src to force refresh after writes.- const transformRef = useRef<any>(null); + const transformRef = useRef<any>(null); + const [imageVersion, setImageVersion] = useState(0); @@ - <img + <img ref={imgRef} - src={convertFileSrc(imagePath) || '/placeholder.svg'} + src={convertFileSrc(imagePath) || '/placeholder.svg'} alt={alt} @@ - className="select-none block max-w-full max-h-full" + className="select-none block max-w-full max-h-full" + key={`${imagePath}-${imageVersion}`} @@ onSave={async (base64Data) => { try { @@ await writeFile(imagePath, bytes); @@ - setIsMagicEraserActive(false); + setIsMagicEraserActive(false); + setImageVersion((v) => v + 1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/app/models/Inpainter.py(1 hunks)backend/app/routes/edit.py(1 hunks)backend/tests/test_inpainter.py(1 hunks)frontend/src/components/Media/ImageViewer.tsx(3 hunks)frontend/src/components/Media/MagicEraserOverlay.tsx(1 hunks)scripts/download_models.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
backend/tests/test_inpainter.py (1)
backend/app/models/Inpainter.py (2)
Inpainter(10-115)inpaint(40-115)
backend/app/routes/edit.py (1)
backend/app/models/Inpainter.py (2)
Inpainter(10-115)inpaint(40-115)
frontend/src/components/Media/ImageViewer.tsx (3)
frontend/src/services/OCRService.ts (1)
ocrService(59-59)frontend/src/components/Media/TextOverlay.tsx (1)
TextOverlay(10-134)frontend/src/components/Media/MagicEraserOverlay.tsx (1)
MagicEraserOverlay(13-340)
🔇 Additional comments (1)
backend/app/models/Inpainter.py (1)
10-115: Inpainter pipeline looks consistent and robustModel loading, preprocessing (resize + normalization), inference, and blending back into the original image are wired together coherently, including the [0,1]/[0,255] output-range handling and session re-init on first use.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
backend/tests/test_inpainter.py (1)
11-51: Test still silently “passes” when the model session is missingIf
inpainter.session is None, the test prints a failure message and thenreturns, so under a test runner this is still reported as a passing test even though the model failed to load. This is effectively the same problem highlighted in the earlier review: regressions in model loading won’t fail the test suite. Also, the broadtry/exceptplussys.exit(1)is unnecessary in a unit test—letting exceptions propagate already fails the test with a clear traceback.Consider tightening this along the lines of:
- Treat missing model as a hard failure:
def test_inpainter(): inpainter = Inpainter() assert inpainter.session is not None, "Model session not initialized. Model file might be missing." # ... construct img/mask and run assertions as you already do ...
- Or, if you truly want this to be an optional integration test, explicitly mark it as skipped instead of returning silently, e.g. with
pytest.skip(...)so the test report reflects that the check was not actually executed.You can then drop the
try/exceptandsys.exit(1)entirely and let any unexpected errors fail the test.
🧹 Nitpick comments (1)
backend/tests/test_inpainter.py (1)
1-9: Avoid mutatingsys.pathinside tests; prefer proper package setupManually appending
backendtosys.pathin the test is brittle and can mask real import/package issues. It’s usually cleaner to rely on the project’s packaging/pytest configuration (e.g., running tests from the backend root with the correct PYTHONPATH) or to use proper package imports instead of adjustingsys.pathat runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/tests/test_inpainter.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/tests/test_inpainter.py (1)
backend/app/models/Inpainter.py (2)
Inpainter(10-115)inpaint(40-115)
2 participants
Fixes #696
Description
Currently, users cannot easily remove unwanted objects or people from their photos within PictoPy. They need to use external tools for basic photo editing tasks like object removal, which breaks their workflow and reduces productivity.
Magic Eraser introduces a powerful AI-powered inpainting feature that allows users to seamlessly remove unwanted elements from their photos with just a few brush strokes. This feature uses a lightweight ONNX model to generate realistic backgrounds where objects are removed, providing professional-grade editing capabilities within the app.
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.