-
Notifications
You must be signed in to change notification settings - Fork 6
Fix color profile #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe pull request converts synchronous image preview and metadata loading methods to asynchronous variants across the codebase. Changes include converting Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Command/Caller
participant PVM as PictureItemViewModel
participant Metadata as MetadataLoader
participant BitmapLoader as BitmapSource Loader
Client->>PVM: LoadPreviewAsync(ct, maxWidth, preservePixelFormat)
activate PVM
alt _metadataString is null
PVM->>Metadata: LoadMetadataAsync(ct)
activate Metadata
Metadata-->>PVM: metadata loaded
deactivate Metadata
end
PVM->>BitmapLoader: Load preview bitmap with options
activate BitmapLoader
BitmapLoader-->>PVM: BitmapSource
deactivate BitmapLoader
PVM-->>Client: Task<BitmapSource?>
deactivate PVM
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@PhotoLocator/PictureFileFormats/GeneralFileFormatHandler.cs`:
- Around line 23-25: Current code unconditionally adds
BitmapCreateOptions.IgnoreColorProfile when preservePixelFormat is true, which
suppresses embedded ICC profiles; change this so IgnoreColorProfile is not
applied by default. Update the bitmap.CreateOptions assignment to only include
BitmapCreateOptions.PreservePixelFormat when preservePixelFormat is true, and
add a conditional code path that appends BitmapCreateOptions.IgnoreColorProfile
only when a new explicit signal indicates it's required (e.g., an added
parameter/field like ignoreColorProfileForCorruptMetadata or a helper
IsColorProfileCorrupt(bitmapMetadata) that detects broken profiles). Reference
preservePixelFormat, bitmap.CreateOptions,
BitmapCreateOptions.PreservePixelFormat and
BitmapCreateOptions.IgnoreColorProfile to locate and adjust the logic; ensure
BitmapCacheOption.OnLoad remains unchanged.
In `@PhotoLocator/SlideShowWindow.xaml.cs`:
- Around line 133-136: The preview load can overlap with subsequent timer ticks;
wrap the async load in a version/cancellation guard and only restart the
DispatcherTimer after applying the preview: increment a private int version
field (e.g., _pictureLoadVersion) at the start of UpdatePictureAsync, capture it
into a local variable before awaiting SelectedPicture.LoadPreviewAsync, then
after the await verify the local version still equals _pictureLoadVersion before
assigning _sourceImage and calling UpdateResampledImage; also stop or avoid
starting _timer until after the successful guarded update so ticks won't trigger
concurrent loads.
🧹 Nitpick comments (1)
PhotoLocator/JpegTransformCommands.cs (1)
131-155: CancellationToken not propagated to async operations.The method uses
defaultfor the cancellation token on line 134, but inBatchProcessLocalContrastAsync(line 192), the caller has access toct. This means batch processing cancellation won't interrupt ongoingLoadPreviewAsynccalls.Consider accepting a
CancellationTokenparameter:♻️ Suggested refactor
-private static async Task<(BitmapSource, BitmapMetadata?)> LoadImageWithMetadataAsync(PictureItemViewModel item) +private static async Task<(BitmapSource, BitmapMetadata?)> LoadImageWithMetadataAsync(PictureItemViewModel item, CancellationToken ct = default) { BitmapMetadata? metadata = null; - var image = await item.LoadPreviewAsync(default, int.MaxValue, preservePixelFormat: true); + var image = await item.LoadPreviewAsync(ct, int.MaxValue, preservePixelFormat: true);
Summary by CodeRabbit
Bug Fixes
Performance Improvements
✏️ Tip: You can customize this high-level summary in your review settings.