Skip to content

Conversation

@meesoft
Copy link
Owner

@meesoft meesoft commented Jan 24, 2026

Summary by CodeRabbit

  • New Features

    • Added Crossfade effect to video transformation and effects processing options.
  • Bug Fixes

    • Improved null-safety validation across bitmap operations for enhanced application stability.
  • Style

    • Enhanced PictureItemView component background rendering with transparent background support.

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

@meesoft meesoft marked this pull request as ready for review January 25, 2026 08:52
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 25, 2026

Walkthrough

Updates .gitignore to ignore publish.bat, removes null-forgiving operators from accumulator array accesses across bitmap operations, adds [MemberNotNull] attribute to PrepareFrame method, adds transparent background to PictureItemView XAML, and introduces Crossfade effect support to VideoTransformCommands with related nullability improvements.

Changes

Cohort / File(s) Summary
Configuration
.gitignore
Adds publish.bat to ignore list; TestResults/ entry re-added with line handling adjustment
Bitmap Operations – Null-Safety Cleanup
PhotoLocator/BitmapOperations/AverageFramesOperation.cs, FadingAverageOperation.cs, FadingMaxOperation.cs, RollingAverageOperation.cs, TimeCompressionAverageOperation.cs
Removes null-forgiving operator (!) from _accumulatorPixels array accesses; assumes non-null at point of use
Bitmap Operations – Nullability Contract
PhotoLocator/BitmapOperations/CombineFramesOperationBase.cs
Adds using System.Diagnostics.CodeAnalysis directive and [MemberNotNull(nameof(_accumulatorPixels))] attribute to PrepareFrame method to declare that _accumulatorPixels is guaranteed non-null after method returns
UI Styling
PhotoLocator/PictureItemView.xaml
Adds Background="Transparent" attribute to root UserControl
Video Transform – Crossfade Effect & Nullability
PhotoLocator/VideoTransformCommands.cs
Introduces Crossfade constant; adds [MemberNotNullWhen(true, nameof(_localContrastSetup))] to IsLocalContrastChecked property; updates scale and local-contrast processing logic to account for Crossfade; refactors property getter/setter to use backing field checks

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • Reverse video effect #54: Modifies VideoTransformCommands.cs to add effect identifiers (Zoom/Reverse) and adjust scale-skipping logic based on active effects, directly related to Crossfade effect handling in this PR.
  • Improve dark frame handling #53: Updates nullability annotations and accumulator handling in CombineFramesOperationBase and fading operation classes, aligning with the [MemberNotNull] contract introduced here.
  • Time Compression Average #64: Adds IsResultReady checks and accumulator readiness validation in CombineFramesOperationBase, complementing the nullability guarantees established by the [MemberNotNull] attribute on PrepareFrame.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Crossfade slideshow effect' directly reflects a major addition to the codebase: a new Crossfade feature with supporting infrastructure changes across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch features/Crossfade

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@PhotoLocator/VideoTransformCommands.cs`:
- Around line 692-693: The Crossfade case is inconsistently handled: ProcessArgs
skips adding the scale filter for Crossfade but StabilizeArgs still adds scale,
causing mismatched vidstab transforms; update the logic in the StabilizeArgs
construction (the code path that contains filters.Add($"scale={ScaleTo}") and
related use of IsScaleChecked, SelectedEffect.Content, ZoomEffect and Crossfade)
to mirror ProcessArgs by skipping adding the scale filter when
SelectedEffect.Content == Crossfade (i.e., only append $"scale={ScaleTo}" when
IsScaleChecked is true AND SelectedEffect.Content is neither ZoomEffect nor
Crossfade).

Comment on lines +692 to 693
if (IsScaleChecked && SelectedEffect.Content != ZoomEffect && SelectedEffect.Content != Crossfade)
filters.Add($"scale={ScaleTo}");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep vidstab detect/transform scale filters consistent for Crossfade.
ProcessArgs now skips scale for Crossfade, but StabilizeArgs still applies it, which can yield incorrect transforms when stabilization is enabled.

🔧 Suggested fix
-                if (IsScaleChecked && SelectedEffect.Content != ZoomEffect)
+                if (IsScaleChecked && SelectedEffect.Content != ZoomEffect && SelectedEffect.Content != Crossfade)
                     filters.Add($"scale={ScaleTo}");
🤖 Prompt for AI Agents
In `@PhotoLocator/VideoTransformCommands.cs` around lines 692 - 693, The Crossfade
case is inconsistently handled: ProcessArgs skips adding the scale filter for
Crossfade but StabilizeArgs still adds scale, causing mismatched vidstab
transforms; update the logic in the StabilizeArgs construction (the code path
that contains filters.Add($"scale={ScaleTo}") and related use of IsScaleChecked,
SelectedEffect.Content, ZoomEffect and Crossfade) to mirror ProcessArgs by
skipping adding the scale filter when SelectedEffect.Content == Crossfade (i.e.,
only append $"scale={ScaleTo}" when IsScaleChecked is true AND
SelectedEffect.Content is neither ZoomEffect nor Crossfade).

@meesoft meesoft merged commit 81e1efe into main Jan 25, 2026
5 checks passed
@meesoft meesoft deleted the features/Crossfade branch January 25, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants