Release v0.4 — more improvements#6
Conversation
WalkthroughThis PR introduces version 0.4 with a two-tier display-aware caching system, asynchronous preload-all-images feature with progress tracking, display size change notifications, Material theming support for dark/light modes, and borderless fullscreen window mode. Changes
Sequence DiagramsequenceDiagram
actor User
participant QML as QML UI
participant UIState as UIState (Provider)
participant AppCtrl as AppController
participant Prefetcher
participant Worker as ThreadPoolExecutor
participant Cache as Image Cache
User->>QML: Click "Preload All Images"
QML->>UIState: preloadAllImages()
UIState->>AppCtrl: preload_all_images()
rect rgb(200, 220, 255)
Note over AppCtrl,Worker: Async Preload Phase
AppCtrl->>Worker: submit image decode tasks
par Background Processing
Worker->>Prefetcher: _decode_and_cache (for each image)
Prefetcher->>Prefetcher: get_display_info()
Prefetcher->>Prefetcher: decode_jpeg_resized(width, height)
Prefetcher->>Cache: store with generation-aware key
Worker->>AppCtrl: emit progress_updated signal
end
end
AppCtrl->>UIState: _update_preload_progress(progress %)
UIState->>UIState: setIsPreloading(true), setPreloadProgress(%)
UIState->>QML: preloadProgressChanged, preloadingStateChanged
QML->>QML: Update ProgressBar, show indicator
Worker->>AppCtrl: emit finished signal
AppCtrl->>UIState: _finish_preloading()
UIState->>UIState: setIsPreloading(false)
UIState->>QML: preloadingStateChanged
QML->>QML: Hide indicator, clear progress
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 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: 1
🧹 Nitpick comments (6)
faststack/README.md (1)
3-3: Optional: Consider adding comma after year per style guidelines.Some style guides recommend a comma after the year in month-day-year dates.
Apply this diff if you wish to follow that convention:
-# Version 0.4 - November 2, 2025 +# Version 0.4 - November 2, 2025,faststack/faststack/logging_setup.py (1)
30-30: Consider making root log level configurable.Setting the root logger to DEBUG will significantly increase log verbosity. While this is helpful for debugging the new caching and preloading features, consider making the log level configurable (e.g., via environment variable or config file) to allow users to reduce verbosity in production use.
faststack/faststack/qml/Components.qml (1)
18-34: Consider debouncing display size change notifications.The implementation correctly reports display size changes to the UI state. However, when the window is resized, both
onWidthChangedandonHeightChangedhandlers will fire separately, potentially causing two rapid calls toonDisplaySizeChanged. While this may be acceptable if the backend handles it efficiently, consider using a Timer to debounce these notifications during rapid resizing.Example debounced approach:
Timer { id: resizeDebounceTimer interval: 100 repeat: false onTriggered: { if (mainImage.width > 0 && mainImage.height > 0) { uiState.onDisplaySizeChanged(mainImage.width, mainImage.height) } } } Component.onCompleted: { if (width > 0 && height > 0) { uiState.onDisplaySizeChanged(width, height) } } onWidthChanged: { resizeDebounceTimer.restart() } onHeightChanged: { resizeDebounceTimer.restart() }faststack/faststack/imaging/jpeg.py (1)
77-93: Improve exception handling and consider PyTurboJPEG support.The function works correctly but has a few areas for improvement:
Exception handling: Use
logging.exceptioninstead oflogging.errorto capture the full stack trace, which aids debugging.PyTurboJPEG support: Unlike
decode_jpeg_rgbanddecode_jpeg_thumb_rgb, this function only uses Pillow. Consider adding PyTurboJPEG support for consistency and potential performance gains.Apply this diff to improve exception handling:
except Exception as e: - log.error(f"Pillow failed to decode and resize image: {e}") + log.exception(f"Pillow failed to decode and resize image: {e}") return NoneConsider adding PyTurboJPEG support similar to other decode functions:
def decode_jpeg_resized( jpeg_bytes: bytes, width: int, height: int ) -> Optional[np.ndarray]: """Decodes and resizes a JPEG to fit within the given dimensions.""" if width == 0 or height == 0: return decode_jpeg_rgb(jpeg_bytes) if TURBO_AVAILABLE and jpeg_decoder: try: # Get dimensions and find appropriate scaling factor img_width, img_height, _, _ = jpeg_decoder.decode_header(jpeg_bytes) scaling_factor = _get_turbojpeg_scaling_factor(img_width, img_height, max(width, height)) if scaling_factor: return jpeg_decoder.decode( jpeg_bytes, scaling_factor=scaling_factor, pixel_format=TJPF_RGB, flags=TJFLAG_FASTDCT ) except Exception as e: log.exception(f"PyTurboJPEG failed to decode resized image: {e}. Trying Pillow.") # Fallback to Pillow try: from io import BytesIO img = Image.open(BytesIO(jpeg_bytes)) img.thumbnail((width, height), Image.Resampling.LANCZOS) return np.array(img.convert("RGB")) except Exception as e: log.exception(f"Pillow failed to decode and resize image: {e}") return Nonefaststack/faststack/qml/Main.qml (1)
141-197: Consider cleaning up MenuBar formatting.The MenuBar section has excessive blank lines and unusual indentation that makes it harder to read. While this doesn't affect functionality, consider normalizing the formatting for better maintainability.
Example cleaned-up version:
MenuBar { id: menuBar Layout.preferredWidth: 300 background: Rectangle { color: root.currentBackgroundColor } palette.buttonText: root.currentTextColor palette.button: root.currentBackgroundColor palette.window: root.currentBackgroundColor palette.text: root.currentTextColor Menu { title: "&File" // ... menu items } // ... other menus }faststack/faststack/app.py (1)
78-80: Remove the duplicateget_display_info.This second definition just overwrites the first one and triggers Ruff F811. Please drop the extra definition and keep a single method.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
faststack/ChangeLog.md(1 hunks)faststack/README.md(1 hunks)faststack/faststack/app.py(9 hunks)faststack/faststack/imaging/jpeg.py(1 hunks)faststack/faststack/imaging/prefetch.py(4 hunks)faststack/faststack/logging_setup.py(1 hunks)faststack/faststack/qml/Components.qml(1 hunks)faststack/faststack/qml/Main.qml(4 hunks)faststack/faststack/ui/provider.py(2 hunks)faststack/pyproject.toml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
faststack/faststack/ui/provider.py (1)
faststack/faststack/app.py (2)
preload_all_images(361-405)on_display_size_changed(81-91)
faststack/faststack/imaging/prefetch.py (3)
faststack/faststack/imaging/jpeg.py (2)
decode_jpeg_rgb(22-39)decode_jpeg_resized(77-93)faststack/faststack/models.py (1)
ImageFile(8-12)faststack/faststack/app.py (2)
get_display_info(75-76)get_display_info(78-79)
faststack/faststack/app.py (3)
faststack/faststack/config.py (1)
getint(62-63)faststack/faststack/imaging/prefetch.py (3)
update_prefetch(31-51)submit_task(53-64)shutdown(116-120)faststack/faststack/ui/provider.py (4)
isPreloading(70-71)isPreloading(74-77)preloadProgress(80-81)preloadProgress(84-87)
🪛 LanguageTool
faststack/README.md
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ... FastStack # Version 0.4 - November 2, 2025 # By Alan Rockefeller Ultra-fast, cach...
(MISSING_COMMA_AFTER_YEAR)
🪛 Ruff (0.14.2)
faststack/faststack/imaging/jpeg.py
91-91: Do not catch blind exception: Exception
(BLE001)
92-92: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
faststack/faststack/app.py
78-78: Redefinition of unused get_display_info from line 75
(F811)
375-375: f-string without any placeholders
Remove extraneous f prefix
(F541)
392-392: Unused function argument: future
(ARG001)
🔇 Additional comments (9)
faststack/faststack/logging_setup.py (1)
36-36: Good addition of PIL logging configuration.Setting PIL to INFO level prevents excessive debug noise while still capturing important information.
faststack/pyproject.toml (1)
8-8: Version bump looks good.The version update to 0.4 is consistent with the changes documented in the ChangeLog and README.
faststack/ChangeLog.md (1)
3-12: ChangeLog entry is comprehensive and well-structured.The Version 0.4 entry clearly documents all the new features and changes introduced in this PR.
faststack/faststack/qml/Main.qml (4)
4-4: Material theming support looks good.The addition of Material theme import and binding correctly implements dark/light mode theming for the UI components.
Also applies to: 18-18
83-96: Preload progress indicator looks good.The progress bar correctly displays preloading status and progress. The visual design is functional and integrates well with the footer.
251-251: "Preload All Images" action is well-integrated.The new menu action correctly triggers the preload functionality through the UI state.
12-14: No critical multi-monitor issue found; current behavior is intentional.The borderless fullscreen implementation (x: 0, y: 0 with Screen.width/Screen.height) is a deliberate design choice documented in the ChangeLog as providing "better integration with the operating system" and "normal Alt-Tab behavior." The application uses only primary screen dimensions, which is standard for most desktop applications. No multi-monitor support is documented as a requirement in the README or project configuration, and the codebase contains no such logic.
If multi-monitor support becomes a user requirement, Screen.virtualX/Y could be considered as an enhancement; however, for the current single-monitor-focused workflow of this JPG viewer, the existing implementation is appropriate.
faststack/faststack/ui/provider.py (2)
60-87: Preloading state management is well-implemented.The new signals, properties, and backing fields follow Qt best practices:
- Properties have notify signals
- Setters check for value changes before emitting
- Type annotations are correct
- Consistent with existing patterns in the file
212-218: New slots are correctly implemented.Both
preloadAllImages()andonDisplaySizeChanged()properly delegate to the app controller, maintaining the separation of concerns between UI state and application logic.
| self.image_cache.clear() | ||
| self.prefetcher.update_prefetch(self.current_index) | ||
| self.sync_ui_state() # To refresh the image |
There was a problem hiding this comment.
Clear stale futures before re-prefetching after display changes.
After self.image_cache.clear() the Prefetcher.futures dict still contains completed entries from the previous display generation. Because update_prefetch() only submits when an index is missing from self.futures, none of those images get re-decoded for the new display size, so navigation becomes synchronous (and slow) until each image is forced. Call self.prefetcher.cancel_all() (or otherwise clear/cancel the futures) before invoking update_prefetch() so the new display size is prefetched immediately.
🤖 Prompt for AI Agents
In faststack/faststack/app.py around lines 89 to 91, after clearing
self.image_cache you must also clear or cancel existing Prefetcher futures so
completed/stale futures from the previous display don't prevent re-decoding;
call self.prefetcher.cancel_all() (or otherwise clear self.prefetcher.futures)
before self.prefetcher.update_prefetch(self.current_index) so the prefetcher
will submit new decode tasks for the current display size, then call
self.sync_ui_state() as before.
|
@coderabbitai What else could be done to improve performance? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3481 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 3517 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 1116 🏁 Script executed: Length of output: 1596 🏁 Script executed: Length of output: 9309 🏁 Script executed: Length of output: 19533 @AlanRockefeller: Based on my analysis of the codebase, here are several performance improvements you could implement: 1. Use PyTurboJPEG for Resized Decoding ⚡ (High Impact)Currently, def decode_jpeg_resized(
jpeg_bytes: bytes, width: int, height: int
) -> Optional[np.ndarray]:
"""Decodes and resizes a JPEG to fit within the given dimensions."""
if width == 0 or height == 0:
return decode_jpeg_rgb(jpeg_bytes)
if TURBO_AVAILABLE and jpeg_decoder:
try:
# Get image header to determine dimensions
img_width, img_height, _, _ = jpeg_decoder.decode_header(jpeg_bytes)
# Calculate best scaling factor for TurboJPEG (supports 1/8, 1/4, 1/2, etc.)
scale_factor = _get_turbojpeg_scaling_factor(img_width, img_height, max(width, height))
if scale_factor:
decoded = jpeg_decoder.decode(
jpeg_bytes,
scaling_factor=scale_factor,
pixel_format=TJPF_RGB,
flags=TJFLAG_FASTDCT
)
# Only use Pillow for final resize if needed
if decoded.shape[0] > height or decoded.shape[1] > width:
from io import BytesIO
img = Image.fromarray(decoded)
img.thumbnail((width, height), Image.Resampling.LANCZOS)
return np.array(img)
return decoded
except Exception as e:
log.error(f"PyTurboJPEG failed: {e}")
# Fallback to Pillow (existing code)
try:
from io import BytesIO
img = Image.open(BytesIO(jpeg_bytes))
img.thumbnail((width, height), Image.Resampling.LANCZOS)
return np.array(img.convert("RGB"))
except Exception as e:
log.error(f"Pillow failed to decode and resize image: {e}")
return None2. Tune ThreadPool Workers Based on System ResourcesThe prefetcher uses a fixed import os
class Prefetcher:
def __init__(self, image_files: List[ImageFile], cache_put: Callable,
prefetch_radius: int, get_display_info: Callable):
# Use CPU count for I/O-bound JPEG decoding
# Rule of thumb: 2x CPU cores for I/O bound, 1x for CPU bound
optimal_workers = min(os.cpu_count() * 2, 8) # Cap at 8
self.executor = ThreadPoolExecutor(
max_workers=optimal_workers,
thread_name_prefix="Prefetch"
)3. Memory-Mapped File I/O 💾Replace synchronous file reads with memory-mapped I/O for better performance: import mmap
def _decode_and_cache(self, image_file: ImageFile, index: int, generation: int,
display_width: int, display_height: int,
display_generation: int) -> Optional[tuple[int, int]]:
try:
# Memory-mapped file reading (faster than traditional read)
with open(image_file.path, "rb") as f:
with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mmapped:
jpeg_bytes = mmapped[:]
buffer = decode_jpeg_resized(jpeg_bytes, display_width, display_height)
# ... rest of the method4. Share Executor for PreloadingThe preload uses a separate single-worker executor. Share the prefetch executor instead: def preload_all_images(self):
if self.ui_state.isPreloading:
log.info("Preloading is already in progress.")
return
log.info("Starting to preload all images.")
self.ui_state.isPreloading = True
self.ui_state.preloadProgress = 0
# Use existing prefetch executor (better resource utilization)
total = len(self.image_files)
completed = 0
def _on_done(future):
nonlocal completed
completed += 1
progress = int((completed / total) * 100)
self.reporter.progress_updated.emit(progress)
if completed == total:
self.reporter.finished.emit()
for i in range(total):
future = self.prefetcher.submit_task(i, self.prefetcher.generation)
if future:
future.add_done_callback(_on_done)5. Implement Disk Cache for Resized Images 💿Add a disk cache layer to avoid re-decoding images on application restart: import hashlib
from pathlib import Path
class DiskCache:
def __init__(self, cache_dir: Path, max_size_gb: float = 5.0):
self.cache_dir = cache_dir / ".faststack_cache"
self.cache_dir.mkdir(exist_ok=True)
self.max_size = int(max_size_gb * 1024**3)
def get_cache_key(self, filepath: Path, width: int, height: int) -> str:
"""Generate cache key from file path, size, and mtime."""
stat = filepath.stat()
key_str = f"{filepath}_{width}x{height}_{stat.st_mtime}_{stat.st_size}"
return hashlib.sha256(key_str.encode()).hexdigest()[:16]
def get(self, cache_key: str) -> Optional[np.ndarray]:
cache_file = self.cache_dir / f"{cache_key}.npy"
if cache_file.exists():
try:
return np.load(cache_file)
except:
cache_file.unlink()
return None
def put(self, cache_key: str, data: np.ndarray):
cache_file = self.cache_dir / f"{cache_key}.npy"
np.save(cache_file, data)6. Adaptive Prefetch Radius 🎯Dynamically adjust prefetch based on navigation speed: class Prefetcher:
def __init__(self, ...):
# ... existing code ...
self.navigation_history = []
self.adaptive_radius = prefetch_radius
def update_prefetch(self, current_index: int):
import time
now = time.time()
self.navigation_history.append((current_index, now))
# Keep last 10 navigations
self.navigation_history = self.navigation_history[-10:]
# Calculate navigation speed
if len(self.navigation_history) >= 3:
time_span = now - self.navigation_history[0][1]
nav_count = len(self.navigation_history) - 1
if time_span > 0:
speed = nav_count / time_span # navigations per second
# Adjust radius: faster navigation = larger radius
if speed > 2: # More than 2 images per second
self.adaptive_radius = min(self.prefetch_radius * 2, 12)
elif speed < 0.5: # Less than 0.5 images per second
self.adaptive_radius = max(self.prefetch_radius // 2, 2)
else:
self.adaptive_radius = self.prefetch_radius
# Use adaptive_radius instead of prefetch_radius in calculations7. Use LANCZOS Only When NecessarySwitch to faster resampling for large scale factors: # In decode_jpeg_resized:
scale_factor_ratio = min(img.width / width, img.height / height)
# Use faster BILINEAR for large downscales, LANCZOS for smaller
if scale_factor_ratio > 4:
resampling = Image.Resampling.BILINEAR # Much faster
else:
resampling = Image.Resampling.LANCZOS # Higher quality
img.thumbnail((width, height), resampling)8. Add Performance ProfilingAdd timing metrics to identify bottlenecks: import time
from contextlib import contextmanager
@contextmanager
def timed_operation(operation_name: str):
start = time.perf_counter()
try:
yield
finally:
elapsed = time.perf_counter() - start
log.debug(f"{operation_name} took {elapsed*1000:.2f}ms")
# Usage in _decode_and_cache:
with timed_operation(f"Decode image {index}"):
buffer = decode_jpeg_resized(jpeg_bytes, display_width, display_height)Priority Recommendations:
Would you like me to help implement any of these optimizations? 🐰✨ |
1 participant
New Features
usage.
en for unviewed images.
Changes
ng system.
Summary by CodeRabbit
Release Notes - Version 0.4
New Features
Changes