Update image editor by AlanRockefeller · Pull Request #26 · AlanRockefeller/faststack

AlanRockefeller · 2026-01-02T18:47:35Z

Summary by CodeRabbit

Release Notes

Performance
- Asynchronous histogram and preview rendering prevents UI freezing during heavy editing operations.
Bug Fixes
- Improved input validation for crop operations with bounds clamping and ordering.
- Added safeguards against division by zero in crop aspect ratio calculation.
- Enhanced image format handling and buffer management for safer editing.
UI/UX Improvements
- Parameter slider changes now throttled for smoother, more responsive editing.
- Console logging added for improved debugging visibility.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-02T18:47:45Z

Walkthrough

The PR introduces asynchronous rendering for histograms and previews using background thread pools with token-based result matching to prevent stale data application. It enhances ImageEditor with thread-safety, caching, and new public methods for preview retrieval. Input validation, logging, and UI slider throttling are refined. Image buffer management is optimized for live editing, and crop box handling gains defensive copies and bounds checking.

Changes

Cohort / File(s)	Summary
Async Rendering Infrastructure `faststack/faststack/app.py`	Introduced histogramReady and previewReady signals with corresponding background ThreadPoolExecutor workers. Added state management (token, in-flight, pending, locks) for concurrent task orchestration and token-based result deduplication. Integrated graceful executor shutdown on app exit. Refactored histogram and preview updates from direct calls to signal-based handlers (_apply_histogram_result, _apply_preview_result). Added throttled preview refresh via _preview_refresh_pending flag and timer. Enhanced execute_crop with robust crop_box_raw validation (4-tuple normalization, bounds clamping, format handling).
Image Processing & Caching `faststack/faststack/imaging/editor.py`	Added re-entrant lock and revision/caching counters for thread-safe state management. Introduced get_preview_data_cached(allow_compute) and get_edit_value(key, default) public methods. Modified _apply_edits signature to accept optional external edits snapshot. Updated rotate_image_cw/rotate_image_ccw to operate under lock and increment revision counter. Enhanced exception logging.
Input Validation `faststack/faststack/imaging/jpeg.py`	Tightened decode_jpeg_resized validation from `== 0` to `<= 0` for width and height, rejecting negative or zero dimensions.
Logging & Instrumentation `faststack/faststack/imaging/prefetch.py`	Added explicit copy phase duration (copy=) timing to ICC and non-ICC decode paths, improving performance metric granularity.
Logging Setup `faststack/faststack/logging_setup.py`	Introduced console handler (StreamHandler) alongside existing rotating file handler; both attached to root logger with shared formatter.
Documentation `faststack/faststack/config.py`	Added extensive inline documentation for Auto Levels feature (behavior, algorithm, tuning guidance); configuration keys and defaults unchanged.
QML Crop Handling `faststack/faststack/qml/Components.qml`	Introduced defensive copy of crop box (array to local variable) to prevent in-place mutations. Added guard for division by zero in cropStartAspect calculation (boxH > 0 check). Replaced repeated currentCropBox fetches with consistent local box variable usage.
QML Slider Throttling & Headers `faststack/faststack/qml/ImageEditorDialog.qml`	Removed headerText property from SectionHeader; now assigned via onLoaded handlers in Loader instances with emoji-label strings. Introduced slider auto-synchronization with internal state (_pendingValue, _lastSentValue) and throttle timer (sendTimer) to batch parameter updates. Enhanced onMoved and onPressedChanged to queue changes and defer backend updates until timer fires or slider release.
Image Provider Buffer Management `faststack/faststack/ui/provider.py`	Added specialized image fetch path for live editing: uses last rendered preview if available, otherwise falls back to get_decoded_image. Conditional buffer management: deep-copies QImage when editing (detach from shared memory), attaches buffer reference when not editing (prevent GC). Guarded QImage creation with format detection (None → RGB888).

Sequence Diagrams

