perf(content): CellStruct clone guard, solid-color texture cache, drop double ToList (2026-07-24 audit review)

Three small verified MeshExtractor.cs fixes from an allocation audit:

- CellStruct clip-map clone guard: the CellStruct polygon-surface path
  cloned decoded texture data unconditionally before applying clip-map
  transparency, even though the comment above it says "if we got this
  from the cache, we need to clone it." The GfxObj path already had the
  correct pattern (clone only when textureDataIsCached). Wired the same
  textureDataIsCached flag through CellStruct's TryGet/GetOrCreate/
  RetainOrUse call sites and gated the clone on it, matching GfxObj
  exactly — freshly-decoded arrays (the common case) now skip a
  redundant clone + BlockCopy.

- Solid-color texture cache: isSolid surfaces (Base1Solid / NoPos-
  stippled polys) allocated a fresh 32x32 RGBA array (4 KiB) on every
  polygon, even for repeated colors. Added a MeshExtractor-scoped
  ConcurrentDictionary<uint, byte[]> keyed by packed ARGB, bounded by
  distinct colors observed (a few hundred at most) rather than call
  volume. Traced every downstream consumer of the solid-color array
  (clip-map clone-guard, translucency clone-guard, GL upload, pak
  serialization) — none currently mutate a solid-color array in place,
  since isSolid is mutually exclusive with the texture-decode branch
  those guards live in — but wired textureDataIsCached = true at both
  call sites anyway so the existing clone-before-mutate machinery
  protects the shared array if that ever changes.

- Dropped the redundant .ToList() on both _dats.ResolveId(...) calls:
  DatCollectionAdapter.ResolveId already returns a materialized List<T>
  typed as IEnumerable; the immediate .OrderByDescending().FirstOrDefault()
  enumerates once, so the extra copy was pure waste.

Added SolidColorTextureCacheTests (dat-gated, matching suite convention)
exercising the new cache directly via internal visibility.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit e9f9d7539966cc26343e45d653a751c5d1045810)
This commit is contained in:
Erik 2026-07-24 11:47:44 +02:00
parent a4b1214e0f
commit 60bc313917
2 changed files with 109 additions and 9 deletions

View file

@ -0,0 +1,66 @@
using AcDream.Content;
using DatReaderWriter;
using DatReaderWriter.Options;
using DatReaderWriter.Types;
using Microsoft.Extensions.Logging.Abstractions;
using Xunit;
namespace AcDream.Content.Tests;
/// <summary>
/// MeshExtractor.GetOrCreateSolidColorTexture (2026-07-24 audit review, Finding
/// D): solid-color surfaces bake a 32x32 RGBA fill and previously allocated a
/// fresh array on every call. These tests exercise the cache directly (internal
/// visibility via InternalsVisibleTo) rather than through the full GfxObj/
/// CellStruct extraction pipeline, so they don't need fixture ids — only a
/// working <see cref="MeshExtractor"/> instance, which still needs real dats to
/// construct (its ctor builds a RetailPhysicsScriptLoader from dats.Portal).
/// Skips cleanly when dats are absent, matching ContentConformanceDats convention.
/// </summary>
public sealed class SolidColorTextureCacheTests {
[Fact]
public void GetOrCreateSolidColorTexture_SameColor_ReturnsSameSharedArray() {
var datDir = ContentConformanceDats.ResolveDatDir();
if (datDir is null) return; // dats absent (CI) — skip, matching suite convention
using var dats = new DatCollection(datDir, DatAccessType.Read);
using var adapter = new DatCollectionAdapter(dats);
var extractor = new MeshExtractor(adapter, NullLogger.Instance, sideStagedSink: null);
var color = new ColorARGB { Alpha = 255, Red = 10, Green = 20, Blue = 30 };
var first = extractor.GetOrCreateSolidColorTexture(color, 32, 32);
var second = extractor.GetOrCreateSolidColorTexture(color, 32, 32);
// Same ARGB value must hit the cache and return the SAME array instance,
// not merely an equal one — that's the allocation this cache exists to avoid.
Assert.Same(first, second);
Assert.Equal(32 * 32 * 4, first.Length);
Assert.Equal(10, first[0]);
Assert.Equal(20, first[1]);
Assert.Equal(30, first[2]);
Assert.Equal(255, first[3]);
}
[Fact]
public void GetOrCreateSolidColorTexture_DifferentColors_ReturnDistinctArrays() {
var datDir = ContentConformanceDats.ResolveDatDir();
if (datDir is null) return; // dats absent (CI) — skip, matching suite convention
using var dats = new DatCollection(datDir, DatAccessType.Read);
using var adapter = new DatCollectionAdapter(dats);
var extractor = new MeshExtractor(adapter, NullLogger.Instance, sideStagedSink: null);
var red = new ColorARGB { Alpha = 255, Red = 255, Green = 0, Blue = 0 };
var blue = new ColorARGB { Alpha = 255, Red = 0, Green = 0, Blue = 255 };
var redTexture = extractor.GetOrCreateSolidColorTexture(red, 32, 32);
var blueTexture = extractor.GetOrCreateSolidColorTexture(blue, 32, 32);
Assert.NotSame(redTexture, blueTexture);
Assert.Equal(255, redTexture[0]);
Assert.Equal(0, redTexture[2]);
Assert.Equal(0, blueTexture[0]);
Assert.Equal(255, blueTexture[2]);
}
}