Преглед изворни кода

Fix Multiple Bugs in Crop Functionality (#284)

Summary
This Pull Request refactors crop management in the PicView Avalonia project, improving code clarity, maintainability, and functionality. It modifies CropLayoutManager, CropResizer, and CropControl.axaml.
Main Changes
CropLayoutManager.cs:
Simplified logic for calculating crop selection dimensions.
Improved logic for setting canvas boundaries.
CropResizer.cs:
Reduced redundant whitespace for consistency.
Added logic to prevent out-of-canvas bounds during resize operations.
Enforced square aspect ratio constraints more effectively.
CropControl.axaml:
Introduced the ClipToBounds property to ensure proper rendering behavior.
F_CIL пре 4 месеци
родитељ
комит
d760f2a525

+ 10 - 22
src/PicView.Avalonia/Crop/CropLayoutManager.cs

@@ -8,7 +8,7 @@ namespace PicView.Avalonia.Crop;
 public class CropLayoutManager(CropControl control)
 public class CropLayoutManager(CropControl control)
 {
 {
     private const int DefaultSelectionSize = 200;
     private const int DefaultSelectionSize = 200;
-    
+
     public void InitializeLayout()
     public void InitializeLayout()
     {
     {
         if (control.DataContext is not MainViewModel vm)
         if (control.DataContext is not MainViewModel vm)
@@ -23,19 +23,15 @@ public class CropLayoutManager(CropControl control)
         }
         }
 
 
         // Set initial width and height for the crop rectangle
         // Set initial width and height for the crop rectangle
-        var pixelWidth = vm.PicViewer.ImageWidth.CurrentValue / vm.PicViewer.AspectRatio.CurrentValue;
-        var pixelHeight = vm.PicViewer.ImageHeight.CurrentValue / vm.PicViewer.AspectRatio.CurrentValue;
+        var originalWidth = vm.PicViewer.ImageWidth.CurrentValue >= DefaultSelectionSize * 2
+            ? DefaultSelectionSize
+            : (uint)(vm.PicViewer.ImageWidth.CurrentValue / 2);
+        var originalHeight = vm.PicViewer.ImageHeight.CurrentValue >= DefaultSelectionSize * 2
+            ? DefaultSelectionSize
+            : (uint)(vm.PicViewer.ImageHeight.CurrentValue / 2);
 
 
-        if (pixelWidth >= DefaultSelectionSize * 2 || pixelHeight >= DefaultSelectionSize * 2)
-        {
-            vm.Crop.SetSelectionWidth(DefaultSelectionSize);
-            vm.Crop.SetSelectionHeight(DefaultSelectionSize);
-        }
-        else if (pixelWidth <= DefaultSelectionSize || pixelHeight <= DefaultSelectionSize)
-        {
-            vm.Crop.SetSelectionWidth((uint)(pixelWidth / 2));
-            vm.Crop.SetSelectionHeight((uint)(pixelHeight / 2));
-        }
+        vm.Crop.SetSelectionWidth(originalWidth);
+        vm.Crop.SetSelectionHeight(originalHeight);
 
 
         // Calculate centered position
         // Calculate centered position
         vm.Crop.SelectionX.Value = Convert.ToInt32((vm.PicViewer.ImageWidth.CurrentValue - vm.Crop.SelectionWidth.CurrentValue) / 2);
         vm.Crop.SelectionX.Value = Convert.ToInt32((vm.PicViewer.ImageWidth.CurrentValue - vm.Crop.SelectionWidth.CurrentValue) / 2);
@@ -202,14 +198,6 @@ public class CropLayoutManager(CropControl control)
         Canvas.SetTop(control.RightMiddleButton, rightMiddleY);
         Canvas.SetTop(control.RightMiddleButton, rightMiddleY);
 
 
         Canvas.SetLeft(control.SizeBorder, topLeftX + control.TopLeftButton.Bounds.Width + 2);
         Canvas.SetLeft(control.SizeBorder, topLeftX + control.TopLeftButton.Bounds.Width + 2);
-
-        if (topLeftY != 0)
-        {
-            Canvas.SetTop(control.SizeBorder, topLeftY - control.TopLeftButton.Bounds.Height);
-        }
-        else
-        {
-            Canvas.SetTop(control.SizeBorder, topLeftY);
-        }
+        Canvas.SetTop(control.SizeBorder, Math.Max(0, topLeftY - control.TopLeftButton.Bounds.Height));
     }
     }
 }
 }