sequenceDiagram
    participant UI as UI Thread
    participant AC as AppController
    participant Pool as Thread Pool
    participant IE as ImageEditor
    
    rect rgb(230, 245, 250)
    Note over UI,IE: Histogram Async Flow
    UI->>AC: set_edit_parameter(key, value)
    AC->>AC: _kick_histogram_worker()
    AC->>Pool: submit(_compute_histogram_worker)
    
    Pool->>IE: get_preview_data_cached()
    IE-->>Pool: DecodedImage (cached)
    Pool->>Pool: compute histogram from image
    
    Pool-->>AC: _on_histogram_done(future)
    AC->>AC: extract result (token-guarded)
    AC->>UI: emit histogramReady(payload)
    UI->>UI: _apply_histogram_result(payload)
    end
    
    rect rgb(245, 230, 250)
    Note over UI,IE: Preview Async Flow
    UI->>AC: load_image_for_editing() / enter crop mode
    AC->>AC: _kick_preview_worker()
    AC->>Pool: submit(_render_preview_worker)
    
    Pool->>IE: render preview with current edits
    IE-->>Pool: rendered DecodedImage
    Pool-->>AC: _on_preview_done(future)
    
    AC->>AC: token match check
    AC->>AC: _last_rendered_preview = result
    AC->>UI: emit previewReady(payload)
    UI->>UI: _apply_preview_result(payload)
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

PR #24: Directly related—modifies the same AppController crop/zoom handling, ImageEditor rotation/autocrop, and UI provider crop wiring that this PR refactors.
PR #23: Directly related—updates ImageEditor._apply_edits contract to accept optional external edits snapshot, mirroring changes in this PR's caching/preview logic.
PR #25: Directly related—overlaps with histogram and execute_crop behavior modifications in AppController.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Title check	❓ Inconclusive	The title 'Update image editor' is vague and generic, failing to convey the specific nature of the substantial changes made across the codebase.	Consider using a more descriptive title that captures the primary change, such as 'Add asynchronous offloading for histogram and preview rendering' or 'Implement background thread pools for image editor operations'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

faststack/faststack/imaging/editor.py (1)
58-58: Minor: Redundant exception object in log.exception() calls.

Per static analysis (TRY401), log.exception() automatically includes the current exception, so passing e is redundant. This is a cosmetic issue and doesn't affect functionality.
🔎 Proposed fix
-        log.exception(f"Failed to create backup: {e}")
+        log.exception("Failed to create backup")
Similar changes can be applied to lines 258, 722, and 731.
Also applies to: 258-258, 722-722, 731-731
faststack/faststack/app.py (2)
3027-3049: Robust crop box normalization with good defensive coding.

The validation handles QJSValue wrappers, list-to-tuple conversion, bounds clamping, and coordinate ordering. This prevents edge cases from causing crashes.

Minor: Static analysis flags l (line 3041) as an ambiguous variable name. Consider using left for clarity.
🔎 Proposed fix for variable naming
-            l, t, r, b = [max(0, min(1000, int(x))) for x in crop_box_raw]
-            
-            # Ensure correct order (left <= right, top <= bottom)
-            crop_box_raw = (min(l, r), min(t, b), max(l, r), max(t, b))
+            left, top, right, bottom = [max(0, min(1000, int(x))) for x in crop_box_raw]
+            
+            # Ensure correct order (left <= right, top <= bottom)
+            crop_box_raw = (min(left, right), min(top, bottom), max(left, right), max(top, bottom))
2504-2528: Broad exception handling is appropriate here for UI stability.

Catching Exception in set_edit_parameter prevents slider interactions from crashing the app. The error is logged for debugging. Per static analysis, consider using log.exception() instead of log.error() to include the stack trace.
🔎 Proposed fix
         except Exception as e:
-            log.error("Error setting edit parameter %s=%s: %s", key, value, e)
+            log.exception("Error setting edit parameter %s=%s", key, value)

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fda086c and ca65fc0.

📒 Files selected for processing (9)

faststack/faststack/app.py
faststack/faststack/config.py
faststack/faststack/imaging/editor.py
faststack/faststack/imaging/jpeg.py
faststack/faststack/imaging/prefetch.py
faststack/faststack/logging_setup.py
faststack/faststack/qml/Components.qml
faststack/faststack/qml/ImageEditorDialog.qml
faststack/faststack/ui/provider.py

🧰 Additional context used

📓 Path-based instructions (5)

faststack/faststack/**/*.py