Skip to content

Commit a72b308

Browse files
geevensinghCopilot
andcommitted
Render added/deleted images full-canvas (image diff polish for #9)
When a binary image change has only one side (added or deleted file), the SideBySide grid was rendering the existing bitmap into only its half of the canvas, leaving the other column showing bare checkerboard -- visually it looked like `ImageDiffView` was broken. The Swipe and Onion-skin overlays had the same problem in reverse: their compositing slots were silently filled with nothing, and the slider controls did nothing visible. Three-mode compositing has no meaningful semantics when one side is absent, so the view now switches to a single-image layout for those cases: - `ImageDiffViewModel.HasBothImages` flips false when either `LeftImage` or `RightImage` is null. - `ImageDiffViewModel.SingleImage` returns whichever side has a bitmap (right for added, left for deleted), used by the new XAML. - `ImageDiffView` shows a single full-canvas `Image` bound to `SingleImage` when `!HasBothImages`, and hides the bottom slider strip (neither swipe-divider nor onion-opacity does anything useful with one side missing). - `DiffPaneViewModel.ShowImageDiffModeControls` gates the toolbar's image-mode radio group; it goes false for single-sided changes so the user isn't presented with three radio buttons that all visually do the same thing. Header strip ("(added) -> 192 x 192 / 4.34 KB" / "... -> (deleted)") keeps working unchanged, so the user still sees which scenario this is. Tests: - 7 new VM tests (`HasBothImages_*`, `SingleImage_*`) using `[StaFact]` for the bitmap-bearing cases via the same `BitmapSource.Create` pattern as `DiffPaneViewModelTests`. - Extended `LoadAsync_BinaryImage_WithDecoder_DispatchesToImageDiff` to assert `ShowImageDiffModeControls` flips true in the both- sides case. Verification: `dotnet build -c Release` -> 0 warnings; `dotnet test` -> 1154 passed, 1 skipped. AI-Local-Session: bd41845c-9c2c-4d85-9096-da614c7d2662 AI-Cloud-Session: 6c878301-7f09-4cfd-80b6-5e34c328c217 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e5443c1 commit a72b308

6 files changed

Lines changed: 171 additions & 53 deletions

File tree

DiffViewer.Tests/ViewModels/DiffPaneViewModelTests.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ await RunOnUiSyncContextAsync(async () =>
9595

9696
vm.ImageDiff.Should().NotBeNull();
9797
vm.ShowImageDiff.Should().BeTrue();
98+
vm.ShowImageDiffModeControls.Should().BeTrue(
99+
"both sides decoded so the three-mode UI applies");
98100
vm.PlaceholderMessage.Should().BeNull();
99101
vm.ShowPlaceholder.Should().BeFalse();
100102
vm.ShowEditors.Should().BeFalse();

DiffViewer.Tests/ViewModels/ImageDiffViewModelTests.cs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
using System.ComponentModel;
2+
using System.Windows.Media;
3+
using System.Windows.Media.Imaging;
24
using DiffViewer.Models;
35
using DiffViewer.ViewModels;
46
using FluentAssertions;
57
using Xunit;
8+
using ImageMetadata = DiffViewer.Models.ImageMetadata;
69

710
namespace DiffViewer.Tests.ViewModels;
811

@@ -184,4 +187,69 @@ public void LeftAndRightImage_AreExposedFromConstructor()
184187
vm.LeftMetadata.Should().NotBeNull();
185188
vm.RightMetadata.Should().NotBeNull();
186189
}
190+
191+
// ---- HasBothImages / SingleImage gates ----
192+
//
193+
// STA needed because BitmapSource.Create touches WPF imaging
194+
// services. Mirrors the helper in DiffPaneViewModelTests.
195+
196+
private static BitmapSource MakeFrozenBitmap()
197+
{
198+
var bmp = BitmapSource.Create(
199+
pixelWidth: 1,
200+
pixelHeight: 1,
201+
dpiX: 96,
202+
dpiY: 96,
203+
pixelFormat: PixelFormats.Bgra32,
204+
palette: null,
205+
pixels: new byte[] { 0, 0, 0, 255 },
206+
stride: 4);
207+
bmp.Freeze();
208+
return bmp;
209+
}
210+
211+
[Fact]
212+
public void HasBothImages_BothNull_IsFalse()
213+
=> new ImageDiffViewModel(null, null, null, null).HasBothImages.Should().BeFalse();
214+
215+
[StaFact]
216+
public void HasBothImages_BothPresent_IsTrue()
217+
{
218+
var vm = new ImageDiffViewModel(MakeFrozenBitmap(), MakeFrozenBitmap(), null, null);
219+
vm.HasBothImages.Should().BeTrue();
220+
}
221+
222+
[StaFact]
223+
public void HasBothImages_AddOnly_IsFalse()
224+
{
225+
var vm = new ImageDiffViewModel(null, MakeFrozenBitmap(), null, null);
226+
vm.HasBothImages.Should().BeFalse();
227+
}
228+
229+
[StaFact]
230+
public void HasBothImages_DeleteOnly_IsFalse()
231+
{
232+
var vm = new ImageDiffViewModel(MakeFrozenBitmap(), null, null, null);
233+
vm.HasBothImages.Should().BeFalse();
234+
}
235+
236+
[Fact]
237+
public void SingleImage_BothNull_IsNull()
238+
=> new ImageDiffViewModel(null, null, null, null).SingleImage.Should().BeNull();
239+
240+
[StaFact]
241+
public void SingleImage_AddOnly_ReturnsRight()
242+
{
243+
var right = MakeFrozenBitmap();
244+
var vm = new ImageDiffViewModel(null, right, null, null);
245+
vm.SingleImage.Should().BeSameAs(right);
246+
}
247+
248+
[StaFact]
249+
public void SingleImage_DeleteOnly_ReturnsLeft()
250+
{
251+
var left = MakeFrozenBitmap();
252+
var vm = new ImageDiffViewModel(left, null, null, null);
253+
vm.SingleImage.Should().BeSameAs(left);
254+
}
187255
}

