From 7c90385d8df7bbf9469fc468480b26ebb134abd8 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Mon, 15 Aug 2022 19:13:24 -0400 Subject: [PATCH 01/17] Pre-calculate the strings for log levels. --- src/SMAPI/Framework/Monitor.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/SMAPI/Framework/Monitor.cs b/src/SMAPI/Framework/Monitor.cs index 6b53daff..8ba175e6 100644 --- a/src/SMAPI/Framework/Monitor.cs +++ b/src/SMAPI/Framework/Monitor.cs @@ -25,7 +25,9 @@ namespace StardewModdingAPI.Framework private readonly LogFileManager LogFile; /// The maximum length of the values. - private static readonly int MaxLevelLength = (from level in Enum.GetValues(typeof(LogLevel)).Cast() select level.ToString().Length).Max(); + private static readonly int MaxLevelLength = (from level in Enum.GetValues() select level.ToString().Length).Max(); + + private static readonly Dictionary LogStrings = Enum.GetValues().ToDictionary(k => k, v => v.ToString().ToUpper().PadRight(MaxLevelLength)); /// A cache of messages that should only be logged once. private readonly HashSet LogOnceCache = new(); @@ -147,7 +149,7 @@ namespace StardewModdingAPI.Framework /// The log level. private string GenerateMessagePrefix(string source, ConsoleLogLevel level) { - string levelStr = level.ToString().ToUpper().PadRight(Monitor.MaxLevelLength); + string levelStr = LogStrings[level]; int? playerIndex = this.GetScreenIdForLog(); return $"[{DateTime.Now:HH:mm:ss} {levelStr}{(playerIndex != null ? $" screen_{playerIndex}" : "")} {source}]"; From 78643710ce09197dbb5505fd8cc2361c8ada0830 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Mon, 15 Aug 2022 19:13:39 -0400 Subject: [PATCH 02/17] Use array pools in editing images. --- .../Framework/Content/AssetDataForImage.cs | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 3393b22f..98d6725a 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; using System.Diagnostics.CodeAnalysis; using Microsoft.Xna.Framework; using Microsoft.Xna.Framework.Graphics; @@ -47,44 +48,55 @@ namespace StardewModdingAPI.Framework.Content int areaHeight = sourceArea.Value.Height; if (areaX == 0 && areaY == 0 && areaWidth == source.Width && areaHeight == source.Height) + { sourceData = source.Data; + this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + } else { - sourceData = new Color[areaWidth * areaHeight]; - int i = 0; - for (int y = areaY, maxY = areaY + areaHeight - 1; y <= maxY; y++) + int pixelCount = areaWidth * areaHeight; + sourceData = ArrayPool.Shared.Rent(pixelCount); + + for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) { - for (int x = areaX, maxX = areaX + areaWidth - 1; x <= maxX; x++) - { - int targetIndex = (y * source.Width) + x; - sourceData[i++] = source.Data[targetIndex]; - } + // avoiding an variable that increments allows the processor to re-arrange here. + int sourceIndex = (y * source.Width) + areaX; + int targetIndex = (y - areaY) * areaWidth; + Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); } + + // apply + this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + + // return + ArrayPool.Shared.Return(sourceData); } } - - // apply - this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); } /// public void PatchImage(Texture2D source, Rectangle? sourceArea = null, Rectangle? targetArea = null, PatchMode patchMode = PatchMode.Replace) { + // validate + if (source == null) + throw new ArgumentNullException(nameof(source), "Can't patch from a null source texture."); + this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); // validate source texture - if (source == null) - throw new ArgumentNullException(nameof(source), "Can't patch from a null source texture."); if (!source.Bounds.Contains(sourceArea.Value)) throw new ArgumentOutOfRangeException(nameof(sourceArea), "The source area is outside the bounds of the source texture."); // get source data int pixelCount = sourceArea.Value.Width * sourceArea.Value.Height; - Color[] sourceData = GC.AllocateUninitializedArray(pixelCount); + Color[] sourceData = ArrayPool.Shared.Rent(pixelCount); source.GetData(0, sourceArea, sourceData, 0, pixelCount); // apply this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + + // return + ArrayPool.Shared.Return(sourceData); } /// @@ -143,7 +155,7 @@ namespace StardewModdingAPI.Framework.Content if (patchMode == PatchMode.Overlay) { // get target data - Color[] mergedData = GC.AllocateUninitializedArray(pixelCount); + Color[] mergedData = ArrayPool.Shared.Rent(pixelCount); target.GetData(0, targetArea, mergedData, 0, pixelCount); // merge pixels @@ -175,6 +187,7 @@ namespace StardewModdingAPI.Framework.Content } target.SetData(0, targetArea, mergedData, 0, pixelCount); + ArrayPool.Shared.Return(mergedData); } else target.SetData(0, targetArea, sourceData, 0, pixelCount); From 4a1055e573e9d8b0aa654238889596be07c29193 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Tue, 16 Aug 2022 15:30:21 -0400 Subject: [PATCH 03/17] arraypool in the modcontentmanager, a bit of fussing --- .../Framework/Content/AssetDataForImage.cs | 15 ++++--- .../ContentManagers/ModContentManager.cs | 40 ++++++++++++------- .../Framework/ModLoading/AssemblyLoader.cs | 2 +- src/SMAPI/Framework/Monitor.cs | 7 +++- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 98d6725a..46c2a22e 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -33,12 +33,12 @@ namespace StardewModdingAPI.Framework.Content /// public void PatchImage(IRawTextureData source, Rectangle? sourceArea = null, Rectangle? targetArea = null, PatchMode patchMode = PatchMode.Replace) { - this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); - - // validate source data + // nullcheck if (source == null) throw new ArgumentNullException(nameof(source), "Can't patch from null source data."); + this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); + // get the pixels for the source area Color[] sourceData; { @@ -59,7 +59,6 @@ namespace StardewModdingAPI.Framework.Content for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) { - // avoiding an variable that increments allows the processor to re-arrange here. int sourceIndex = (y * source.Width) + areaX; int targetIndex = (y - areaY) * areaWidth; Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); @@ -77,13 +76,13 @@ namespace StardewModdingAPI.Framework.Content /// public void PatchImage(Texture2D source, Rectangle? sourceArea = null, Rectangle? targetArea = null, PatchMode patchMode = PatchMode.Replace) { - // validate + // nullcheck if (source == null) throw new ArgumentNullException(nameof(source), "Can't patch from a null source texture."); this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); - // validate source texture + // validate source bounds if (!source.Bounds.Contains(sourceArea.Value)) throw new ArgumentOutOfRangeException(nameof(sourceArea), "The source area is outside the bounds of the source texture."); @@ -161,8 +160,8 @@ namespace StardewModdingAPI.Framework.Content // merge pixels for (int i = 0; i < pixelCount; i++) { - Color above = sourceData[i]; - Color below = mergedData[i]; + ref Color above = ref sourceData[i]; + ref Color below = ref mergedData[i]; // shortcut transparency if (above.A < MinOpacity) diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index cc6f8372..dd30c225 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -1,9 +1,11 @@ using System; +using System.Buffers; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using BmFont; using Microsoft.Xna.Framework; using Microsoft.Xna.Framework.Content; @@ -111,7 +113,7 @@ namespace StardewModdingAPI.Framework.ContentManagers if (this.Coordinator.TryParseManagedAssetKey(assetName.Name, out string? contentManagerID, out IAssetName? relativePath)) { if (contentManagerID != this.Name) - throw this.GetLoadError(assetName, ContentLoadErrorType.AccessDenied, "can't load a different mod's managed asset key through this mod content manager."); + this.ThrowLoadError(assetName, ContentLoadErrorType.AccessDenied, "can't load a different mod's managed asset key through this mod content manager."); assetName = relativePath; } } @@ -123,7 +125,7 @@ namespace StardewModdingAPI.Framework.ContentManagers // get file FileInfo file = this.GetModFile(assetName.Name); if (!file.Exists) - throw this.GetLoadError(assetName, ContentLoadErrorType.AssetDoesNotExist, "the specified path doesn't exist."); + this.ThrowLoadError(assetName, ContentLoadErrorType.AssetDoesNotExist, "the specified path doesn't exist."); // load content asset = file.Extension.ToLower() switch @@ -141,7 +143,8 @@ namespace StardewModdingAPI.Framework.ContentManagers if (ex is SContentLoadException) throw; - throw this.GetLoadError(assetName, ContentLoadErrorType.Other, "an unexpected error occurred.", ex); + this.ThrowLoadError(assetName, ContentLoadErrorType.Other, "an unexpected error occurred.", ex); + return default; } // track & return asset @@ -189,7 +192,7 @@ namespace StardewModdingAPI.Framework.ContentManagers private T LoadDataFile(IAssetName assetName, FileInfo file) { if (!this.JsonHelper.ReadJsonFileIfExists(file.FullName, out T? asset)) - throw this.GetLoadError(assetName, ContentLoadErrorType.InvalidData, "the JSON file is invalid."); // should never happen since we check for file existence before calling this method + this.ThrowLoadError(assetName, ContentLoadErrorType.InvalidData, "the JSON file is invalid."); // should never happen since we check for file existence before calling this method return asset; } @@ -301,7 +304,7 @@ namespace StardewModdingAPI.Framework.ContentManagers private T LoadXnbFile(IAssetName assetName) { if (typeof(IRawTextureData).IsAssignableFrom(typeof(T))) - throw this.GetLoadError(assetName, ContentLoadErrorType.Other, $"can't read XNB file as type {typeof(IRawTextureData)}; that type can only be read from a PNG file."); + this.ThrowLoadError(assetName, ContentLoadErrorType.Other, $"can't read XNB file as type {typeof(IRawTextureData)}; that type can only be read from a PNG file."); // the underlying content manager adds a .xnb extension implicitly, so // we need to strip it here to avoid trying to load a '.xnb.xnb' file. @@ -326,7 +329,8 @@ namespace StardewModdingAPI.Framework.ContentManagers /// The file to load. private T HandleUnknownFileType(IAssetName assetName, FileInfo file) { - throw this.GetLoadError(assetName, ContentLoadErrorType.InvalidName, $"unknown file extension '{file.Extension}'; must be one of '.fnt', '.json', '.png', '.tbin', '.tmx', or '.xnb'."); + this.ThrowLoadError(assetName, ContentLoadErrorType.InvalidName, $"unknown file extension '{file.Extension}'; must be one of '.fnt', '.json', '.png', '.tbin', '.tmx', or '.xnb'."); + return default; } /// Assert that the asset type is compatible with one of the allowed types. @@ -338,18 +342,20 @@ namespace StardewModdingAPI.Framework.ContentManagers private void AssertValidType(IAssetName assetName, FileInfo file, params Type[] validTypes) { if (!validTypes.Any(validType => validType.IsAssignableFrom(typeof(TAsset)))) - throw this.GetLoadError(assetName, ContentLoadErrorType.InvalidData, $"can't read file with extension '{file.Extension}' as type '{typeof(TAsset)}'; must be type '{string.Join("' or '", validTypes.Select(p => p.FullName))}'."); + this.ThrowLoadError(assetName, ContentLoadErrorType.InvalidData, $"can't read file with extension '{file.Extension}' as type '{typeof(TAsset)}'; must be type '{string.Join("' or '", validTypes.Select(p => p.FullName))}'."); } - /// Get an error which indicates that an asset couldn't be loaded. + /// Throws an error which indicates that an asset couldn't be loaded. /// Why loading an asset through the content pipeline failed. /// The asset name that failed to load. /// The reason the file couldn't be loaded. /// The underlying exception, if applicable. + [DoesNotReturn] [DebuggerStepThrough, DebuggerHidden] - private SContentLoadException GetLoadError(IAssetName assetName, ContentLoadErrorType errorType, string reasonPhrase, Exception? exception = null) + [MethodImpl(MethodImplOptions.NoInlining)] + private void ThrowLoadError(IAssetName assetName, ContentLoadErrorType errorType, string reasonPhrase, Exception? exception = null) { - return new(errorType, $"Failed loading asset '{assetName}' from {this.Name}: {reasonPhrase}", exception); + throw new SContentLoadException(errorType, $"Failed loading asset '{assetName}' from {this.Name}: {reasonPhrase}", exception); } /// Get a file from the mod folder. @@ -384,12 +390,14 @@ namespace StardewModdingAPI.Framework.ContentManagers private Texture2D PremultiplyTransparency(Texture2D texture) { // premultiply pixels - Color[] data = GC.AllocateUninitializedArray(texture.Width * texture.Height); - texture.GetData(data); + int count = texture.Width * texture.Height; + Color[] data = ArrayPool.Shared.Rent(count); + texture.GetData(data, 0, count); + bool changed = false; - for (int i = 0; i < data.Length; i++) + for (int i = 0; i < count; i++) { - Color pixel = data[i]; + ref Color pixel = ref data[i]; if (pixel.A is (byte.MinValue or byte.MaxValue)) continue; // no need to change fully transparent/opaque pixels @@ -398,8 +406,10 @@ namespace StardewModdingAPI.Framework.ContentManagers } if (changed) - texture.SetData(data); + texture.SetData(data, 0, count); + // return + ArrayPool.Shared.Return(data); return texture; } diff --git a/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs b/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs index 01037870..ae08d972 100644 --- a/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs +++ b/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs @@ -221,7 +221,7 @@ namespace StardewModdingAPI.Framework.ModLoading /// public static Assembly? ResolveAssembly(string name) { - string shortName = name.Split(new[] { ',' }, 2).First(); // get simple name (without version and culture) + string shortName = name.Split(',', 2).First(); // get simple name (without version and culture) return AppDomain.CurrentDomain .GetAssemblies() .FirstOrDefault(p => p.GetName().Name == shortName); diff --git a/src/SMAPI/Framework/Monitor.cs b/src/SMAPI/Framework/Monitor.cs index 8ba175e6..d33bf259 100644 --- a/src/SMAPI/Framework/Monitor.cs +++ b/src/SMAPI/Framework/Monitor.cs @@ -27,10 +27,13 @@ namespace StardewModdingAPI.Framework /// The maximum length of the values. private static readonly int MaxLevelLength = (from level in Enum.GetValues() select level.ToString().Length).Max(); + /// A mapping of console log levels to their string form. private static readonly Dictionary LogStrings = Enum.GetValues().ToDictionary(k => k, v => v.ToString().ToUpper().PadRight(MaxLevelLength)); + private readonly record struct LogOnceCacheEntry(string message, LogLevel level); + /// A cache of messages that should only be logged once. - private readonly HashSet LogOnceCache = new(); + private readonly HashSet LogOnceCache = new(); /// Get the screen ID that should be logged to distinguish between players in split-screen mode, if any. private readonly Func GetScreenIdForLog; @@ -86,7 +89,7 @@ namespace StardewModdingAPI.Framework /// public void LogOnce(string message, LogLevel level = LogLevel.Trace) { - if (this.LogOnceCache.Add($"{message}|{level}")) + if (this.LogOnceCache.Add(new LogOnceCacheEntry(message, level))) this.LogImpl(this.Source, message, (ConsoleLogLevel)level); } From 581763c36392e28ed6e05690ff84b66da5882e78 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Tue, 16 Aug 2022 16:46:10 -0400 Subject: [PATCH 04/17] Skip math if above is fully opaque. --- src/SMAPI/Framework/Content/AssetDataForImage.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 46c2a22e..ea04f57a 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -160,13 +160,13 @@ namespace StardewModdingAPI.Framework.Content // merge pixels for (int i = 0; i < pixelCount; i++) { - ref Color above = ref sourceData[i]; - ref Color below = ref mergedData[i]; + Color above = sourceData[i]; + Color below = mergedData[i]; // shortcut transparency if (above.A < MinOpacity) continue; - if (below.A < MinOpacity) + if (below.A < MinOpacity || above.A == byte.MaxValue) mergedData[i] = above; // merge pixels From 0a2a1a08def3d97f21b4138d9e39c563389b492a Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Tue, 16 Aug 2022 19:30:20 -0400 Subject: [PATCH 05/17] Favor record structs when there are four or fewer elements. --- .../Framework/Content/AssetDataForImage.cs | 41 +++++++++++++------ .../Framework/Content/AssetEditOperation.cs | 2 +- .../Framework/Content/AssetLoadOperation.cs | 2 +- .../ContentManagers/GameContentManager.cs | 4 +- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index ea04f57a..5016bcd4 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -40,7 +40,7 @@ namespace StardewModdingAPI.Framework.Content this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); // get the pixels for the source area - Color[] sourceData; + Color[] trimmedSourceData; { int areaX = sourceArea.Value.X; int areaY = sourceArea.Value.Y; @@ -49,26 +49,42 @@ namespace StardewModdingAPI.Framework.Content if (areaX == 0 && areaY == 0 && areaWidth == source.Width && areaHeight == source.Height) { - sourceData = source.Data; - this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + trimmedSourceData = source.Data; + this.PatchImageImpl(trimmedSourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); } else { int pixelCount = areaWidth * areaHeight; - sourceData = ArrayPool.Shared.Rent(pixelCount); + trimmedSourceData = ArrayPool.Shared.Rent(pixelCount); - for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) + // shortcut! If I want a horizontal slice of the texture + // I can copy the whole array in one pass + // Likely ~uncommon but Array.Copy significantly benefits + // from being able to do this. + if (areaWidth == source.Width && areaX == 0) { - int sourceIndex = (y * source.Width) + areaX; - int targetIndex = (y - areaY) * areaWidth; - Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); + int sourceIndex = areaY * source.Width; + int targetIndex = 0; + + Array.Copy(source.Data, sourceIndex, trimmedSourceData, targetIndex, pixelCount); + } + else + { + // copying line-by-line + // Array.Copy isn't great at small scale + for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) + { + int sourceIndex = (y * source.Width) + areaX; + int targetIndex = (y - areaY) * areaWidth; + Array.Copy(source.Data, sourceIndex, trimmedSourceData, targetIndex, areaWidth); + } } // apply - this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + this.PatchImageImpl(trimmedSourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); // return - ArrayPool.Shared.Return(sourceData); + ArrayPool.Shared.Return(trimmedSourceData); } } } @@ -160,8 +176,9 @@ namespace StardewModdingAPI.Framework.Content // merge pixels for (int i = 0; i < pixelCount; i++) { - Color above = sourceData[i]; - Color below = mergedData[i]; + // should probably benchmark this... + ref Color above = ref sourceData[i]; + ref Color below = ref mergedData[i]; // shortcut transparency if (above.A < MinOpacity) diff --git a/src/SMAPI/Framework/Content/AssetEditOperation.cs b/src/SMAPI/Framework/Content/AssetEditOperation.cs index 11b8811b..893f59bd 100644 --- a/src/SMAPI/Framework/Content/AssetEditOperation.cs +++ b/src/SMAPI/Framework/Content/AssetEditOperation.cs @@ -8,5 +8,5 @@ namespace StardewModdingAPI.Framework.Content /// If there are multiple edits that apply to the same asset, the priority with which this one should be applied. /// The content pack on whose behalf the edit is being applied, if any. /// Apply the edit to an asset. - internal record AssetEditOperation(IModMetadata Mod, AssetEditPriority Priority, IModMetadata? OnBehalfOf, Action ApplyEdit); + internal readonly record struct AssetEditOperation(IModMetadata Mod, AssetEditPriority Priority, IModMetadata? OnBehalfOf, Action ApplyEdit); } diff --git a/src/SMAPI/Framework/Content/AssetLoadOperation.cs b/src/SMAPI/Framework/Content/AssetLoadOperation.cs index 7af07dfd..58886849 100644 --- a/src/SMAPI/Framework/Content/AssetLoadOperation.cs +++ b/src/SMAPI/Framework/Content/AssetLoadOperation.cs @@ -8,5 +8,5 @@ namespace StardewModdingAPI.Framework.Content /// If there are multiple loads that apply to the same asset, the priority with which this one should be applied. /// The content pack on whose behalf the asset is being loaded, if any. /// Load the initial value for an asset. - internal record AssetLoadOperation(IModMetadata Mod, IModMetadata? OnBehalfOf, AssetLoadPriority Priority, Func GetData); + internal readonly record struct AssetLoadOperation(IModMetadata Mod, IModMetadata? OnBehalfOf, AssetLoadPriority Priority, Func GetData); } diff --git a/src/SMAPI/Framework/ContentManagers/GameContentManager.cs b/src/SMAPI/Framework/ContentManagers/GameContentManager.cs index df7bdc59..a8c70356 100644 --- a/src/SMAPI/Framework/ContentManagers/GameContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/GameContentManager.cs @@ -172,7 +172,7 @@ namespace StardewModdingAPI.Framework.ContentManagers where T : notnull { // find matching loader - AssetLoadOperation? loader = null; + AssetLoadOperation loader = default; if (loadOperations?.Count > 0) { if (!this.AssertMaxOneRequiredLoader(info, loadOperations, out string? error)) @@ -183,7 +183,7 @@ namespace StardewModdingAPI.Framework.ContentManagers loader = loadOperations.OrderByDescending(p => p.Priority).FirstOrDefault(); } - if (loader == null) + if (loader.Mod == null) // aka, this is default. return null; // fetch asset from loader From 627100509c0a9c637b495473ef71f13093e885d2 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Wed, 17 Aug 2022 21:11:47 -0400 Subject: [PATCH 06/17] hide throwhelper from stack trace in dotnet 6 --- src/SMAPI/Framework/ContentManagers/ModContentManager.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index dd30c225..0c793808 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -353,6 +353,9 @@ namespace StardewModdingAPI.Framework.ContentManagers [DoesNotReturn] [DebuggerStepThrough, DebuggerHidden] [MethodImpl(MethodImplOptions.NoInlining)] +#if NET6_0_OR_GREATER + [StackTraceHidden] +#endif private void ThrowLoadError(IAssetName assetName, ContentLoadErrorType errorType, string reasonPhrase, Exception? exception = null) { throw new SContentLoadException(errorType, $"Failed loading asset '{assetName}' from {this.Name}: {reasonPhrase}", exception); From d29c01b8155a6fe2607066da6f0a5cfb45b28d3c Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Thu, 18 Aug 2022 20:51:45 -0400 Subject: [PATCH 07/17] Partially revert "Favor record structs when there are four or fewer elements." This reverts commit f5d49515c4eddfb415903a89d70654cf9b6de299. --- .../Framework/Content/AssetDataForImage.cs | 38 +++++++++---------- .../Framework/Content/AssetEditOperation.cs | 2 +- .../Framework/Content/AssetLoadOperation.cs | 2 +- .../ContentManagers/GameContentManager.cs | 4 +- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 5016bcd4..5f175217 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -40,7 +40,7 @@ namespace StardewModdingAPI.Framework.Content this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); // get the pixels for the source area - Color[] trimmedSourceData; + Color[] sourceData; { int areaX = sourceArea.Value.X; int areaY = sourceArea.Value.Y; @@ -49,42 +49,40 @@ namespace StardewModdingAPI.Framework.Content if (areaX == 0 && areaY == 0 && areaWidth == source.Width && areaHeight == source.Height) { - trimmedSourceData = source.Data; - this.PatchImageImpl(trimmedSourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + sourceData = source.Data; + this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); } else { int pixelCount = areaWidth * areaHeight; - trimmedSourceData = ArrayPool.Shared.Rent(pixelCount); + sourceData = ArrayPool.Shared.Rent(pixelCount); - // shortcut! If I want a horizontal slice of the texture - // I can copy the whole array in one pass - // Likely ~uncommon but Array.Copy significantly benefits - // from being able to do this. - if (areaWidth == source.Width && areaX == 0) + if (areaX == 0 && areaWidth == source.Width) { - int sourceIndex = areaY * source.Width; - int targetIndex = 0; + // shortcut copying because the area to copy is contiguous. This is + // probably uncommon, but Array.Copy benefits a lot. + + int sourceIndex = areaY * areaWidth; + int targetIndex = 0; + Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); - Array.Copy(source.Data, sourceIndex, trimmedSourceData, targetIndex, pixelCount); } else { - // copying line-by-line - // Array.Copy isn't great at small scale + // slower copying, line by line for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) { int sourceIndex = (y * source.Width) + areaX; int targetIndex = (y - areaY) * areaWidth; - Array.Copy(source.Data, sourceIndex, trimmedSourceData, targetIndex, areaWidth); + Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); } } // apply - this.PatchImageImpl(trimmedSourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); // return - ArrayPool.Shared.Return(trimmedSourceData); + ArrayPool.Shared.Return(sourceData); } } } @@ -176,9 +174,9 @@ namespace StardewModdingAPI.Framework.Content // merge pixels for (int i = 0; i < pixelCount; i++) { - // should probably benchmark this... - ref Color above = ref sourceData[i]; - ref Color below = ref mergedData[i]; + // ref locals here? Not sure. + Color above = sourceData[i]; + Color below = mergedData[i]; // shortcut transparency if (above.A < MinOpacity) diff --git a/src/SMAPI/Framework/Content/AssetEditOperation.cs b/src/SMAPI/Framework/Content/AssetEditOperation.cs index 893f59bd..11b8811b 100644 --- a/src/SMAPI/Framework/Content/AssetEditOperation.cs +++ b/src/SMAPI/Framework/Content/AssetEditOperation.cs @@ -8,5 +8,5 @@ namespace StardewModdingAPI.Framework.Content /// If there are multiple edits that apply to the same asset, the priority with which this one should be applied. /// The content pack on whose behalf the edit is being applied, if any. /// Apply the edit to an asset. - internal readonly record struct AssetEditOperation(IModMetadata Mod, AssetEditPriority Priority, IModMetadata? OnBehalfOf, Action ApplyEdit); + internal record AssetEditOperation(IModMetadata Mod, AssetEditPriority Priority, IModMetadata? OnBehalfOf, Action ApplyEdit); } diff --git a/src/SMAPI/Framework/Content/AssetLoadOperation.cs b/src/SMAPI/Framework/Content/AssetLoadOperation.cs index 58886849..7af07dfd 100644 --- a/src/SMAPI/Framework/Content/AssetLoadOperation.cs +++ b/src/SMAPI/Framework/Content/AssetLoadOperation.cs @@ -8,5 +8,5 @@ namespace StardewModdingAPI.Framework.Content /// If there are multiple loads that apply to the same asset, the priority with which this one should be applied. /// The content pack on whose behalf the asset is being loaded, if any. /// Load the initial value for an asset. - internal readonly record struct AssetLoadOperation(IModMetadata Mod, IModMetadata? OnBehalfOf, AssetLoadPriority Priority, Func GetData); + internal record AssetLoadOperation(IModMetadata Mod, IModMetadata? OnBehalfOf, AssetLoadPriority Priority, Func GetData); } diff --git a/src/SMAPI/Framework/ContentManagers/GameContentManager.cs b/src/SMAPI/Framework/ContentManagers/GameContentManager.cs index a8c70356..df7bdc59 100644 --- a/src/SMAPI/Framework/ContentManagers/GameContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/GameContentManager.cs @@ -172,7 +172,7 @@ namespace StardewModdingAPI.Framework.ContentManagers where T : notnull { // find matching loader - AssetLoadOperation loader = default; + AssetLoadOperation? loader = null; if (loadOperations?.Count > 0) { if (!this.AssertMaxOneRequiredLoader(info, loadOperations, out string? error)) @@ -183,7 +183,7 @@ namespace StardewModdingAPI.Framework.ContentManagers loader = loadOperations.OrderByDescending(p => p.Priority).FirstOrDefault(); } - if (loader.Mod == null) // aka, this is default. + if (loader == null) return null; // fetch asset from loader From ff523c619a040c8eb2d2f1c1a0d19c8fe83f1c75 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Fri, 19 Aug 2022 11:10:19 -0400 Subject: [PATCH 08/17] fix fast-track array copying --- src/SMAPI/Framework/Content/AssetDataForImage.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 5f175217..9c71328f 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -64,7 +64,7 @@ namespace StardewModdingAPI.Framework.Content int sourceIndex = areaY * areaWidth; int targetIndex = 0; - Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); + Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, pixelCount); } else From ce63efa2f45ee770fdb628a45f5a6b63544b0031 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Mon, 22 Aug 2022 19:32:45 -0400 Subject: [PATCH 09/17] Avoid making copy if the source image is just taller than the sourceArea. --- src/SMAPI/Framework/Content/AssetDataForImage.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 9c71328f..bcdebff6 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -47,10 +47,12 @@ namespace StardewModdingAPI.Framework.Content int areaWidth = sourceArea.Value.Width; int areaHeight = sourceArea.Value.Height; - if (areaX == 0 && areaY == 0 && areaWidth == source.Width && areaHeight == source.Height) + if (areaX == 0 && areaY == 0 && areaWidth == source.Width && areaHeight <= source.Height) { + // It's actually fine if the source is taller than the sourceArea + // the "extra" bits on the end of the array can just be ignored. sourceData = source.Data; - this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + this.PatchImageImpl(sourceData, areaWidth, areaHeight, sourceArea.Value, targetArea.Value, patchMode); } else { From c1d5d19e43a9305ebf71696408fc8a0777794f55 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Mon, 22 Aug 2022 22:35:08 -0400 Subject: [PATCH 10/17] Skip transparent rows at the start and end when doing a patch overlay. --- .../Framework/Content/AssetDataForImage.cs | 51 +++++++++++++++++-- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index bcdebff6..90468316 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -1,6 +1,7 @@ using System; using System.Buffers; using System.Diagnostics.CodeAnalysis; +using System.Linq; using Microsoft.Xna.Framework; using Microsoft.Xna.Framework.Graphics; using StardewValley; @@ -166,9 +167,51 @@ namespace StardewModdingAPI.Framework.Content if (sourceArea.Size != targetArea.Size) throw new InvalidOperationException("The source and target areas must be the same size."); - // merge data - if (patchMode == PatchMode.Overlay) + if (patchMode == PatchMode.Replace) + target.SetData(0, targetArea, sourceData, 0, pixelCount); + else { + // merge data + + // Content packs have a habit of using large amounts of blank space. + // Adjusting bounds to ignore transparent pixels at the start and end. + + int startIndex = -1; + for (int i = 0; i < pixelCount; i++) + { + if (sourceData[i].A >= MinOpacity) + { + startIndex = i; + break; + } + } + + if (startIndex == -1) + return; + + int endIndex = -1; + for (int i = pixelCount - 1; i >= startIndex; i--) + { + if (sourceData[i].A >= MinOpacity) + { + endIndex = i; + break; + } + } + + if (endIndex == -1) + return; + + int topoffset = startIndex / sourceArea.Width; + int bottomoffset = endIndex / sourceArea.Width; + + // Update target rectangle + targetArea = new(targetArea.X, targetArea.Y + topoffset, targetArea.Width, bottomoffset - topoffset + 1); + + pixelCount = targetArea.Width * targetArea.Height; + + int sourceoffset = topoffset * sourceArea.Width; + // get target data Color[] mergedData = ArrayPool.Shared.Rent(pixelCount); target.GetData(0, targetArea, mergedData, 0, pixelCount); @@ -177,7 +220,7 @@ namespace StardewModdingAPI.Framework.Content for (int i = 0; i < pixelCount; i++) { // ref locals here? Not sure. - Color above = sourceData[i]; + Color above = sourceData[sourceoffset + i]; Color below = mergedData[i]; // shortcut transparency @@ -205,8 +248,6 @@ namespace StardewModdingAPI.Framework.Content target.SetData(0, targetArea, mergedData, 0, pixelCount); ArrayPool.Shared.Return(mergedData); } - else - target.SetData(0, targetArea, sourceData, 0, pixelCount); } } } From 09fd12ddfe89c5d36b1db66167dd741e4f8a7e0f Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Mon, 22 Aug 2022 23:44:07 -0400 Subject: [PATCH 11/17] use startindex/endindex since I've already calculated those... --- src/SMAPI/Framework/Content/AssetDataForImage.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 90468316..70fa369f 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -207,7 +207,6 @@ namespace StardewModdingAPI.Framework.Content // Update target rectangle targetArea = new(targetArea.X, targetArea.Y + topoffset, targetArea.Width, bottomoffset - topoffset + 1); - pixelCount = targetArea.Width * targetArea.Height; int sourceoffset = topoffset * sourceArea.Width; @@ -217,11 +216,11 @@ namespace StardewModdingAPI.Framework.Content target.GetData(0, targetArea, mergedData, 0, pixelCount); // merge pixels - for (int i = 0; i < pixelCount; i++) + for (int i = startIndex; i <= endIndex; i++) { // ref locals here? Not sure. - Color above = sourceData[sourceoffset + i]; - Color below = mergedData[i]; + Color above = sourceData[i]; + Color below = mergedData[i - sourceoffset]; // shortcut transparency if (above.A < MinOpacity) From a3b8546ec8975ef1941e7618cef773c41d5c423f Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Mon, 22 Aug 2022 23:50:52 -0400 Subject: [PATCH 12/17] cleanup and comments --- src/SMAPI/Framework/Content/AssetDataForImage.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 70fa369f..f00d2d4c 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -187,7 +187,7 @@ namespace StardewModdingAPI.Framework.Content } if (startIndex == -1) - return; + return; // apparently a completely blank texture? int endIndex = -1; for (int i = pixelCount - 1; i >= startIndex; i--) @@ -200,8 +200,9 @@ namespace StardewModdingAPI.Framework.Content } if (endIndex == -1) - return; + return; // should never happen + // Calculate new Y bounds int topoffset = startIndex / sourceArea.Width; int bottomoffset = endIndex / sourceArea.Width; From 496c438be244c6e51f34cbcf238913edf55a8618 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Tue, 23 Aug 2022 14:34:23 -0400 Subject: [PATCH 13/17] fix indexing again, because apparently I'm bad at math now? --- src/SMAPI/Framework/Content/AssetDataForImage.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index f00d2d4c..2bbcc60c 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -219,15 +219,17 @@ namespace StardewModdingAPI.Framework.Content // merge pixels for (int i = startIndex; i <= endIndex; i++) { + int targetIndex = i - sourceoffset; + // ref locals here? Not sure. Color above = sourceData[i]; - Color below = mergedData[i - sourceoffset]; + Color below = mergedData[targetIndex]; // shortcut transparency if (above.A < MinOpacity) continue; if (below.A < MinOpacity || above.A == byte.MaxValue) - mergedData[i] = above; + mergedData[targetIndex] = above; // merge pixels else @@ -236,7 +238,7 @@ namespace StardewModdingAPI.Framework.Content // premultiplied by the content pipeline. The formula is derived from // https://blogs.msdn.microsoft.com/shawnhar/2009/11/06/premultiplied-alpha/. float alphaBelow = 1 - (above.A / 255f); - mergedData[i] = new Color( + mergedData[targetIndex] = new Color( r: (int)(above.R + (below.R * alphaBelow)), g: (int)(above.G + (below.G * alphaBelow)), b: (int)(above.B + (below.B * alphaBelow)), From 798a56bd2e94e9e60b588222c730e313c3dbe075 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Sun, 28 Aug 2022 16:14:57 -0400 Subject: [PATCH 14/17] Avoid copying memory for contingous buffers. --- .../Framework/Content/AssetDataForImage.cs | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 2bbcc60c..068634b3 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -40,6 +40,11 @@ namespace StardewModdingAPI.Framework.Content this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); + // check to see if the Data is sufficiently long. + // while SMAPI's impl is going to be, it's not necessarily the case for mod impl. + if (source.Data.Length < (sourceArea.Value.Bottom - 1) * source.Width + sourceArea.Value.Right) + throw new ArgumentException("Source data insufficiently long for this operation."); + // get the pixels for the source area Color[] sourceData; { @@ -48,37 +53,24 @@ namespace StardewModdingAPI.Framework.Content int areaWidth = sourceArea.Value.Width; int areaHeight = sourceArea.Value.Height; - if (areaX == 0 && areaY == 0 && areaWidth == source.Width && areaHeight <= source.Height) + if (areaWidth == source.Width) { // It's actually fine if the source is taller than the sourceArea // the "extra" bits on the end of the array can just be ignored. sourceData = source.Data; - this.PatchImageImpl(sourceData, areaWidth, areaHeight, sourceArea.Value, targetArea.Value, patchMode); + this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode, areaY); } else { int pixelCount = areaWidth * areaHeight; sourceData = ArrayPool.Shared.Rent(pixelCount); - if (areaX == 0 && areaWidth == source.Width) + // slower copying, line by line + for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) { - // shortcut copying because the area to copy is contiguous. This is - // probably uncommon, but Array.Copy benefits a lot. - - int sourceIndex = areaY * areaWidth; - int targetIndex = 0; - Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, pixelCount); - - } - else - { - // slower copying, line by line - for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) - { - int sourceIndex = (y * source.Width) + areaX; - int targetIndex = (y - areaY) * areaWidth; - Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); - } + int sourceIndex = (y * source.Width) + areaX; + int targetIndex = (y - areaY) * areaWidth; + Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); } // apply @@ -150,10 +142,11 @@ namespace StardewModdingAPI.Framework.Content /// The part of the to copy (or null to take the whole texture). This must be within the bounds of the texture. /// The part of the content to patch (or null to patch the whole texture). The original content within this area will be erased. This must be within the bounds of the existing spritesheet. /// Indicates how an image should be patched. + /// The row to start on, for the sourceData. /// One of the arguments is null. /// The is outside the bounds of the spritesheet. /// The content being read isn't an image. - private void PatchImageImpl(Color[] sourceData, int sourceWidth, int sourceHeight, Rectangle sourceArea, Rectangle targetArea, PatchMode patchMode) + private void PatchImageImpl(Color[] sourceData, int sourceWidth, int sourceHeight, Rectangle sourceArea, Rectangle targetArea, PatchMode patchMode, int startRow = 0) { // get texture Texture2D target = this.Data; @@ -168,7 +161,7 @@ namespace StardewModdingAPI.Framework.Content throw new InvalidOperationException("The source and target areas must be the same size."); if (patchMode == PatchMode.Replace) - target.SetData(0, targetArea, sourceData, 0, pixelCount); + target.SetData(0, targetArea, sourceData, startRow * sourceArea.Width, pixelCount); else { // merge data @@ -177,7 +170,7 @@ namespace StardewModdingAPI.Framework.Content // Adjusting bounds to ignore transparent pixels at the start and end. int startIndex = -1; - for (int i = 0; i < pixelCount; i++) + for (int i = startRow * sourceArea.Width; i < pixelCount; i++) { if (sourceData[i].A >= MinOpacity) { From 48d0f70ffd07df9ba364808a71407800834f95d3 Mon Sep 17 00:00:00 2001 From: atravita-mods <94934860+atravita-mods@users.noreply.github.com> Date: Thu, 6 Oct 2022 08:16:57 -0400 Subject: [PATCH 15/17] fix indexing math again. --- src/SMAPI/Framework/Content/AssetDataForImage.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 068634b3..636d4a71 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -137,8 +137,8 @@ namespace StardewModdingAPI.Framework.Content /// Overwrite part of the image. /// The image data to patch into the content. - /// The pixel width of the source image. - /// The pixel height of the source image. + /// The pixel width of the original source image. + /// The pixel height of the original source image. /// The part of the to copy (or null to take the whole texture). This must be within the bounds of the texture. /// The part of the content to patch (or null to patch the whole texture). The original content within this area will be erased. This must be within the bounds of the existing spritesheet. /// Indicates how an image should be patched. @@ -183,7 +183,7 @@ namespace StardewModdingAPI.Framework.Content return; // apparently a completely blank texture? int endIndex = -1; - for (int i = pixelCount - 1; i >= startIndex; i--) + for (int i = startRow * sourceArea.Width + pixelCount - 1; i >= startIndex; i--) { if (sourceData[i].A >= MinOpacity) { From 40d5cd7c05d4e0a4e6894cd7b9f6d7d747716837 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 8 Oct 2022 17:42:32 -0400 Subject: [PATCH 16/17] use try..finally to make sure rented arrays are returned --- .../Framework/Content/AssetDataForImage.cs | 110 ++++++++++-------- .../ContentManagers/ModContentManager.cs | 37 +++--- 2 files changed, 81 insertions(+), 66 deletions(-) diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 636d4a71..3abcd328 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -1,7 +1,6 @@ using System; using System.Buffers; using System.Diagnostics.CodeAnalysis; -using System.Linq; using Microsoft.Xna.Framework; using Microsoft.Xna.Framework.Graphics; using StardewValley; @@ -64,20 +63,23 @@ namespace StardewModdingAPI.Framework.Content { int pixelCount = areaWidth * areaHeight; sourceData = ArrayPool.Shared.Rent(pixelCount); - - // slower copying, line by line - for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) + try { - int sourceIndex = (y * source.Width) + areaX; - int targetIndex = (y - areaY) * areaWidth; - Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); + // slower copying, line by line + for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) + { + int sourceIndex = (y * source.Width) + areaX; + int targetIndex = (y - areaY) * areaWidth; + Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); + } + + // apply + this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + } + finally + { + ArrayPool.Shared.Return(sourceData); } - - // apply - this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); - - // return - ArrayPool.Shared.Return(sourceData); } } } @@ -98,13 +100,15 @@ namespace StardewModdingAPI.Framework.Content // get source data int pixelCount = sourceArea.Value.Width * sourceArea.Value.Height; Color[] sourceData = ArrayPool.Shared.Rent(pixelCount); - source.GetData(0, sourceArea, sourceData, 0, pixelCount); - - // apply - this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); - - // return - ArrayPool.Shared.Return(sourceData); + try + { + source.GetData(0, sourceArea, sourceData, 0, pixelCount); + this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + } + finally + { + ArrayPool.Shared.Return(sourceData); + } } /// @@ -207,41 +211,47 @@ namespace StardewModdingAPI.Framework.Content // get target data Color[] mergedData = ArrayPool.Shared.Rent(pixelCount); - target.GetData(0, targetArea, mergedData, 0, pixelCount); - - // merge pixels - for (int i = startIndex; i <= endIndex; i++) + try { - int targetIndex = i - sourceoffset; - - // ref locals here? Not sure. - Color above = sourceData[i]; - Color below = mergedData[targetIndex]; - - // shortcut transparency - if (above.A < MinOpacity) - continue; - if (below.A < MinOpacity || above.A == byte.MaxValue) - mergedData[targetIndex] = above; + target.GetData(0, targetArea, mergedData, 0, pixelCount); // merge pixels - else + for (int i = startIndex; i <= endIndex; i++) { - // This performs a conventional alpha blend for the pixels, which are already - // premultiplied by the content pipeline. The formula is derived from - // https://blogs.msdn.microsoft.com/shawnhar/2009/11/06/premultiplied-alpha/. - float alphaBelow = 1 - (above.A / 255f); - mergedData[targetIndex] = new Color( - r: (int)(above.R + (below.R * alphaBelow)), - g: (int)(above.G + (below.G * alphaBelow)), - b: (int)(above.B + (below.B * alphaBelow)), - alpha: Math.Max(above.A, below.A) - ); - } - } + int targetIndex = i - sourceoffset; - target.SetData(0, targetArea, mergedData, 0, pixelCount); - ArrayPool.Shared.Return(mergedData); + // ref locals here? Not sure. + Color above = sourceData[i]; + Color below = mergedData[targetIndex]; + + // shortcut transparency + if (above.A < MinOpacity) + continue; + if (below.A < MinOpacity || above.A == byte.MaxValue) + mergedData[targetIndex] = above; + + // merge pixels + else + { + // This performs a conventional alpha blend for the pixels, which are already + // premultiplied by the content pipeline. The formula is derived from + // https://blogs.msdn.microsoft.com/shawnhar/2009/11/06/premultiplied-alpha/. + float alphaBelow = 1 - (above.A / 255f); + mergedData[targetIndex] = new Color( + r: (int)(above.R + (below.R * alphaBelow)), + g: (int)(above.G + (below.G * alphaBelow)), + b: (int)(above.B + (below.B * alphaBelow)), + alpha: Math.Max(above.A, below.A) + ); + } + } + + target.SetData(0, targetArea, mergedData, 0, pixelCount); + } + finally + { + ArrayPool.Shared.Return(mergedData); + } } } } diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index 0c793808..6b8a5874 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -395,25 +395,30 @@ namespace StardewModdingAPI.Framework.ContentManagers // premultiply pixels int count = texture.Width * texture.Height; Color[] data = ArrayPool.Shared.Rent(count); - texture.GetData(data, 0, count); - - bool changed = false; - for (int i = 0; i < count; i++) + try { - ref Color pixel = ref data[i]; - if (pixel.A is (byte.MinValue or byte.MaxValue)) - continue; // no need to change fully transparent/opaque pixels + texture.GetData(data, 0, count); - data[i] = new Color(pixel.R * pixel.A / byte.MaxValue, pixel.G * pixel.A / byte.MaxValue, pixel.B * pixel.A / byte.MaxValue, pixel.A); // slower version: Color.FromNonPremultiplied(data[i].ToVector4()) - changed = true; + bool changed = false; + for (int i = 0; i < count; i++) + { + ref Color pixel = ref data[i]; + if (pixel.A is (byte.MinValue or byte.MaxValue)) + continue; // no need to change fully transparent/opaque pixels + + data[i] = new Color(pixel.R * pixel.A / byte.MaxValue, pixel.G * pixel.A / byte.MaxValue, pixel.B * pixel.A / byte.MaxValue, pixel.A); // slower version: Color.FromNonPremultiplied(data[i].ToVector4()) + changed = true; + } + + if (changed) + texture.SetData(data, 0, count); + + return texture; + } + finally + { + ArrayPool.Shared.Return(data); } - - if (changed) - texture.SetData(data, 0, count); - - // return - ArrayPool.Shared.Return(data); - return texture; } /// Fix custom map tilesheet paths so they can be found by the content manager. From 2e0bc5ddfe90102fe5adbc90b2d53c5cbb8405fe Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 8 Oct 2022 17:45:50 -0400 Subject: [PATCH 17/17] tweak new code --- .../Framework/Content/AssetDataForImage.cs | 181 ++++++++---------- .../ContentManagers/ModContentManager.cs | 13 +- .../Framework/Logging/LogOnceCacheKey.cs | 10 + src/SMAPI/Framework/Monitor.cs | 14 +- 4 files changed, 103 insertions(+), 115 deletions(-) create mode 100644 src/SMAPI/Framework/Logging/LogOnceCacheKey.cs diff --git a/src/SMAPI/Framework/Content/AssetDataForImage.cs b/src/SMAPI/Framework/Content/AssetDataForImage.cs index 3abcd328..0380dd9e 100644 --- a/src/SMAPI/Framework/Content/AssetDataForImage.cs +++ b/src/SMAPI/Framework/Content/AssetDataForImage.cs @@ -33,71 +33,59 @@ namespace StardewModdingAPI.Framework.Content /// public void PatchImage(IRawTextureData source, Rectangle? sourceArea = null, Rectangle? targetArea = null, PatchMode patchMode = PatchMode.Replace) { - // nullcheck if (source == null) throw new ArgumentNullException(nameof(source), "Can't patch from null source data."); + // get normalized bounds this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); - - // check to see if the Data is sufficiently long. - // while SMAPI's impl is going to be, it's not necessarily the case for mod impl. if (source.Data.Length < (sourceArea.Value.Bottom - 1) * source.Width + sourceArea.Value.Right) - throw new ArgumentException("Source data insufficiently long for this operation."); + throw new ArgumentException("Can't apply image patch because the source image is smaller than the source area.", nameof(source)); + int areaX = sourceArea.Value.X; + int areaY = sourceArea.Value.Y; + int areaWidth = sourceArea.Value.Width; + int areaHeight = sourceArea.Value.Height; - // get the pixels for the source area - Color[] sourceData; + // shortcut: if the area width matches the source image, we can apply the image as-is without needing + // to copy the pixels into a smaller subset. It's fine if the source is taller than the area, since we'll + // just ignore the extra data at the end of the pixel array. + if (areaWidth == source.Width) { - int areaX = sourceArea.Value.X; - int areaY = sourceArea.Value.Y; - int areaWidth = sourceArea.Value.Width; - int areaHeight = sourceArea.Value.Height; + this.PatchImageImpl(source.Data, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode, areaY); + return; + } - if (areaWidth == source.Width) + // else copy the pixels within the smaller area & apply that + int pixelCount = areaWidth * areaHeight; + Color[] sourceData = ArrayPool.Shared.Rent(pixelCount); + try + { + for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) { - // It's actually fine if the source is taller than the sourceArea - // the "extra" bits on the end of the array can just be ignored. - sourceData = source.Data; - this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode, areaY); + int sourceIndex = (y * source.Width) + areaX; + int targetIndex = (y - areaY) * areaWidth; + Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); } - else - { - int pixelCount = areaWidth * areaHeight; - sourceData = ArrayPool.Shared.Rent(pixelCount); - try - { - // slower copying, line by line - for (int y = areaY, maxY = areaY + areaHeight; y < maxY; y++) - { - int sourceIndex = (y * source.Width) + areaX; - int targetIndex = (y - areaY) * areaWidth; - Array.Copy(source.Data, sourceIndex, sourceData, targetIndex, areaWidth); - } - // apply - this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); - } - finally - { - ArrayPool.Shared.Return(sourceData); - } - } + this.PatchImageImpl(sourceData, source.Width, source.Height, sourceArea.Value, targetArea.Value, patchMode); + } + finally + { + ArrayPool.Shared.Return(sourceData); } } /// public void PatchImage(Texture2D source, Rectangle? sourceArea = null, Rectangle? targetArea = null, PatchMode patchMode = PatchMode.Replace) { - // nullcheck if (source == null) throw new ArgumentNullException(nameof(source), "Can't patch from a null source texture."); + // get normalized bounds this.GetPatchBounds(ref sourceArea, ref targetArea, source.Width, source.Height); - - // validate source bounds if (!source.Bounds.Contains(sourceArea.Value)) throw new ArgumentOutOfRangeException(nameof(sourceArea), "The source area is outside the bounds of the source texture."); - // get source data + // get source data & apply int pixelCount = sourceArea.Value.Width * sourceArea.Value.Height; Color[] sourceData = ArrayPool.Shared.Rent(pixelCount); try @@ -164,94 +152,91 @@ namespace StardewModdingAPI.Framework.Content if (sourceArea.Size != targetArea.Size) throw new InvalidOperationException("The source and target areas must be the same size."); + // shortcut: replace the entire area if (patchMode == PatchMode.Replace) - target.SetData(0, targetArea, sourceData, startRow * sourceArea.Width, pixelCount); - else { - // merge data + target.SetData(0, targetArea, sourceData, startRow * sourceArea.Width, pixelCount); + return; + } - // Content packs have a habit of using large amounts of blank space. - // Adjusting bounds to ignore transparent pixels at the start and end. - - int startIndex = -1; + // skip transparent pixels at the start & end (e.g. large spritesheet with a few sprites replaced) + int startIndex = -1; + int endIndex = -1; + { for (int i = startRow * sourceArea.Width; i < pixelCount; i++) { - if (sourceData[i].A >= MinOpacity) + if (sourceData[i].A >= AssetDataForImage.MinOpacity) { startIndex = i; break; } } - if (startIndex == -1) - return; // apparently a completely blank texture? + return; // blank texture - int endIndex = -1; for (int i = startRow * sourceArea.Width + pixelCount - 1; i >= startIndex; i--) { - if (sourceData[i].A >= MinOpacity) + if (sourceData[i].A >= AssetDataForImage.MinOpacity) { endIndex = i; break; } } - if (endIndex == -1) - return; // should never happen + return; // ??? + } - // Calculate new Y bounds - int topoffset = startIndex / sourceArea.Width; - int bottomoffset = endIndex / sourceArea.Width; + // update target rectangle + int sourceOffset; + { + int topOffset = startIndex / sourceArea.Width; + int bottomOffset = endIndex / sourceArea.Width; - // Update target rectangle - targetArea = new(targetArea.X, targetArea.Y + topoffset, targetArea.Width, bottomoffset - topoffset + 1); + targetArea = new(targetArea.X, targetArea.Y + topOffset, targetArea.Width, bottomOffset - topOffset + 1); pixelCount = targetArea.Width * targetArea.Height; + sourceOffset = topOffset * sourceArea.Width; + } - int sourceoffset = topoffset * sourceArea.Width; + // apply + Color[] mergedData = ArrayPool.Shared.Rent(pixelCount); + try + { + target.GetData(0, targetArea, mergedData, 0, pixelCount); - // get target data - Color[] mergedData = ArrayPool.Shared.Rent(pixelCount); - try + for (int i = startIndex; i <= endIndex; i++) { - target.GetData(0, targetArea, mergedData, 0, pixelCount); + int targetIndex = i - sourceOffset; + + Color above = sourceData[i]; + Color below = mergedData[targetIndex]; + + // shortcut transparency + if (above.A < AssetDataForImage.MinOpacity) + continue; + if (below.A < AssetDataForImage.MinOpacity || above.A == byte.MaxValue) + mergedData[targetIndex] = above; // merge pixels - for (int i = startIndex; i <= endIndex; i++) + else { - int targetIndex = i - sourceoffset; - - // ref locals here? Not sure. - Color above = sourceData[i]; - Color below = mergedData[targetIndex]; - - // shortcut transparency - if (above.A < MinOpacity) - continue; - if (below.A < MinOpacity || above.A == byte.MaxValue) - mergedData[targetIndex] = above; - - // merge pixels - else - { - // This performs a conventional alpha blend for the pixels, which are already - // premultiplied by the content pipeline. The formula is derived from - // https://blogs.msdn.microsoft.com/shawnhar/2009/11/06/premultiplied-alpha/. - float alphaBelow = 1 - (above.A / 255f); - mergedData[targetIndex] = new Color( - r: (int)(above.R + (below.R * alphaBelow)), - g: (int)(above.G + (below.G * alphaBelow)), - b: (int)(above.B + (below.B * alphaBelow)), - alpha: Math.Max(above.A, below.A) - ); - } + // This performs a conventional alpha blend for the pixels, which are already + // premultiplied by the content pipeline. The formula is derived from + // https://blogs.msdn.microsoft.com/shawnhar/2009/11/06/premultiplied-alpha/. + float alphaBelow = 1 - (above.A / 255f); + mergedData[targetIndex] = new Color( + r: (int)(above.R + (below.R * alphaBelow)), + g: (int)(above.G + (below.G * alphaBelow)), + b: (int)(above.B + (below.B * alphaBelow)), + alpha: Math.Max(above.A, below.A) + ); } + } - target.SetData(0, targetArea, mergedData, 0, pixelCount); - } - finally - { - ArrayPool.Shared.Return(mergedData); - } + target.SetData(0, targetArea, mergedData, 0, pixelCount); + } + finally + { + ArrayPool.Shared.Return(mergedData); } } } diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index 6b8a5874..72dcf6e1 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -241,7 +241,7 @@ namespace StardewModdingAPI.Framework.ContentManagers { using FileStream stream = File.OpenRead(file.FullName); Texture2D texture = Texture2D.FromStream(Game1.graphics.GraphicsDevice, stream); - texture = this.PremultiplyTransparency(texture); + this.PremultiplyTransparency(texture); return (T)(object)texture; } } @@ -345,17 +345,15 @@ namespace StardewModdingAPI.Framework.ContentManagers this.ThrowLoadError(assetName, ContentLoadErrorType.InvalidData, $"can't read file with extension '{file.Extension}' as type '{typeof(TAsset)}'; must be type '{string.Join("' or '", validTypes.Select(p => p.FullName))}'."); } - /// Throws an error which indicates that an asset couldn't be loaded. + /// Throw an error which indicates that an asset couldn't be loaded. /// Why loading an asset through the content pipeline failed. /// The asset name that failed to load. /// The reason the file couldn't be loaded. /// The underlying exception, if applicable. + /// [DoesNotReturn] [DebuggerStepThrough, DebuggerHidden] [MethodImpl(MethodImplOptions.NoInlining)] -#if NET6_0_OR_GREATER - [StackTraceHidden] -#endif private void ThrowLoadError(IAssetName assetName, ContentLoadErrorType errorType, string reasonPhrase, Exception? exception = null) { throw new SContentLoadException(errorType, $"Failed loading asset '{assetName}' from {this.Name}: {reasonPhrase}", exception); @@ -390,9 +388,8 @@ namespace StardewModdingAPI.Framework.ContentManagers /// The texture to premultiply. /// Returns a premultiplied texture. /// Based on code by David Gouveia. - private Texture2D PremultiplyTransparency(Texture2D texture) + private void PremultiplyTransparency(Texture2D texture) { - // premultiply pixels int count = texture.Width * texture.Height; Color[] data = ArrayPool.Shared.Rent(count); try @@ -412,8 +409,6 @@ namespace StardewModdingAPI.Framework.ContentManagers if (changed) texture.SetData(data, 0, count); - - return texture; } finally { diff --git a/src/SMAPI/Framework/Logging/LogOnceCacheKey.cs b/src/SMAPI/Framework/Logging/LogOnceCacheKey.cs new file mode 100644 index 00000000..4d31ffeb --- /dev/null +++ b/src/SMAPI/Framework/Logging/LogOnceCacheKey.cs @@ -0,0 +1,10 @@ +using System.Diagnostics.CodeAnalysis; + +namespace StardewModdingAPI.Framework.Logging +{ + /// The cache key for the . + /// The log message. + /// The log level. + [SuppressMessage("ReSharper", "NotAccessedPositionalProperty.Local", Justification = "This is only used as a lookup key.")] + internal readonly record struct LogOnceCacheKey(string Message, LogLevel Level); +} diff --git a/src/SMAPI/Framework/Monitor.cs b/src/SMAPI/Framework/Monitor.cs index d33bf259..4ed2c9bb 100644 --- a/src/SMAPI/Framework/Monitor.cs +++ b/src/SMAPI/Framework/Monitor.cs @@ -25,15 +25,13 @@ namespace StardewModdingAPI.Framework private readonly LogFileManager LogFile; /// The maximum length of the values. - private static readonly int MaxLevelLength = (from level in Enum.GetValues() select level.ToString().Length).Max(); + private static readonly int MaxLevelLength = Enum.GetValues().Max(level => level.ToString().Length); - /// A mapping of console log levels to their string form. - private static readonly Dictionary LogStrings = Enum.GetValues().ToDictionary(k => k, v => v.ToString().ToUpper().PadRight(MaxLevelLength)); - - private readonly record struct LogOnceCacheEntry(string message, LogLevel level); + /// The cached representation for each level when added to a log header. + private static readonly Dictionary LogStrings = Enum.GetValues().ToDictionary(level => level, level => level.ToString().ToUpper().PadRight(Monitor.MaxLevelLength)); /// A cache of messages that should only be logged once. - private readonly HashSet LogOnceCache = new(); + private readonly HashSet LogOnceCache = new(); /// Get the screen ID that should be logged to distinguish between players in split-screen mode, if any. private readonly Func GetScreenIdForLog; @@ -89,7 +87,7 @@ namespace StardewModdingAPI.Framework /// public void LogOnce(string message, LogLevel level = LogLevel.Trace) { - if (this.LogOnceCache.Add(new LogOnceCacheEntry(message, level))) + if (this.LogOnceCache.Add(new LogOnceCacheKey(message, level))) this.LogImpl(this.Source, message, (ConsoleLogLevel)level); } @@ -152,7 +150,7 @@ namespace StardewModdingAPI.Framework /// The log level. private string GenerateMessagePrefix(string source, ConsoleLogLevel level) { - string levelStr = LogStrings[level]; + string levelStr = Monitor.LogStrings[level]; int? playerIndex = this.GetScreenIdForLog(); return $"[{DateTime.Now:HH:mm:ss} {levelStr}{(playerIndex != null ? $" screen_{playerIndex}" : "")} {source}]";