diff --git a/src/Avalonia.Base/Media/Imaging/Bitmap.cs b/src/Avalonia.Base/Media/Imaging/Bitmap.cs index dc1541414b..87ef0a234f 100644 --- a/src/Avalonia.Base/Media/Imaging/Bitmap.cs +++ b/src/Avalonia.Base/Media/Imaging/Bitmap.cs @@ -196,32 +196,63 @@ namespace Avalonia.Media.Imaging return sourceRect; } - private protected unsafe void CopyPixelsCore(PixelRect sourceRect, IntPtr buffer, int bufferSize, int stride, - ILockedFramebuffer fb) + /// + /// Performs a row-by-row copy of pixels from a source buffer into a destination buffer. + /// + /// + /// is signed and may be negative: a negative value means the + /// source rows are laid out bottom-up, with pointing at the + /// first (top) row. The destination must be positive and at least the + /// tightly-packed row size. The caller is responsible for validating + /// against the source bounds (e.g. via ). + /// + internal static unsafe void CopyPixelsCore(PixelRect sourceRect, IntPtr sourceAddress, int sourceRowBytes, + PixelFormat sourceFormat, IntPtr buffer, int bufferSize, int stride) { - if (Format == null) - throw new NotSupportedException("CopyPixels is not supported for this bitmap type"); - - sourceRect = ValidateSourceRect(sourceRect); - - int minStride = checked(((sourceRect.Width * fb.Format.BitsPerPixel) + 7) / 8); + int minStride = checked(((sourceRect.Width * sourceFormat.BitsPerPixel) + 7) / 8); if (stride < minStride) throw new ArgumentOutOfRangeException(nameof(stride)); - var minBufferSize = stride * sourceRect.Height; + // 64-bit to avoid overflowing the guard for very large strides/heights, which would + // otherwise let an oversized contiguous blit/loop run past the buffers. + var minBufferSize = (long)stride * sourceRect.Height; if (minBufferSize > bufferSize) throw new ArgumentOutOfRangeException(nameof(bufferSize)); - var offsetX = checked(((sourceRect.X * Format.Value.BitsPerPixel) + 7) / 8); + var offsetX = checked(((sourceRect.X * sourceFormat.BitsPerPixel) + 7) / 8); + + // Fast-path: when the source and destination layouts are identical, tightly-packed and + // forward (no row padding, no X offset, positive stride), the whole region is contiguous in + // both buffers and can be copied with a single blit. This is meaningfully faster than the + // per-row loop (up to ~5x for small images, ~30% for large ones). Requiring stride == minStride + // also guarantees we don't read past the source's last row. + if (offsetX == 0 && sourceRowBytes == stride && stride == minStride) + { + Unsafe.CopyBlock(buffer.ToPointer(), + (sourceAddress + sourceRowBytes * sourceRect.Y).ToPointer(), (uint)minBufferSize); + return; + } for (var y = 0; y < sourceRect.Height; y++) { - var srcAddress = fb.Address + fb.RowBytes * (sourceRect.Y + y) + offsetX; + var srcAddress = sourceAddress + sourceRowBytes * (sourceRect.Y + y) + offsetX; var dstAddress = buffer + stride * y; Unsafe.CopyBlock(dstAddress.ToPointer(), srcAddress.ToPointer(), (uint)minStride); } } + /// + /// Validates against this bitmap and copies pixels out of the + /// given framebuffer. Self-contained convenience wrapper around the static + /// for inheritors. + /// + private protected void CopyPixelsCore(PixelRect sourceRect, IntPtr buffer, int bufferSize, int stride, + ILockedFramebuffer fb) + { + sourceRect = ValidateSourceRect(sourceRect); + CopyPixelsCore(sourceRect, fb.Address, fb.RowBytes, fb.Format, buffer, bufferSize, stride); + } + public virtual void CopyPixels(PixelRect sourceRect, IntPtr buffer, int bufferSize, int stride) { if ( @@ -235,7 +266,7 @@ namespace Avalonia.Media.Imaging if (_isTranscoded) throw new NotSupportedException("CopyPixels is not supported for transcoded bitmaps"); - + using (var fb = readable.Lock()) CopyPixelsCore(sourceRect, buffer, bufferSize, stride, fb); } diff --git a/src/Skia/Avalonia.Skia/ImmutableBitmap.cs b/src/Skia/Avalonia.Skia/ImmutableBitmap.cs index 44408a5333..c168394768 100644 --- a/src/Skia/Avalonia.Skia/ImmutableBitmap.cs +++ b/src/Skia/Avalonia.Skia/ImmutableBitmap.cs @@ -131,18 +131,26 @@ namespace Avalonia.Skia /// Data pixels. public ImmutableBitmap(PixelSize size, Vector dpi, int stride, PixelFormat format, AlphaFormat alphaFormat, IntPtr data) { - using (var tmp = new SKBitmap()) + var info = new SKImageInfo(size.Width, size.Height, format.ToSkColorType(), alphaFormat.ToSkAlphaType()); + + _bitmap = new SKBitmap(); + if (!_bitmap.TryAllocPixels(info)) { - tmp.InstallPixels( - new SKImageInfo(size.Width, size.Height, format.ToSkColorType(), alphaFormat.ToSkAlphaType()), - data, stride); - _bitmap = tmp.Copy(); + _bitmap.Dispose(); + throw new ArgumentException("Unable to create bitmap from provided data"); } + + // Our CopyPixels is 6-15x faster than SKBitmap.Copy(), which internally spins up an + // SKCanvas and draws the source bitmap by assigning it as a shader on an SKPaint + Bitmap.CopyPixelsCore(new PixelRect(size), data, stride, format, _bitmap.GetPixels(), + _bitmap.RowBytes * size.Height, _bitmap.RowBytes); + _bitmap.SetImmutable(); _image = SKImage.FromBitmap(_bitmap); if (_image == null) { + _bitmap.Dispose(); throw new ArgumentException("Unable to create bitmap from provided data"); } diff --git a/tests/Avalonia.Skia.UnitTests/Avalonia.Skia.UnitTests.csproj b/tests/Avalonia.Skia.UnitTests/Avalonia.Skia.UnitTests.csproj index 1edcbfa5aa..5ee6d2b787 100644 --- a/tests/Avalonia.Skia.UnitTests/Avalonia.Skia.UnitTests.csproj +++ b/tests/Avalonia.Skia.UnitTests/Avalonia.Skia.UnitTests.csproj @@ -2,6 +2,7 @@ $(AvsCurrentTargetFramework) Exe + true diff --git a/tests/Avalonia.Skia.UnitTests/Media/ImmutableBitmapTests.cs b/tests/Avalonia.Skia.UnitTests/Media/ImmutableBitmapTests.cs new file mode 100644 index 0000000000..34fc7dadd5 --- /dev/null +++ b/tests/Avalonia.Skia.UnitTests/Media/ImmutableBitmapTests.cs @@ -0,0 +1,68 @@ +using System; +using System.Runtime.InteropServices; +using Avalonia.Platform; +using Xunit; + +namespace Avalonia.Skia.UnitTests.Media +{ + public class ImmutableBitmapTests + { + [Theory] + [InlineData(1, 1, false)] + [InlineData(3, 5, false)] + [InlineData(64, 64, false)] + [InlineData(1, 1, true)] + [InlineData(3, 5, true)] + [InlineData(64, 64, true)] + public unsafe void Constructor_From_Pixels_Copies_Source_Data(int width, int height, bool negativeStride) + { + var size = new PixelSize(width, height); + var rowBytes = width * 4; + var absStride = rowBytes; + var byteSize = absStride * height; + + // Logical pixel byte: deterministic function of (row, byteIndexWithinRow). + byte Expected(int row, int x) => (byte)((row * rowBytes + x) * 7 + 1); + + var source = Marshal.AllocHGlobal(byteSize); + try + { + // Lay the logical rows out in physical memory. For a negative stride the rows are stored + // bottom-up and the data pointer addresses the first (top) logical row, which sits at the + // highest address. + var buffer = new Span((void*)source, byteSize); + for (var row = 0; row < height; row++) + { + var physicalRow = negativeStride ? height - 1 - row : row; + for (var x = 0; x < rowBytes; x++) + buffer[physicalRow * absStride + x] = Expected(row, x); + } + + var stride = negativeStride ? -absStride : absStride; + var data = negativeStride ? source + absStride * (height - 1) : source; + + using var bitmap = new ImmutableBitmap( + size, new Vector(96, 96), stride, + PixelFormat.Bgra8888, AlphaFormat.Premul, data); + + // The constructor must take its own copy: corrupting (and freeing) the source + // afterwards must not affect the bitmap's pixels. + buffer.Fill(0xCD); + + Assert.Equal(size, bitmap.PixelSize); + + using var locked = bitmap.Lock(); + Assert.Equal(size, locked.Size); + + var dst = new ReadOnlySpan((void*)locked.Address, locked.RowBytes * height); + for (var row = 0; row < height; row++) + for (var x = 0; x < rowBytes; x++) + Assert.Equal(Expected(row, x), dst[row * locked.RowBytes + x]); + } + finally + { + Marshal.FreeHGlobal(source); + } + } + } +}