DiffViewer/ViewModels/DiffPaneViewModel.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,13 @@ private readonly record struct LoadSignature(
160160
internal const string BinaryPlaceholderMessage = "Binary file - diff not displayed.";
161161

162162
public bool ShowImageDiff => ImageDiff is not null;
163+
/// <summary>
164+
/// Gates the toolbar's image-mode radio group: visible only when an
165+
/// image is dispatched <em>and</em> both sides have a bitmap. For
166+
/// add-only / delete-only changes the modes don't apply (nothing
167+
/// to compose against), so the group is hidden.
168+
/// </summary>
169+
public bool ShowImageDiffModeControls => ImageDiff is not null && ImageDiff.HasBothImages;
163170
public bool ShowPlaceholder => PlaceholderMessage is not null && ImageDiff is null;
164171
public bool ShowEditors => PlaceholderMessage is null && ImageDiff is null;
165172
public bool ShowSideBySide => ShowEditors && IsSideBySide;
@@ -825,6 +832,7 @@ partial void OnPlaceholderMessageChanged(string? value)
825832
partial void OnImageDiffChanged(ImageDiffViewModel? value)
826833
{
827834
OnPropertyChanged(nameof(ShowImageDiff));
835+
OnPropertyChanged(nameof(ShowImageDiffModeControls));
828836
OnPropertyChanged(nameof(ShowPlaceholder));
829837
OnPropertyChanged(nameof(ShowEditors));
830838
OnPropertyChanged(nameof(ShowSideBySide));

DiffViewer/ViewModels/ImageDiffViewModel.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,25 @@ public sealed partial class ImageDiffViewModel : ObservableObject
5353
public ImageMetadata? LeftMetadata { get; }
5454
public ImageMetadata? RightMetadata { get; }
5555

56+
/// <summary>
57+
/// <c>true</c> when both sides decoded into a real bitmap, so the
58+
/// three-mode (SideBySide / Swipe / OnionSkin) UI has something to
59+
/// compose. <c>false</c> for add-only / delete-only changes, where
60+
/// the view should render the single available image full-canvas
61+
/// and hide the mode controls — the three modes have no meaningful
62+
/// semantics when one side is absent.
63+
/// </summary>
64+
public bool HasBothImages => LeftImage is not null && RightImage is not null;
65+
66+
/// <summary>
67+
/// The single image to render when <see cref="HasBothImages"/> is
68+
/// <c>false</c>: <see cref="RightImage"/> for an added file or
69+
/// <see cref="LeftImage"/> for a deleted file. <c>null</c> when
70+
/// neither side has a bitmap (only possible in tests, since
71+
/// production never constructs such a VM).
72+
/// </summary>
73+
public BitmapSource? SingleImage => LeftImage ?? RightImage;
74+
5675
public ImageDiffViewModel(
5776
BitmapSource? leftImage,
5877
BitmapSource? rightImage,

DiffViewer/Views/DiffToolbarView.xaml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,12 @@
244244
</StackPanel>
245245

246246
<!-- Image-mode radio group: visible only when an image is
247-
dispatched. RadioButtons bind to ImageDiff.Mode via the
248-
shared EnumToBoolConverter pattern. -->
247+
dispatched AND both sides have a bitmap. For add-only /
248+
delete-only changes the three modes don't apply (nothing
249+
to compose), so the group is hidden. RadioButtons bind
250+
to ImageDiff.Mode via the shared EnumToBoolConverter. -->
249251
<StackPanel Orientation="Horizontal"
250-
Visibility="{Binding ShowImageDiff, Converter={x:Static vm:BoolToVisibilityConverter.Instance}}">
252+
Visibility="{Binding ShowImageDiffModeControls, Converter={x:Static vm:BoolToVisibilityConverter.Instance}}">
251253
<RadioButton Style="{StaticResource ToolbarRadioStyle}"
252254
IsChecked="{Binding ImageDiff.Mode, Converter={x:Static vm:EnumToBoolConverter.Instance}, ConverterParameter={x:Static models:ImageDiffMode.SideBySide}}"
253255
Content="Side-by-side"

DiffViewer/Views/ImageDiffView.xaml

Lines changed: 69 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -56,70 +56,89 @@
5656
Foreground="#333"/>
5757
</Border>
5858

59-
<!-- Image canvas (checkerboard background). Three mutually
60-
exclusive layout overlays, switched by Mode. -->
59+
<!-- Image canvas (checkerboard background). Either three
60+
mutually exclusive layout overlays (HasBothImages = true,
61+
switched by Mode) or a single full-canvas image for
62+
add-only / delete-only changes. -->
6163
<Border Grid.Row="1" Background="{StaticResource CheckerboardBrush}">
6264
<Grid>
63-
<!-- SideBySide: two columns with a thin gap. -->
64-
<Grid Visibility="{Binding Mode, Converter={x:Static vm:EnumToVisibilityConverter.Instance}, ConverterParameter={x:Static models:ImageDiffMode.SideBySide}}">
65-
<Grid.ColumnDefinitions>
66-
<ColumnDefinition Width="*"/>
67-
<ColumnDefinition Width="2"/>
68-
<ColumnDefinition Width="*"/>
69-
</Grid.ColumnDefinitions>
70-
<Image Grid.Column="0"
71-
Source="{Binding LeftImage}"
72-
Stretch="Uniform"
73-
Margin="4"/>
74-
<Border Grid.Column="1" Background="#DDD"/>
75-
<Image Grid.Column="2"
76-
Source="{Binding RightImage}"
77-
Stretch="Uniform"
78-
Margin="4"/>
79-
</Grid>
65+
<!-- Single-image fallback for add-only / delete-only
66+
changes. Renders whichever side has a bitmap full-
67+
canvas with Stretch=Uniform; the three modes have
68+
no meaningful semantics when one side is absent. -->
69+
<Image Source="{Binding SingleImage}"
70+
Stretch="Uniform"
71+
Margin="4"
72+
Visibility="{Binding HasBothImages, Converter={x:Static vm:FalseToVisibleConverter.Instance}}"/>
8073

81-
<!-- Swipe: both images stacked, left image clipped to the
82-
area left of the draggable divider. Clip + divider
83-
position are updated from code-behind in response to
84-
SizeChanged + SwipePosition / Mode property changes. -->
85-
<Grid x:Name="SwipeCanvas"
86-
Background="Transparent"
87-
Visibility="{Binding Mode, Converter={x:Static vm:EnumToVisibilityConverter.Instance}, ConverterParameter={x:Static models:ImageDiffMode.Swipe}}">
88-
<Image Source="{Binding RightImage}" Stretch="Uniform"/>
89-
<Image x:Name="SwipeLeftImage"
90-
Source="{Binding LeftImage}"
91-
Stretch="Uniform">
92-
<Image.Clip>
93-
<RectangleGeometry x:Name="SwipeLeftClip" Rect="0,0,0,0"/>
94-
</Image.Clip>
95-
</Image>
96-
<Line x:Name="SwipeDividerLine"
97-
Stroke="White"
98-
StrokeThickness="2"
99-
Y1="0" Y2="0"
100-
X1="0" X2="0"
101-
IsHitTestVisible="False"/>
102-
</Grid>
74+
<!-- Three-mode compositing container, gated on
75+
HasBothImages so it stays Collapsed when single-
76+
sided (the Swipe code-behind also early-outs when
77+
SwipeCanvas has zero size). -->
78+
<Grid Visibility="{Binding HasBothImages, Converter={x:Static vm:BoolToVisibilityConverter.Instance}}">
79+
<!-- SideBySide: two columns with a thin gap. -->
80+
<Grid Visibility="{Binding Mode, Converter={x:Static vm:EnumToVisibilityConverter.Instance}, ConverterParameter={x:Static models:ImageDiffMode.SideBySide}}">
81+
<Grid.ColumnDefinitions>
82+
<ColumnDefinition Width="*"/>
83+
<ColumnDefinition Width="2"/>
84+
<ColumnDefinition Width="*"/>
85+
</Grid.ColumnDefinitions>
86+
<Image Grid.Column="0"
87+
Source="{Binding LeftImage}"
88+
Stretch="Uniform"
89+
Margin="4"/>
90+
<Border Grid.Column="1" Background="#DDD"/>
91+
<Image Grid.Column="2"
92+
Source="{Binding RightImage}"
93+
Stretch="Uniform"
94+
Margin="4"/>
95+
</Grid>
10396

104-
<!-- OnionSkin: left image full opacity, right image overlay
105-
with user-controllable opacity. -->
106-
<Grid Visibility="{Binding Mode, Converter={x:Static vm:EnumToVisibilityConverter.Instance}, ConverterParameter={x:Static models:ImageDiffMode.OnionSkin}}">
107-
<Image Source="{Binding LeftImage}" Stretch="Uniform"/>
108-
<Image Source="{Binding RightImage}"
109-
Stretch="Uniform"
110-
Opacity="{Binding OnionOpacity}"/>
97+
<!-- Swipe: both images stacked, left image clipped to the
98+
area left of the draggable divider. Clip + divider
99+
position are updated from code-behind in response to
100+
SizeChanged + SwipePosition / Mode property changes. -->
101+
<Grid x:Name="SwipeCanvas"
102+
Background="Transparent"
103+
Visibility="{Binding Mode, Converter={x:Static vm:EnumToVisibilityConverter.Instance}, ConverterParameter={x:Static models:ImageDiffMode.Swipe}}">
104+
<Image Source="{Binding RightImage}" Stretch="Uniform"/>
105+
<Image x:Name="SwipeLeftImage"
106+
Source="{Binding LeftImage}"
107+
Stretch="Uniform">
108+
<Image.Clip>
109+
<RectangleGeometry x:Name="SwipeLeftClip" Rect="0,0,0,0"/>
110+
</Image.Clip>
111+
</Image>
112+
<Line x:Name="SwipeDividerLine"
113+
Stroke="White"
114+
StrokeThickness="2"
115+
Y1="0" Y2="0"
116+
X1="0" X2="0"
117+
IsHitTestVisible="False"/>
118+
</Grid>
119+
120+
<!-- OnionSkin: left image full opacity, right image overlay
121+
with user-controllable opacity. -->
122+
<Grid Visibility="{Binding Mode, Converter={x:Static vm:EnumToVisibilityConverter.Instance}, ConverterParameter={x:Static models:ImageDiffMode.OnionSkin}}">
123+
<Image Source="{Binding LeftImage}" Stretch="Uniform"/>
124+
<Image Source="{Binding RightImage}"
125+
Stretch="Uniform"
126+
Opacity="{Binding OnionOpacity}"/>
127+
</Grid>
111128
</Grid>
112129
</Grid>
113130
</Border>
114131

115132
<!-- Bottom controls row. Slider is mode-specific: Swipe uses the
116133
whole-canvas divider position; OnionSkin uses the right-side
117-
opacity. SideBySide gets nothing. -->
134+
opacity. SideBySide gets nothing. Hidden entirely for
135+
single-sided changes since neither slider does anything. -->
118136
<Border Grid.Row="2"
119137
Background="#F8F8F8"
120138
BorderBrush="#DDD"
121139
BorderThickness="0,1,0,0"
122-
Padding="12,6">
140+
Padding="12,6"
141+
Visibility="{Binding HasBothImages, Converter={x:Static vm:BoolToVisibilityConverter.Instance}}">
123142
<Grid>
124143
<StackPanel Orientation="Horizontal"
125144
Visibility="{Binding Mode, Converter={x:Static vm:EnumToVisibilityConverter.Instance}, ConverterParameter={x:Static models:ImageDiffMode.Swipe}}">

0 commit comments

Comments
 (0)