From 57f58fe3cca5f6f3372065ba1c49847a52fae8aa Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 13 Jun 2025 15:04:07 +1000 Subject: [PATCH] Backport #2940 --- .../Webp/Lossless/BackwardReferenceEncoder.cs | 25 +++++------ .../Formats/Webp/Lossless/CostModel.cs | 4 +- .../Formats/Webp/Lossless/HistogramEncoder.cs | 19 ++++---- .../Formats/Webp/Lossless/PixOrCopy.cs | 45 +++++++------------ .../Formats/Webp/Lossless/Vp8LBackwardRefs.cs | 29 +++++++----- .../Formats/Webp/Lossless/Vp8LEncoder.cs | 20 ++++----- .../Formats/Webp/Lossless/Vp8LHistogram.cs | 6 +-- .../Formats/WebP/Vp8LHistogramTests.cs | 12 ++--- 8 files changed, 71 insertions(+), 89 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs b/src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs index 2e7dd722fc..defa63ed06 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs @@ -149,9 +149,8 @@ private static int CalculateBestCacheSize( } // Find the cacheBits giving the lowest entropy. - for (int idx = 0; idx < refs.Refs.Count; idx++) + foreach (PixOrCopy v in refs) { - PixOrCopy v = refs.Refs[idx]; if (v.IsLiteral()) { uint pix = bgra[pos++]; @@ -387,7 +386,7 @@ private static void BackwardReferencesHashChainFollowChosenPath(ReadOnlySpan bgra, int cach { int pixelIndex = 0; ColorCache colorCache = new ColorCache(cacheBits); - for (int idx = 0; idx < refs.Refs.Count; idx++) + foreach (ref PixOrCopy v in refs) { - PixOrCopy v = refs.Refs[idx]; if (v.IsLiteral()) { uint bgraLiteral = v.BgraOrDistance; @@ -790,9 +788,7 @@ private static void BackwardRefsWithLocalCache(ReadOnlySpan bgra, int cach if (ix >= 0) { // Color cache contains bgraLiteral - v.Mode = PixOrCopyMode.CacheIdx; - v.BgraOrDistance = (uint)ix; - v.Len = 1; + v = PixOrCopy.CreateCacheIdx(ix); } else { @@ -814,14 +810,13 @@ private static void BackwardRefsWithLocalCache(ReadOnlySpan bgra, int cach private static void BackwardReferences2DLocality(int xSize, Vp8LBackwardRefs refs) { - using List.Enumerator c = refs.Refs.GetEnumerator(); - while (c.MoveNext()) + foreach (ref PixOrCopy v in refs) { - if (c.Current.IsCopy()) + if (v.IsCopy()) { - int dist = (int)c.Current.BgraOrDistance; + int dist = (int)v.BgraOrDistance; int transformedDist = DistanceToPlaneCode(xSize, dist); - c.Current.BgraOrDistance = (uint)transformedDist; + v = PixOrCopy.CreateCopy((uint)transformedDist, v.Len); } } } diff --git a/src/ImageSharp/Formats/Webp/Lossless/CostModel.cs b/src/ImageSharp/Formats/Webp/Lossless/CostModel.cs index beebc48abc..c6131bc2aa 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/CostModel.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/CostModel.cs @@ -40,9 +40,9 @@ public void Build(int xSize, int cacheBits, Vp8LBackwardRefs backwardRefs) using OwnedVp8LHistogram histogram = OwnedVp8LHistogram.Create(this.memoryAllocator, cacheBits); // The following code is similar to HistogramCreate but converts the distance to plane code. - for (int i = 0; i < backwardRefs.Refs.Count; i++) + foreach (PixOrCopy v in backwardRefs) { - histogram.AddSinglePixOrCopy(backwardRefs.Refs[i], true, xSize); + histogram.AddSinglePixOrCopy(in v, true, xSize); } ConvertPopulationCountTableToBitEstimates(histogram.NumCodes(), histogram.Literal, this.Literal); diff --git a/src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs b/src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs index 3a96362cfd..595332eeef 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs @@ -109,12 +109,11 @@ private static void HistogramBuild( { int x = 0, y = 0; int histoXSize = LosslessUtils.SubSampleSize(xSize, histoBits); - using List.Enumerator backwardRefsEnumerator = backwardRefs.Refs.GetEnumerator(); - while (backwardRefsEnumerator.MoveNext()) + + foreach (PixOrCopy v in backwardRefs) { - PixOrCopy v = backwardRefsEnumerator.Current; int ix = ((y >> histoBits) * histoXSize) + (x >> histoBits); - histograms[ix].AddSinglePixOrCopy(v, false); + histograms[ix].AddSinglePixOrCopy(in v, false); x += v.Len; while (x >= xSize) { @@ -465,7 +464,7 @@ private static bool HistogramCombineStochastic(Vp8LHistogramSet histograms, int } } - HistoListUpdateHead(histoPriorityList, p); + HistoListUpdateHead(histoPriorityList, p, j); j++; } @@ -525,7 +524,7 @@ private static void HistogramCombineGreedy(Vp8LHistogramSet histograms) } else { - HistoListUpdateHead(histoPriorityList, p); + HistoListUpdateHead(histoPriorityList, p, i); i++; } } @@ -647,7 +646,7 @@ private static double HistoPriorityListPush( histoList.Add(pair); - HistoListUpdateHead(histoList, pair); + HistoListUpdateHead(histoList, pair, histoList.Count - 1); return pair.CostDiff; } @@ -674,13 +673,11 @@ private static void HistoListUpdatePair( /// /// Check whether a pair in the list should be updated as head or not. /// - private static void HistoListUpdateHead(List histoList, HistogramPair pair) + private static void HistoListUpdateHead(List histoList, HistogramPair pair, int idx) { if (pair.CostDiff < histoList[0].CostDiff) { - // Replace the best pair. - int oldIdx = histoList.IndexOf(pair); - histoList[oldIdx] = histoList[0]; + histoList[idx] = histoList[0]; histoList[0] = pair; } } diff --git a/src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs b/src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs index d6b10ada55..bb8ce18aad 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs @@ -6,37 +6,24 @@ namespace SixLabors.ImageSharp.Formats.Webp.Lossless; [DebuggerDisplay("Mode: {Mode}, Len: {Len}, BgraOrDistance: {BgraOrDistance}")] -internal sealed class PixOrCopy +internal readonly struct PixOrCopy { - public PixOrCopyMode Mode { get; set; } - - public ushort Len { get; set; } - - public uint BgraOrDistance { get; set; } - - public static PixOrCopy CreateCacheIdx(int idx) => - new PixOrCopy - { - Mode = PixOrCopyMode.CacheIdx, - BgraOrDistance = (uint)idx, - Len = 1 - }; - - public static PixOrCopy CreateLiteral(uint bgra) => - new PixOrCopy - { - Mode = PixOrCopyMode.Literal, - BgraOrDistance = bgra, - Len = 1 - }; - - public static PixOrCopy CreateCopy(uint distance, ushort len) => - new PixOrCopy + public readonly PixOrCopyMode Mode; + public readonly ushort Len; + public readonly uint BgraOrDistance; + + private PixOrCopy(PixOrCopyMode mode, ushort len, uint bgraOrDistance) { - Mode = PixOrCopyMode.Copy, - BgraOrDistance = distance, - Len = len - }; + this.Mode = mode; + this.Len = len; + this.BgraOrDistance = bgraOrDistance; + } + + public static PixOrCopy CreateCacheIdx(int idx) => new(PixOrCopyMode.CacheIdx, 1, (uint)idx); + + public static PixOrCopy CreateLiteral(uint bgra) => new(PixOrCopyMode.Literal, 1, bgra); + + public static PixOrCopy CreateCopy(uint distance, ushort len) => new(PixOrCopyMode.Copy, len, distance); public int Literal(int component) => (int)(this.BgraOrDistance >> (component * 8)) & 0xFF; diff --git a/src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs b/src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs index ace9d62271..634fac5e82 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs @@ -1,21 +1,28 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Buffers; +using SixLabors.ImageSharp.Memory; + namespace SixLabors.ImageSharp.Formats.Webp.Lossless; -internal class Vp8LBackwardRefs +internal class Vp8LBackwardRefs : IDisposable { - public Vp8LBackwardRefs(int pixels) => this.Refs = new List(pixels); + private readonly IMemoryOwner refs; + private int count; + + public Vp8LBackwardRefs(MemoryAllocator memoryAllocator, int pixels) + { + this.refs = memoryAllocator.Allocate(pixels); + this.count = 0; + } + + public void Add(PixOrCopy pixOrCopy) => this.refs.Memory.Span[this.count++] = pixOrCopy; - /// - /// Gets or sets the common block-size. - /// - public int BlockSize { get; set; } + public void Clear() => this.count = 0; - /// - /// Gets the backward references. - /// - public List Refs { get; } + public Span.Enumerator GetEnumerator() => this.refs.Slice(0, this.count).GetEnumerator(); - public void Add(PixOrCopy pixOrCopy) => this.Refs.Add(pixOrCopy); + /// + public void Dispose() => this.refs.Dispose(); } diff --git a/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs b/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs index 0351798b5f..0ef110bb9f 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs @@ -137,14 +137,9 @@ public Vp8LEncoder( this.Refs = new Vp8LBackwardRefs[3]; this.HashChain = new Vp8LHashChain(memoryAllocator, pixelCount); - // We round the block size up, so we're guaranteed to have at most MaxRefsBlockPerImage blocks used: - int refsBlockSize = ((pixelCount - 1) / MaxRefsBlockPerImage) + 1; for (int i = 0; i < this.Refs.Length; i++) { - this.Refs[i] = new Vp8LBackwardRefs(pixelCount) - { - BlockSize = refsBlockSize < MinBlockSize ? MinBlockSize : refsBlockSize - }; + this.Refs[i] = new Vp8LBackwardRefs(memoryAllocator, pixelCount); } } @@ -1071,9 +1066,8 @@ private void StoreImageToBitMask( int histogramIx = histogramSymbols[0]; Span codes = huffmanCodes.AsSpan(5 * histogramIx); - for (int i = 0; i < backwardRefs.Refs.Count; i++) + foreach (PixOrCopy v in backwardRefs) { - PixOrCopy v = backwardRefs.Refs[i]; if (tileX != (x & tileMask) || tileY != (y & tileMask)) { tileX = x & tileMask; @@ -1907,9 +1901,9 @@ public void AllocateTransformBuffer(int width, int height) /// public void ClearRefs() { - foreach (Vp8LBackwardRefs t in this.Refs) + foreach (Vp8LBackwardRefs refs in this.Refs) { - t.Refs.Clear(); + refs.Clear(); } } @@ -1921,6 +1915,12 @@ public void Dispose() this.BgraScratch?.Dispose(); this.Palette.Dispose(); this.TransformData?.Dispose(); + + foreach (Vp8LBackwardRefs refs in this.Refs) + { + refs.Dispose(); + } + this.HashChain.Dispose(); } diff --git a/src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs b/src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs index f473977908..03bedfe672 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs @@ -138,9 +138,9 @@ public void Clear() /// The backward references. public void StoreRefs(Vp8LBackwardRefs refs) { - for (int i = 0; i < refs.Refs.Count; i++) + foreach (PixOrCopy v in refs) { - this.AddSinglePixOrCopy(refs.Refs[i], false); + this.AddSinglePixOrCopy(in v, false); } } @@ -150,7 +150,7 @@ public void StoreRefs(Vp8LBackwardRefs refs) /// The token to add. /// Indicates whether to use the distance modifier. /// xSize is only used when useDistanceModifier is true. - public void AddSinglePixOrCopy(PixOrCopy v, bool useDistanceModifier, int xSize = 0) + public void AddSinglePixOrCopy(in PixOrCopy v, bool useDistanceModifier, int xSize = 0) { if (v.IsLiteral()) { diff --git a/tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs b/tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs index cfe79e49e6..7777c61084 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs @@ -66,18 +66,14 @@ private static void RunAddVectorTest() // All remaining values are expected to be zero. literals.AsSpan().CopyTo(expectedLiterals); - Vp8LBackwardRefs backwardRefs = new(pixelData.Length); + MemoryAllocator memoryAllocator = Configuration.Default.MemoryAllocator; + + using Vp8LBackwardRefs backwardRefs = new(memoryAllocator, pixelData.Length); for (int i = 0; i < pixelData.Length; i++) { - backwardRefs.Add(new PixOrCopy() - { - BgraOrDistance = pixelData[i], - Len = 1, - Mode = PixOrCopyMode.Literal - }); + backwardRefs.Add(PixOrCopy.CreateLiteral(pixelData[i])); } - MemoryAllocator memoryAllocator = Configuration.Default.MemoryAllocator; using OwnedVp8LHistogram histogram0 = OwnedVp8LHistogram.Create(memoryAllocator, backwardRefs, 3); using OwnedVp8LHistogram histogram1 = OwnedVp8LHistogram.Create(memoryAllocator, backwardRefs, 3); for (int i = 0; i < 5; i++)