fix: keep a path-tracked texture published across UpdateTexture's recreate [patch]

ImGui.App/ImGuiApp.cs

@@ -1907,7 +1907,10 @@ public static ImGuiAppTextureInfo CreateTexture(ReadOnlySpan<byte> rgba, int wid
 	/// <param name="height">Texture height in pixels.</param>
 	/// <remarks>
 	/// Falls back to deleting and recreating the texture when the active renderer backend cannot update
-	/// in place, mutating <paramref name="textureInfo"/> to carry the new handle.
+	/// in place, mutating <paramref name="textureInfo"/> to carry the new handle. A texture that came
+	/// from <see cref="GetOrLoadTexture(AbsoluteFilePath)"/> stays published in the path-keyed cache
+	/// across that fallback, so <see cref="TryGetTexture(AbsoluteFilePath, out ImGuiAppTextureInfo?)"/>
+	/// keeps finding it and <c>CleanupAllTextures</c> keeps owning it.
 	/// </remarks>
 	/// <exception cref="ArgumentNullException"><paramref name="textureInfo"/> is null.</exception>
 	/// <exception cref="ArgumentException">The span length does not equal width * height * 4.</exception>
@@ -1923,15 +1926,44 @@ public static void UpdateTexture(ImGuiAppTextureInfo textureInfo, ReadOnlySpan<b
 			return;
 		}
 
-		DeleteTexture(textureInfo.TextureId);
-		textureInfo.TextureId = UploadTextureRGBA(rgba.ToArray(), width, height);
-		unsafe
+		// A span cannot be captured by the Invoke body below, and the upload needs the pixels on the
+		// invoker thread, so the copy the recreate path always made happens here instead.
+		byte[] pixels = rgba.ToArray();
+
+		// Capture, delete, recreate and republish happen together inside one Invoke, for the same
+		// reason GetOrLoadTexture publishes inside its own: between the delete and the republish the
+		// path is absent from the cache, so a concurrent GetOrLoadTexture for it would miss, decode
+		// the file again and upload a second GPU texture. Invoke bodies never overlap, so nothing can
+		// observe the gap. Nested Invokes below run inline on the invoker thread.
+		Invoker.Invoke(() =>
 		{
-			textureInfo.TextureRef = new ImTextureRef(default, textureInfo.TextureId);
-		}
+			// DeleteTexture removes every cache entry whose handle matches the one being deleted,
+			// which for a path-tracked texture is the entry GetOrLoadTexture published. The recreate
+			// below gives textureInfo a live handle again but restores no entry, so without this
+			// republish the texture stays on the GPU while TryGetTexture reports it missing, the next
+			// GetOrLoadTexture uploads a duplicate, and CleanupAllTextures - which walks
+			// Textures.Keys - can no longer reach either handle to free it.
+			//
+			// Reference equality is the test rather than a handle comparison: only the entry this
+			// instance is actually published under should come back, never one that a different
+			// ImGuiAppTextureInfo happens to share a handle with.
+			List<AbsoluteFilePath> publishedKeys = [.. Textures.Where(entry => ReferenceEquals(entry.Value, textureInfo)).Select(entry => entry.Key)];
+
+			DeleteTexture(textureInfo.TextureId);
+			textureInfo.TextureId = UploadTextureRGBA(pixels, width, height);
+			unsafe
+			{
+				textureInfo.TextureRef = new ImTextureRef(default, textureInfo.TextureId);
+			}
+
+			textureInfo.Width = width;
+			textureInfo.Height = height;
 
-		textureInfo.Width = width;
-		textureInfo.Height = height;
+			foreach (AbsoluteFilePath key in publishedKeys)
+			{
+				Textures[key] = textureInfo;
+			}
+		});
 	}
 
 	private static void ValidatePixelSpan(ReadOnlySpan<byte> rgba, int width, int height)

tests/ImGui.App.Tests/ImGuiAppTests.cs

@@ -587,13 +587,13 @@
 			// all four callers on the uncached side of the cache check. Give them a moment to get
 			// there and queue their upload. Pumping too early would only let one caller win by
 			// itself, which weakens this assertion rather than failing it spuriously.
 			Thread.Sleep(250);
 
 			Stopwatch pump = Stopwatch.StartNew();
 			while (Array.Exists(callers, caller => !caller.IsCompleted) && pump.Elapsed < TimeSpan.FromSeconds(30))
 			{
 				ImGuiApp.Invoker.DoInvokes();
 				Thread.Sleep(1);
 			}
 
 			foreach (Task<ImGuiAppTextureInfo> caller in callers)
@@ -702,6 +702,103 @@
 		Assert.AreEqual(2, backend.CreateTextureCallCount, "A declined update must recreate the texture");
 	}
 
