Fix ImageCropper returning a crop backed by a disposed stream (breaking: crop is no longer JPEG)

[imnasnainaec]

Warning

Breaking change. GetCroppedImage no longer returns a JPEG-encoded bitmap for JPEG sources; it returns a stand-alone bitmap whose RawFormat is MemoryBmp. Callers that read RawFormat, or that call Image.Save(path) on the result and rely on GDI+ picking the JPEG encoder implicitly, must now pass an explicit ImageFormat — a .jpg path would otherwise receive PNG bytes. Going through PalasoImage.Save(path) is unaffected, since it picks the encoder from the file extension. ImageToolbox's own use goes through PalasoImage, so nothing in this repo changes behaviour.

GetCroppedImage re-encodes a JPEG crop through a MemoryStream and returns what Image.FromStream produces. That stream sits in a using, so it is disposed before the bitmap is returned, and GDI+ decodes lazily — the crop refers to a stream that is already gone. The next thing to touch its pixels fails with "A generic error occurred in GDI+".

That is #1275: choose an image from a file, Crop, Choose, Crop. Leaving the Crop tab calls GetImage, which stores the stream-backed crop on the PalasoImage; returning re-enters the Image setter, which calls value.Image.Save(...) on that crop and throws. Only JPEG sources take the re-encode path, matching the report's "happens for most but not all".

The change

Return a stand-alone new Bitmap(cropped) instead of the re-encoded one, so nothing holds a lazy reference to a stream or a file. Preserving the JPEG RawFormat is what required the re-encode in the first place, so giving it up is what makes the fix possible — hence the breaking change above.

With the re-encode gone _originalFormat has no remaining use, so it goes too, along with the Require.That self-check, the SIL.Code import, and the comments describing the old workaround.

Removing _originalFormat fixes a second bug on the way. It was assigned only in SetImage, so assigning the Image property directly left it null, and GetCroppedImage threw a NullReferenceException reading _originalFormat.Guid — swallowed by the catch and surfaced as "Sorry, there was a problem getting the image".

ShowToolboxWith_PreExisting_EnsureRawFormatUnchanged is deleted: it is [Explicit("By hand only")] so it never ran in CI, and it asserted exactly the JPEG RawFormat this change gives up.

Tests

Three of the four added tests fail without the change:

• GetCroppedImage_JpegImage_ReturnsUsableBitmap — re-encoding the returned crop throws.
• GetCroppedImage_ImageSetViaPropertyDirectly_ReturnsUsableBitmap — the null _originalFormat NRE.
• GetImage_ReCropPreviouslyCroppedJpeg_DoesNotThrow — the Crash in ImageToolbox switching between Crop and Choose #1275 round trip.

GetCroppedImage_PngImage_ReturnsUsableBitmap passes without the change too, since PNG never took the re-encode path; it is there so both formats are covered symmetrically.

--filter ImageToolbox is green on net8.0-windows and net48: 49 passed, 1 pre-existing skip.

Fixes #1275

---

This change is

[github-actions]

Palaso Tests

     4 files  ±0       4 suites  ±0   10m 10s ⏱️ -23s
 5 122 tests +3   4 889 ✅ + 4  233 💤  - 1  0 ❌ ±0 
16 678 runs  +9  15 960 ✅ +12  718 💤  - 3  0 ❌ ±0 

Results for commit 6e07840. ± Comparison against base commit a8bc5a7.

This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both.

SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolboxWith_PreExisting_EnsureRawFormatUnchanged

SIL.Windows.Forms.Tests.ImageToolbox.ImageCropperTests ‑ GetCroppedImage_ImageSetViaPropertyDirectly_ReturnsUsableBitmap
SIL.Windows.Forms.Tests.ImageToolbox.ImageCropperTests ‑ GetCroppedImage_JpegImage_ReturnsUsableBitmap
SIL.Windows.Forms.Tests.ImageToolbox.ImageCropperTests ‑ GetCroppedImage_PngImage_ReturnsUsableBitmap
SIL.Windows.Forms.Tests.ImageToolbox.ImageCropperTests ‑ GetImage_ReCropPreviouslyCroppedJpeg_DoesNotThrow

♻️ This comment has been updated with latest results.

[imnasnainaec]

@andrew-polk What's your take on the breaking change of this pr? (Don't worry about doing code review... not worthwhile unless and until we decide it's a worthwhile change.)