From 89ce060c1153c200d0e7811ccca9818c7b829d75 Mon Sep 17 00:00:00 2001 From: Ben Olden-Cooligan Date: Thu, 9 Mar 2023 18:45:46 -0800 Subject: [PATCH] Twain: Handle image data with mismatching dimensions --- .../Scan/TwainImageProcessorTests.cs | 383 ++++++++++++++++++ .../Scan/TwainMemoryBufferReaderTests.cs | 2 +- .../Internal/Twain/TwainImageProcessor.cs | 73 +++- .../Internal/Twain/TwainMemoryBufferReader.cs | 1 + .../Internal/Twain/TwainProgressEstimator.cs | 6 +- .../Scan/Internal/Twain/TwainScanDriver.cs | 1 + 6 files changed, 449 insertions(+), 17 deletions(-) create mode 100644 NAPS2.Sdk.Tests/Scan/TwainImageProcessorTests.cs diff --git a/NAPS2.Sdk.Tests/Scan/TwainImageProcessorTests.cs b/NAPS2.Sdk.Tests/Scan/TwainImageProcessorTests.cs new file mode 100644 index 000000000..d4e6780ba --- /dev/null +++ b/NAPS2.Sdk.Tests/Scan/TwainImageProcessorTests.cs @@ -0,0 +1,383 @@ +using Google.Protobuf; +using Moq; +using NAPS2.Remoting.Worker; +using NAPS2.Scan; +using NAPS2.Scan.Internal; +using NAPS2.Scan.Internal.Twain; +using NAPS2.Sdk.Tests.Asserts; +using NTwain.Data; +using Xunit; + +namespace NAPS2.Sdk.Tests.Scan; + +public class TwainImageProcessorTests : ContextualTests +{ + private static readonly (int, int, int) RED = (0xFF, 0, 0); + private static readonly (int, int, int) GREEN = (0, 0xFF, 0); + private static readonly (int, int, int) BLUE = (0, 0, 0xFF); + private static readonly (int, int, int) WHITE = (0xFF, 0xFF, 0xFF); + private static readonly (int, int, int) BLACK = (0, 0, 0); + private static readonly (int, int, int) GRAY = (0x80, 0x80, 0x80); + private static readonly (int, int, int) LIGHT_GRAY = (0xD3, 0xD3, 0xD3); + + private readonly Mock _scanEvents; + private readonly Mock> _callback; + private readonly TwainImageProcessor _processor; + private readonly List _images; + + public TwainImageProcessorTests() + { + _scanEvents = new Mock(); + _callback = new Mock>(); + + _images = new List(); + _callback.Setup(x => x(It.IsAny())) + .Callback((IMemoryImage image) => _images.Add(image)); + + _processor = new TwainImageProcessor(ScanningContext, new ScanOptions(), _scanEvents.Object, _callback.Object); + } + + [Fact] + public void SingleImageSingleBuffer() + { + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(2, 2) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0xFF, 0x00, 0x00, + 0x00, 0xFF, 0x00, + 0x00, 0x00, + 0x00, 0x00, 0xFF, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 2, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.Flush(); + + Assert.Single(_images); + ImageAsserts.PixelColors(_images[0], new() + { + { (0, 0), RED }, + { (1, 0), GREEN }, + { (0, 1), BLUE }, + { (1, 1), WHITE }, + }); + } + + [Fact] + public void SingleImageTwoBuffers() + { + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(2, 2) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0xFF, 0x00, 0x00, + 0x00, 0xFF, 0x00, + 0x00, 0x00), + Columns = 2, + Rows = 1, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0x00, 0x00, 0xFF, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 1, + BytesPerRow = 8, + XOffset = 0, + YOffset = 1 + }); + _processor.Flush(); + + Assert.Single(_images); + ImageAsserts.PixelColors(_images[0], new() + { + { (0, 0), RED }, + { (1, 0), GREEN }, + { (0, 1), BLUE }, + { (1, 1), WHITE }, + }); + } + + [Fact] + public void MultipleImages() + { + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(2, 2) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0xFF, 0x00, 0x00, + 0x00, 0xFF, 0x00, + 0x00, 0x00, + 0x00, 0x00, 0xFF, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 2, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(2, 2) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0x00, 0x00, 0x00, + 0x80, 0x80, 0x80, + 0x00, 0x00, + 0xD3, 0xD3, 0xD3, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 2, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.Flush(); + + Assert.Equal(2, _images.Count); + ImageAsserts.PixelColors(_images[0], new() + { + { (0, 0), RED }, + { (1, 0), GREEN }, + { (0, 1), BLUE }, + { (1, 1), WHITE }, + }); + ImageAsserts.PixelColors(_images[1], new() + { + { (0, 0), BLACK }, + { (1, 0), GRAY }, + { (0, 1), LIGHT_GRAY }, + { (1, 1), WHITE }, + }); + } + + [Fact] + public void SingleImageTooSmall() + { + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(3, 3) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0xFF, 0x00, 0x00, + 0x00, 0xFF, 0x00, + 0x00, 0x00, + 0x00, 0x00, 0xFF, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 2, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.Flush(); + + Assert.Single(_images); + Assert.Equal(2, _images[0].Width); + Assert.Equal(2, _images[0].Height); + ImageAsserts.PixelColors(_images[0], new() + { + { (0, 0), RED }, + { (1, 0), GREEN }, + { (0, 1), BLUE }, + { (1, 1), WHITE }, + }); + } + + [Fact] + public void SingleImageTooBig() + { + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(1, 1) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0xFF, 0x00, 0x00, + 0x00, 0xFF, 0x00, + 0x00, 0x00, + 0x00, 0x00, 0xFF, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 2, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.Flush(); + + Assert.Single(_images); + Assert.Equal(2, _images[0].Width); + Assert.Equal(2, _images[0].Height); + ImageAsserts.PixelColors(_images[0], new() + { + { (0, 0), RED }, + { (1, 0), GREEN }, + { (0, 1), BLUE }, + { (1, 1), WHITE }, + }); + } + + [Fact] + public void MultipleImagesWrongSize() + { + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(3, 3) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0xFF, 0x00, 0x00, + 0x00, 0xFF, 0x00, + 0x00, 0x00, + 0x00, 0x00, 0xFF, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 2, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(1, 1) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0x00, 0x00, 0x00, + 0x80, 0x80, 0x80, + 0x00, 0x00, + 0xD3, 0xD3, 0xD3, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 2, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.Flush(); + + Assert.Equal(2, _images.Count); + Assert.Equal(2, _images[0].Width); + Assert.Equal(2, _images[0].Height); + ImageAsserts.PixelColors(_images[0], new() + { + { (0, 0), RED }, + { (1, 0), GREEN }, + { (0, 1), BLUE }, + { (1, 1), WHITE }, + }); + Assert.Equal(2, _images[1].Width); + Assert.Equal(2, _images[1].Height); + ImageAsserts.PixelColors(_images[1], new() + { + { (0, 0), BLACK }, + { (1, 0), GRAY }, + { (0, 1), LIGHT_GRAY }, + { (1, 1), WHITE }, + }); + } + + [Fact] + public void DisposeFlushesWithFullImage() + { + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(2, 2) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0xFF, 0x00, 0x00, + 0x00, 0xFF, 0x00, + 0x00, 0x00, + 0x00, 0x00, 0xFF, + 0xFF, 0xFF, 0xFF, + 0x00, 0x00), + Columns = 2, + Rows = 2, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.Dispose(); + + Assert.Single(_images); + ImageAsserts.PixelColors(_images[0], new() + { + { (0, 0), RED }, + { (1, 0), GREEN }, + { (0, 1), BLUE }, + { (1, 1), WHITE }, + }); + } + + [Fact] + public void DisposeDoesntFlushWithPartialImage() + { + _processor.PageStart(new TwainPageStart + { + ImageData = CreateColorImageData(2, 2) + }); + _processor.MemoryBufferTransferred(new TwainMemoryBuffer + { + Buffer = ByteString.CopyFrom( + 0xFF, 0x00, 0x00, + 0x00, 0xFF, 0x00, + 0x00, 0x00), + Columns = 2, + Rows = 1, + BytesPerRow = 8, + XOffset = 0, + YOffset = 0 + }); + _processor.Dispose(); + + Assert.Empty(_images); + } + + private static TwainImageData CreateColorImageData(int width, int height) + { + return new TwainImageData + { + Height = height, + Width = width, + PixelType = (int) PixelType.RGB, + BitsPerPixel = 24, + BitsPerSample = { 8, 8, 8 }, + SamplesPerPixel = 3 + }; + } +} \ No newline at end of file diff --git a/NAPS2.Sdk.Tests/Scan/TwainMemoryBufferReaderTests.cs b/NAPS2.Sdk.Tests/Scan/TwainMemoryBufferReaderTests.cs index 2cc00c1e3..307f07c07 100644 --- a/NAPS2.Sdk.Tests/Scan/TwainMemoryBufferReaderTests.cs +++ b/NAPS2.Sdk.Tests/Scan/TwainMemoryBufferReaderTests.cs @@ -339,7 +339,7 @@ public class TwainMemoryBufferReaderTests : ContextualTests Width = 2, PixelType = (int) PixelType.RGB, BitsPerPixel = 24, - BitsPerSample = { 8, 7, 9 }, + BitsPerSample = { 7, 8, 9 }, SamplesPerPixel = 3 }; var image = Create24BitImage(2, 2); diff --git a/NAPS2.Sdk/Scan/Internal/Twain/TwainImageProcessor.cs b/NAPS2.Sdk/Scan/Internal/Twain/TwainImageProcessor.cs index f84b2eb5a..fb8c1e1e5 100644 --- a/NAPS2.Sdk/Scan/Internal/Twain/TwainImageProcessor.cs +++ b/NAPS2.Sdk/Scan/Internal/Twain/TwainImageProcessor.cs @@ -1,4 +1,5 @@ #if !MAC +using NAPS2.Images.Bitwise; using NAPS2.Remoting.Worker; namespace NAPS2.Scan.Internal.Twain; @@ -12,7 +13,9 @@ internal class TwainImageProcessor : ITwainEvents, IDisposable private readonly ScanningContext _scanningContext; private readonly Action _callback; private TwainImageData? _currentImageData; - private IMemoryImage? _currentMemoryImage; + private IMemoryImage? _currentImage; + private int _transferredWidth; + private int _transferredHeight; private long _transferredPixels; private long _totalPixels; private readonly TwainProgressEstimator _progressEstimator; @@ -27,9 +30,12 @@ internal class TwainImageProcessor : ITwainEvents, IDisposable public void PageStart(TwainPageStart pageStart) { + Flush(); _currentImageData = pageStart.ImageData; - _currentMemoryImage?.Dispose(); - _currentMemoryImage = null; + _currentImage?.Dispose(); + _currentImage = null; + _transferredWidth = 0; + _transferredHeight = 0; _transferredPixels = 0; _totalPixels = _currentImageData == null ? 0 : _currentImageData.Width * (long) _currentImageData.Height; _progressEstimator.MarkStart(_totalPixels); @@ -50,27 +56,68 @@ internal class TwainImageProcessor : ITwainEvents, IDisposable } var pixelFormat = _currentImageData.BitsPerPixel == 1 ? ImagePixelFormat.BW1 : ImagePixelFormat.RGB24; - _currentMemoryImage ??= _scanningContext.ImageContext.Create( + _currentImage ??= _scanningContext.ImageContext.Create( _currentImageData.Width, _currentImageData.Height, pixelFormat); - _currentMemoryImage.SetResolution((float) _currentImageData.XRes, (float) _currentImageData.YRes); + _currentImage.SetResolution((float) _currentImageData.XRes, (float) _currentImageData.YRes); _transferredPixels += memoryBuffer.Columns * (long) memoryBuffer.Rows; + _transferredWidth = Math.Max(_transferredWidth, memoryBuffer.Columns + memoryBuffer.XOffset); + _transferredHeight = Math.Max(_transferredHeight, memoryBuffer.Rows + memoryBuffer.YOffset); - TwainMemoryBufferReader.CopyBufferToImage(memoryBuffer, _currentImageData, _currentMemoryImage); - _progressEstimator.MarkProgress(_transferredPixels, _totalPixels); - - if (_transferredPixels == _totalPixels) + // In case the real image dimensions don't match the specified image dimensions, we may need to get more memory. + // The image will be realloc'd to the real size once we're done and know what that is. + if (_transferredWidth > _currentImage.Width) { + ReallocImage(_currentImage.Width * 2, _currentImage.Height); + } + if (_transferredHeight > _currentImage.Height) + { + ReallocImage(_currentImage.Width, _currentImage.Height * 2); + } + + TwainMemoryBufferReader.CopyBufferToImage(memoryBuffer, _currentImageData, _currentImage); + _progressEstimator.MarkProgress(Math.Min(_transferredPixels, _totalPixels), _totalPixels); + } + + private void ReallocImage(int width, int height) + { + Debug.WriteLine($"NAPS2.TW - Realloc image {_currentImage!.Width}x{_currentImage.Height} -> {width}x{height}"); + var copy = _scanningContext.ImageContext.Create(width, height, _currentImage!.PixelFormat); + new CopyBitwiseImageOp + { + Columns = Math.Min(width, _currentImage.Width), + Rows = Math.Min(height, _currentImage.Height) + }.Perform(_currentImage, copy); + _currentImage.Dispose(); + _currentImage = copy; + } + + public void Flush() + { + if (_currentImage != null && _transferredWidth > 0 && _transferredHeight > 0) + { + if (_transferredWidth != _currentImage.Width || _transferredHeight != _currentImage.Height) + { + // The real image dimensions don't match the specified image dimensions, so we have to realloc. + ReallocImage(_transferredWidth, _transferredHeight); + } _progressEstimator.MarkCompletion(); - // TODO: Throw an error if there's a pixel mismatch, i.e. we go to the next page / finish with too few, or have too many - _callback(_currentMemoryImage); - _currentMemoryImage = null; + _callback(_currentImage); + _currentImage = null; } } public void Dispose() { - _currentMemoryImage?.Dispose(); + if (_currentImage != null && _transferredPixels == _totalPixels && + _transferredWidth == _currentImageData?.Width && _transferredHeight == _currentImageData?.Height) + { + // If we have an error after a successful scan (so Flush isn't called normally) we still want to flush. + // Obviously this won't work if the image dimensions are off (as we can't tell if the scan is complete or + // not) but that should be a rare case. + Flush(); + } + _currentImage?.Dispose(); } } #endif \ No newline at end of file diff --git a/NAPS2.Sdk/Scan/Internal/Twain/TwainMemoryBufferReader.cs b/NAPS2.Sdk/Scan/Internal/Twain/TwainMemoryBufferReader.cs index a1caac541..2f10e2bd8 100644 --- a/NAPS2.Sdk/Scan/Internal/Twain/TwainMemoryBufferReader.cs +++ b/NAPS2.Sdk/Scan/Internal/Twain/TwainMemoryBufferReader.cs @@ -16,6 +16,7 @@ public static class TwainMemoryBufferReader var subPixelType = ((PixelType) imageData.PixelType, imageData.BitsPerPixel, imageData.SamplesPerPixel, imageData.BitsPerSample) switch { + // Technically for RGB we should check for [8, 8, 8, ...] but some scanners only set the first value (PixelType.RGB, 24, 3, [8, ..]) => SubPixelType.Rgb, (PixelType.Gray, 8, 1, [8, ..]) => SubPixelType.Gray, (PixelType.BlackWhite, 1, 1, [1, ..]) => SubPixelType.Bit, diff --git a/NAPS2.Sdk/Scan/Internal/Twain/TwainProgressEstimator.cs b/NAPS2.Sdk/Scan/Internal/Twain/TwainProgressEstimator.cs index 37ad9b1e2..42dc5cc15 100644 --- a/NAPS2.Sdk/Scan/Internal/Twain/TwainProgressEstimator.cs +++ b/NAPS2.Sdk/Scan/Internal/Twain/TwainProgressEstimator.cs @@ -29,7 +29,7 @@ internal class TwainProgressEstimator private static TimingKey GetTimingKey(ScanOptions options) { - return new TimingKey(options.Device!.ID!, options.BitDepth, options.Dpi, options.PageSize!); + return new TimingKey(options.Device?.ID, options.BitDepth, options.Dpi, options.PageSize); } public TwainProgressEstimator(ScanOptions options, IScanEvents scanEvents) @@ -111,7 +111,7 @@ internal class TwainProgressEstimator _pixelsAtFirstBuffer = transferredPixels; } var progress = transferredPixels / (double) totalPixels; - if (_previousTimingInfo != null) + if (_previousTimingInfo != null && _previousTimingInfo.TotalMillis > 0) { var overheadDone = _previousTimingInfo.OverheadMillis / (double) _previousTimingInfo.TotalMillis; var nonOverheadRatio = (_previousTimingInfo.TotalMillis - _previousTimingInfo.OverheadMillis) / @@ -145,7 +145,7 @@ internal class TwainProgressEstimator private readonly Dictionary _cache = new(); } - public record TimingKey(string DeviceId, BitDepth BitDepth, int Dpi, PageSize PageSize); + public record TimingKey(string? DeviceId, BitDepth BitDepth, int Dpi, PageSize? PageSize); public record TimingInfo(long OverheadMillis, long TotalMillis); } \ No newline at end of file diff --git a/NAPS2.Sdk/Scan/Internal/Twain/TwainScanDriver.cs b/NAPS2.Sdk/Scan/Internal/Twain/TwainScanDriver.cs index b71b81999..1e66aade4 100644 --- a/NAPS2.Sdk/Scan/Internal/Twain/TwainScanDriver.cs +++ b/NAPS2.Sdk/Scan/Internal/Twain/TwainScanDriver.cs @@ -36,6 +36,7 @@ internal class TwainScanDriver : IScanDriver var controller = GetSessionController(options); using var state = new TwainImageProcessor(_scanningContext, options, scanEvents, callback); await controller.StartScan(options, state, cancelToken); + state.Flush(); }); }