+	// Writes a decodable PNG to a temp file and returns its path. The caller deletes it.
+	private static string WriteTempPng(int width, int height)
+	{
+		byte[] pixels = new byte[width * height * 4];
+		Array.Fill(pixels, (byte)0xFF);
+		byte[] png = TestImageBuilder.Png(width, height, colorType: 6, bitDepth: 8, pixels);
+		string file = Path.Join(Path.GetTempPath(), $"{Guid.NewGuid():N}.png");
+		File.WriteAllBytes(file, png);
+		return file;
+	}
+
+	[TestMethod]
+	public void UpdateTexture_OnAPathTrackedTexture_KeepsItPublishedInTheCache()
+	{
+		// UpdateTexture's recreate fallback went through DeleteTexture, which drops every cache entry
+		// pointing at the deleted handle - including the entry GetOrLoadTexture published for this
+		// path - and then never restored it. The texture stayed live on the GPU but became invisible
+		// to TryGetTexture, so callers were told a loaded texture was missing.
+		ResetState();
+		ImGuiApp.Invoker = new Invoker.Invoker();
+		FakeRendererBackend backend = new() { NextHandle = 4242 };
+		ImGuiApp.renderer = backend;
+		ImGuiApp.controller = null;
+
+		string file = WriteTempPng(2, 2);
+		try
+		{
+			AbsoluteFilePath path = file.As<AbsoluteFilePath>();
+			ImGuiAppTextureInfo info = ImGuiApp.GetOrLoadTexture(path);
+
+			// A size change forces the delete/recreate fallback regardless of what the backend can do
+			// in place, which is the path the issue reports.
+			ImGuiApp.UpdateTexture(info, new byte[1 * 1 * 4], 1, 1);
+
+			Assert.IsTrue(ImGuiApp.TryGetTexture(path, out ImGuiAppTextureInfo? cached), "A path-tracked texture must stay published after a resize");
+			Assert.AreSame(info, cached, "The cache should still hold the instance the caller is using");
+			Assert.AreEqual(1, cached!.Width, "The published entry must carry the new size");
+			Assert.AreEqual(1, cached.Height, "The published entry must carry the new size");
+			Assert.AreEqual(backend.NextHandle, cached.TextureId, "The published entry must carry the recreated handle");
+		}
+		finally
+		{
+			File.Delete(file);
+		}
+	}
+
+	[TestMethod]
+	public void UpdateTexture_OnAPathTrackedTexture_DoesNotUploadADuplicateOnTheNextLoad()
+	{
+		// The downstream cost of the desync, which the cache-presence assertion above does not by
+		// itself pin: with the path evicted, the next GetOrLoadTexture missed, re-decoded the file and
+		// uploaded a second GPU texture, leaving the first one reachable only through the caller's own
+		// ImGuiAppTextureInfo. CleanupAllTextures walks Textures.Keys, so whichever handle was not
+		// published could never be freed - the leak the issue reports.
+		ResetState();
+		ImGuiApp.Invoker = new Invoker.Invoker();
+		FakeRendererBackend backend = new() { NextHandle = 4242 };
+		ImGuiApp.renderer = backend;
+		ImGuiApp.controller = null;
+
+		string file = WriteTempPng(2, 2);
+		try
+		{
+			AbsoluteFilePath path = file.As<AbsoluteFilePath>();
+			ImGuiAppTextureInfo info = ImGuiApp.GetOrLoadTexture(path);
+			ImGuiApp.UpdateTexture(info, new byte[1 * 1 * 4], 1, 1);
+
+			int uploadsAfterUpdate = backend.CreateTextureCallCount;
+			ImGuiAppTextureInfo reloaded = ImGuiApp.GetOrLoadTexture(path);
+
+			Assert.AreSame(info, reloaded, "Re-loading the path should hand back the texture already in use");
+			Assert.AreEqual(uploadsAfterUpdate, backend.CreateTextureCallCount, "Re-loading a still-cached path must not upload a duplicate GPU texture");
+			CollectionAssert.Contains(ImGuiApp.Textures.Keys.ToList(), path, "CleanupAllTextures walks Textures.Keys, so the path must still be among them for the handle to be freeable");
+		}
+		finally
+		{
+			File.Delete(file);
+		}
+	}
+
+	[TestMethod]
+	public void UpdateTexture_OnAnUntrackedTexture_StaysOutOfTheCache()
+	{
+		// The republish must restore only entries this instance was already published under. A texture
+		// from CreateTexture has no path and was never in the cache, so a resize must not add one.
+		ResetState();
+		ImGuiApp.Invoker = new Invoker.Invoker();
+		FakeRendererBackend backend = new() { NextHandle = 42 };
+		ImGuiApp.renderer = backend;
+		ImGuiApp.controller = null;
+		ImGuiAppTextureInfo info = ImGuiApp.CreateTexture(new byte[2 * 2 * 4], 2, 2);
+
+		ImGuiApp.UpdateTexture(info, new byte[1 * 1 * 4], 1, 1);
+
+		Assert.AreEqual(0, ImGuiApp.Textures.Count, "A memory texture must stay out of the path-keyed cache across a resize");
+	}
+
 	[TestMethod]
 	public void PerformanceSettings_DefaultValues_AreCorrect()
 	{