+ 37 - 27
src/PicView.Avalonia/Crop/CropResizer.cs

@@ -15,13 +15,13 @@ public static class CropResizer
     {
     {
         var currentPos = e.GetPosition(control.RootCanvas);
         var currentPos = e.GetPosition(control.RootCanvas);
         var delta = currentPos - resizeStart;
         var delta = currentPos - resizeStart;
-        
+
         // Initialize with original values
         // Initialize with original values
         var newX = originalRect.X;
         var newX = originalRect.X;
         var newY = originalRect.Y;
         var newY = originalRect.Y;
         var newWidth = originalRect.Width;
         var newWidth = originalRect.Width;
         var newHeight = originalRect.Height;
         var newHeight = originalRect.Height;
-        
+
         // Calculate new dimensions based on resize mode
         // Calculate new dimensions based on resize mode
         switch (mode)
         switch (mode)
         {
         {
@@ -31,49 +31,49 @@ public static class CropResizer
                 newWidth -= delta.X;
                 newWidth -= delta.X;
                 newHeight -= delta.Y;
                 newHeight -= delta.Y;
                 break;
                 break;
-                
+
             case CropResizeMode.TopRight:
             case CropResizeMode.TopRight:
                 newY += delta.Y;
                 newY += delta.Y;
                 newWidth += delta.X;
                 newWidth += delta.X;
                 newHeight -= delta.Y;
                 newHeight -= delta.Y;
                 break;
                 break;
-                
+
             case CropResizeMode.BottomLeft:
             case CropResizeMode.BottomLeft:
                 newX += delta.X;
                 newX += delta.X;
                 newWidth -= delta.X;
                 newWidth -= delta.X;
                 newHeight += delta.Y;
                 newHeight += delta.Y;
                 break;
                 break;
-                
+
             case CropResizeMode.BottomRight:
             case CropResizeMode.BottomRight:
                 newWidth += delta.X;
                 newWidth += delta.X;
                 newHeight += delta.Y;
                 newHeight += delta.Y;
                 break;
                 break;
-                
+
             case CropResizeMode.Left:
             case CropResizeMode.Left:
                 newX += delta.X;
                 newX += delta.X;
                 newWidth -= delta.X;
                 newWidth -= delta.X;
                 break;
                 break;
-                
+
             case CropResizeMode.Right:
             case CropResizeMode.Right:
                 newWidth += delta.X;
                 newWidth += delta.X;
                 break;
                 break;
-                
+
             case CropResizeMode.Top:
             case CropResizeMode.Top:
                 newY += delta.Y;
                 newY += delta.Y;
                 newHeight -= delta.Y;
                 newHeight -= delta.Y;
                 break;
                 break;
-                
+
             case CropResizeMode.Bottom:
             case CropResizeMode.Bottom:
                 newHeight += delta.Y;
                 newHeight += delta.Y;
                 break;
                 break;
         }
         }
-        
+
         // Handle aspect ratio constraints (maintain square when Shift is pressed)
         // Handle aspect ratio constraints (maintain square when Shift is pressed)
         if (MainKeyboardShortcuts.ShiftDown && IsCornerMode(mode))
         if (MainKeyboardShortcuts.ShiftDown && IsCornerMode(mode))
         {
         {
             // Use the larger dimension to determine square size
             // Use the larger dimension to determine square size
             var size = Math.Max(newWidth, newHeight);
             var size = Math.Max(newWidth, newHeight);
-            
+
             // Adjust position based on resize mode to maintain the correct anchor point
             // Adjust position based on resize mode to maintain the correct anchor point
             switch (mode)
             switch (mode)
             {
             {
@@ -81,23 +81,23 @@ public static class CropResizer
                     newX = originalRect.X + originalRect.Width - size;
                     newX = originalRect.X + originalRect.Width - size;
                     newY = originalRect.Y + originalRect.Height - size;
                     newY = originalRect.Y + originalRect.Height - size;
                     break;
                     break;
-                    
+
                 case CropResizeMode.TopRight:
                 case CropResizeMode.TopRight:
                     newY = originalRect.Y + originalRect.Height - size;
                     newY = originalRect.Y + originalRect.Height - size;
                     break;
                     break;
-                    
+
                 case CropResizeMode.BottomLeft:
                 case CropResizeMode.BottomLeft:
                     newX = originalRect.X + originalRect.Width - size;
                     newX = originalRect.X + originalRect.Width - size;
                     break;
                     break;
             }
             }
-            
+
             newWidth = size;
             newWidth = size;
             newHeight = size;
             newHeight = size;
         }
         }
-        
+
         // Apply bounds constraints
         // Apply bounds constraints
         ApplyBoundsConstraints(ref newX, ref newY, ref newWidth, ref newHeight, vm.ImageWidth.CurrentValue, vm.ImageHeight.CurrentValue);
         ApplyBoundsConstraints(ref newX, ref newY, ref newWidth, ref newHeight, vm.ImageWidth.CurrentValue, vm.ImageHeight.CurrentValue);
-        
+
         // Update the view model
         // Update the view model
         try
         try
         {
         {
@@ -110,18 +110,18 @@ public static class CropResizer
         {
         {
             DebugHelper.LogDebug(nameof(CropResizer), nameof(Resize), exception);
             DebugHelper.LogDebug(nameof(CropResizer), nameof(Resize), exception);
         }
         }
-        
+
         // Update the rectangle position on canvas
         // Update the rectangle position on canvas
         Canvas.SetLeft(control.MainRectangle, newX);
         Canvas.SetLeft(control.MainRectangle, newX);
         Canvas.SetTop(control.MainRectangle, newY);
         Canvas.SetTop(control.MainRectangle, newY);
     }
     }
-    
-    private static bool IsCornerMode(CropResizeMode mode) => mode is 
-        CropResizeMode.TopLeft or 
-        CropResizeMode.TopRight or 
-        CropResizeMode.BottomLeft or 
+
+    private static bool IsCornerMode(CropResizeMode mode) => mode is
+        CropResizeMode.TopLeft or
+        CropResizeMode.TopRight or
+        CropResizeMode.BottomLeft or
         CropResizeMode.BottomRight;
         CropResizeMode.BottomRight;
-        
+
     private static void ApplyBoundsConstraints(ref double x, ref double y, ref double width, ref double height, double maxWidth, double maxHeight)
     private static void ApplyBoundsConstraints(ref double x, ref double y, ref double width, ref double height, double maxWidth, double maxHeight)
     {
     {
         // Ensure we don't go beyond canvas bounds
         // Ensure we don't go beyond canvas bounds
@@ -130,23 +130,33 @@ public static class CropResizer
             width += x;
             width += x;
             x = 0;
             x = 0;
         }
         }
-        
+
         if (y < 0)
         if (y < 0)
         {
         {
             height += y;
             height += y;
             y = 0;
             y = 0;
         }
         }
-        
+
+        if (x >= maxWidth)
+        {
+            x = maxWidth - 1;
+        }
+
+        if (y >= maxHeight)
+        {
+            y = maxHeight - 1;
+        }
+
         // Ensure minimum dimensions
         // Ensure minimum dimensions
         width = Math.Max(width, 1);
         width = Math.Max(width, 1);
         height = Math.Max(height, 1);
         height = Math.Max(height, 1);
-        
+
         // Ensure we don't exceed the canvas bounds
         // Ensure we don't exceed the canvas bounds
         if (x + width > maxWidth)
         if (x + width > maxWidth)
         {
         {
             width = maxWidth - x;
             width = maxWidth - x;
         }
         }
-        
+
         if (y + height > maxHeight)
         if (y + height > maxHeight)
         {
         {
             height = maxHeight - y;
             height = maxHeight - y;

+ 2 - 1
src/PicView.Avalonia/Views/UC/CropControl.axaml

@@ -3,7 +3,8 @@
     x:DataType="viewModels:MainViewModel"
     x:DataType="viewModels:MainViewModel"
     xmlns="https://github.com/avaloniaui"
     xmlns="https://github.com/avaloniaui"
     xmlns:viewModels="clr-namespace:PicView.Avalonia.ViewModels"
     xmlns:viewModels="clr-namespace:PicView.Avalonia.ViewModels"
-    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
+    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
+    ClipToBounds="False">
     <UserControl.Resources>
     <UserControl.Resources>
         <SolidColorBrush Color="{DynamicResource MainTextColor}" x:Key="Brush0" />
         <SolidColorBrush Color="{DynamicResource MainTextColor}" x:Key="Brush0" />
     </UserControl.Resources>
     </UserControl.Resources>