diff --git a/CHANGELOG.md b/CHANGELOG.md index 517764067..3374f766c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Windows.Forms] Updated ImageToolbox UI to consistently use "image" (not "picture"). - [SIL.Windows.Forms.Keyboarding] Removed Timer-based deferred IME conversion status restore from WindowsKeyboardSwitchingAdapter, which disrupted active Chinese Pinyin IME compositions (LT-22442). Added diagnostic tracing for keyboard switching and IME state. - [SIL.DictionaryServices] Fix memory leak in LiftWriter +- [SIL.Windows.Forms] ImageCropper now unsubscribes from `Application.Idle` when disposed, fixing a handler leak that could fire on a disposed cropper +- [SIL.Windows.Forms] Fixed ImageCropper `Image` setter leaking the previous temp file and cropping image on re-set; fields are now nulled after disposal so a mid-setter failure does not leave disposed-but-non-null references +- [SIL.Windows.Forms] Fixed ImageCropper not downscaling tall images before cropping (height condition was checking width) +- [SIL.Windows.Forms] Fixed ImageCropper `GetCroppedImage` throwing `NullReferenceException` when `Image` property is set directly rather than via `SetImage` - [SIL.WritingSystems] Fix IetfLanguageTag.GetGeneralCode to handle cases when zh-CN or zh-TW is a prefix and not the whole string. - [SIL.WritingSystems] More fixes to consistently use 繁体中文 and 简体中文 for Traditional and Simplified Chinese native language names, and Chinese (Traditional) and Chinese (Simplified) for their English names. - [SIL.Windows.Forms] Prevent BetterLabel from responding to OnTextChanged when it has been disposed. diff --git a/SIL.Windows.Forms.Tests/ImageToolbox/ImageCropperTests.cs b/SIL.Windows.Forms.Tests/ImageToolbox/ImageCropperTests.cs new file mode 100644 index 000000000..794b068bc --- /dev/null +++ b/SIL.Windows.Forms.Tests/ImageToolbox/ImageCropperTests.cs @@ -0,0 +1,110 @@ +using System.Drawing; +using System.Drawing.Imaging; +using System.IO; +using System.Threading; +using NUnit.Framework; +using SIL.IO; +using SIL.Windows.Forms.ImageToolbox; +using SIL.Windows.Forms.ImageToolbox.Cropping; + +namespace SIL.Windows.Forms.Tests.ImageToolbox +{ + [Apartment(ApartmentState.STA)] + [TestFixture] + public class ImageCropperTests + { + [Test] + public void Dispose_CalledTwice_DoesNotThrow() + { + var cropper = new ImageCropper(); + Assert.DoesNotThrow(() => cropper.Dispose()); + Assert.DoesNotThrow(() => cropper.Dispose()); + } + + [Test] + public void GetCroppedImage_PngImage_ReturnsUsableBitmap() + { + using (var tempFile = TempFile.WithExtension(".png")) + { + using (var bmp = new Bitmap(100, 80)) + bmp.Save(tempFile.Path, ImageFormat.Png); + + using (var palasoImage = PalasoImage.FromFile(tempFile.Path)) + using (var cropper = new ImageCropper()) + { + cropper.Size = new Size(400, 300); + cropper.SetImage(palasoImage); + + using (var result = cropper.GetCroppedImage()) + { + Assert.IsNotNull(result); + Assert.Greater(result.Width, 0); + // Re-encode to force GDI+ to re-read the pixel data from the backing store. + using (var ms = new MemoryStream()) + Assert.DoesNotThrow(() => result.Save(ms, ImageFormat.Png)); + } + } + } + } + + [Test] + public void GetCroppedImage_JpegImage_SetViaPropertyDirectly_ReturnsJpegBitmap() + { + // Setting Image directly (not via SetImage) must still initialize _originalFormat so + // GetCroppedImage does not throw NullReferenceException on JPEG re-encoding. + using (var tempFile = TempFile.WithExtension(".jpg")) + { + using (var bmp = new Bitmap(100, 80)) + bmp.Save(tempFile.Path, ImageFormat.Jpeg); + + using (var palasoImage = PalasoImage.FromFile(tempFile.Path)) + using (var cropper = new ImageCropper()) + { + cropper.Size = new Size(400, 300); + cropper.Image = palasoImage; // bypass SetImage intentionally + + using (var result = cropper.GetCroppedImage()) + { + Assert.IsNotNull(result); + Assert.AreEqual(ImageFormat.Jpeg.Guid, result.RawFormat.Guid); + Assert.Greater(result.Width, 0); + // Re-encode to force GDI+ to re-read the pixel data. The JPEG path returns an + // image built from a MemoryStream that GetCroppedImage disposes, so this guards + // against that disposal corrupting the returned bitmap. + using (var ms = new MemoryStream()) + Assert.DoesNotThrow(() => result.Save(ms, ImageFormat.Jpeg)); + } + } + } + } + + [Test] + public void SetImage_Reassign_UpdatesFormatAndDoesNotThrow() + { + using (var tempFile1 = TempFile.WithExtension(".png")) + using (var tempFile2 = TempFile.WithExtension(".jpg")) + { + using (var bmp = new Bitmap(100, 80)) + { + bmp.Save(tempFile1.Path, ImageFormat.Png); + bmp.Save(tempFile2.Path, ImageFormat.Jpeg); + } + + using (var img1 = PalasoImage.FromFile(tempFile1.Path)) + using (var img2 = PalasoImage.FromFile(tempFile2.Path)) + using (var cropper = new ImageCropper()) + { + cropper.Size = new Size(400, 300); + cropper.SetImage(img1); + cropper.SetImage(img2); + + using (var result = cropper.GetCroppedImage()) + { + Assert.IsNotNull(result); + Assert.AreEqual(ImageFormat.Jpeg.Guid, result.RawFormat.Guid); + } + } + } + } + } +} diff --git a/SIL.Windows.Forms/ImageToolbox/Cropping/ImageCropper.cs b/SIL.Windows.Forms/ImageToolbox/Cropping/ImageCropper.cs index 4ded520a6..eccfb08c8 100644 --- a/SIL.Windows.Forms/ImageToolbox/Cropping/ImageCropper.cs +++ b/SIL.Windows.Forms/ImageToolbox/Cropping/ImageCropper.cs @@ -145,6 +145,12 @@ public PalasoImage Image if (value == null) return; + _savedOriginalImage?.Dispose(); + _savedOriginalImage = null; + _croppingImage?.Dispose(); + _croppingImage = null; + _originalFormat = value.Image.RawFormat; + //other code changes the image of this palaso image, at which time the PI disposes of its copy, //so we better keep our own. @@ -153,7 +159,7 @@ public PalasoImage Image value.Image.Save(_savedOriginalImage.Path, ImageFormat.Png); // make a reasonable sized copy to crop - if ((value.Image.Width > 1000) || (value.Image.Width > 1000)) + if ((value.Image.Width > 1000) || (value.Image.Height > 1000)) { _croppingImage = CreateCroppingImage(value.Image.Height, value.Image.Width); @@ -267,6 +273,8 @@ private void ImageCropper_Resize(object sender, EventArgs e) private void CalculateSourceImageArea() { + if (_croppingImage == null) + return; float imageToCanvaseScaleFactor = GetImageToCanvasScaleFactor(_croppingImage); _sourceImageArea = new Rectangle(GripThickness, GripThickness, (int)(_croppingImage.Width*imageToCanvaseScaleFactor), @@ -424,8 +432,9 @@ public Image GetCroppedImage() using (var stream = new MemoryStream()) { cropped.Save(stream, ImageFormat.Jpeg); + stream.Position = 0; var oldCropped = cropped; - cropped = System.Drawing.Image.FromStream(stream) as Bitmap; + cropped = (Bitmap)System.Drawing.Image.FromStream(stream); oldCropped.Dispose(); Require.That(ImageFormat.Jpeg.Guid == cropped.RawFormat.Guid, "lost jpeg formatting"); } @@ -449,7 +458,6 @@ public void SetImage(PalasoImage image) } else { - _originalFormat = image.Image.RawFormat; Image = image; } } @@ -482,6 +490,8 @@ protected override void Dispose(bool disposing) components = null; } + Application.Idle -= Application_Idle; + try { if (_savedOriginalImage != null)