Fixed animated group textures not animating#1124
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a compiler-side issue where animated textures applied as sub-areas (via group texturing tools using ParentArea) were not being recognized as animated, causing them to compile as static textures instead.
Changes:
- Detects textures with a non-zero
ParentAreaand attempts to match them against reference animated textures using reconstructed “full-frame” UVs. - When a match is found, synthesizes a sub-area
AnimatedTextureSet, generates lookup entries for it, and retriesAddTextureso the sub-area can be treated as animated. - Implements the same logic in both the classic TR compiler texinfo manager and the TombEngine texinfo manager.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| TombLib/TombLib/LevelData/Compilers/Util/TexInfoManager.cs | Adds sub-area animated-texture detection + synthetic sub-animation lookup generation for classic/TR compilers. |
| TombLib/TombLib/LevelData/Compilers/TombEngine/TombEngineTexInfoManager.cs | Mirrors the sub-area animated-texture handling for TombEngine texture compilation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Generate reference lookups for this sub-area animation set | ||
| GenerateAnimLookups(new List<AnimatedTextureSet> { subSet }, destination); | ||
|
|
||
| // Retry - the sub-area coordinates should now match the new reference lookups | ||
| var result = AddTexture(texture, destination, isForTriangle, blendMode); | ||
| result.Animated = true; | ||
| return result; | ||
| } |
There was a problem hiding this comment.
GenerateAnimLookups() appends to _referenceAnimTextures and can be triggered at runtime for each distinct sub-rect encountered. Without any deduplication/memoization, this can grow the lookup table significantly and slow down future AddTexture calls (which linearly scan _referenceAnimTextures). Consider caching generated sub-area sets (e.g., keyed by origin set + parentRect + subRect + triangle/quad + destination) and skipping GenerateAnimLookups when the same sub-area has already been added.
| var fRect = new Rectangle2( | ||
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | ||
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); |
There was a problem hiding this comment.
The rectangle extraction logic is repeated several times using manual MathF.Min/Max chains. Rectangle2.FromCoordinates(...) already encapsulates this and reduces the risk of subtle mistakes (and makes the code much easier to read). Consider replacing these manual constructions with Rectangle2.FromCoordinates for fRect/mfRect/frameRect.
| var fRect = new Rectangle2( | |
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | |
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | |
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | |
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); | |
| var fRect = Rectangle2.FromCoordinates( | |
| frame.TexCoord0, | |
| frame.TexCoord1, | |
| frame.TexCoord2, | |
| frame.TexCoord3); |
| float relX0 = (subRect.X0 - mfRect.X0) / mfRect.Width; | ||
| float relY0 = (subRect.Y0 - mfRect.Y0) / mfRect.Height; | ||
| float relX1 = (subRect.X1 - mfRect.X0) / mfRect.Width; | ||
| float relY1 = (subRect.Y1 - mfRect.Y0) / mfRect.Height; |
There was a problem hiding this comment.
The relative coordinate calculation divides by mfRect.Width / mfRect.Height without guarding against zero-sized frames. If an animated frame has degenerate UVs (width or height == 0), this will produce NaN/Infinity and can poison subsequent lookups. Add a check to skip/continue when mfRect.Width or mfRect.Height is 0 (or within epsilon).
| // Generate reference lookups for this sub-area animation set | ||
| GenerateAnimLookups(new List<AnimatedTextureSet> { subSet }); | ||
|
|
||
| // Retry - the sub-area coordinates should now match the new reference lookups | ||
| return AddTexture(texture, isForRoom, isForTriangle, topmostAndUnpadded); | ||
| } |
There was a problem hiding this comment.
GenerateAnimLookups() appends to _referenceAnimTextures and can be triggered at runtime for each distinct sub-rect encountered. Without any deduplication/memoization, this can grow the lookup table significantly and slow down future AddTexture calls (which linearly scan _referenceAnimTextures). Consider caching generated sub-area sets (e.g., keyed by origin set + parentRect + subRect + triangle/quad) and skipping GenerateAnimLookups when the same sub-area has already been added.
| var fRect = new Rectangle2( | ||
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | ||
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); |
There was a problem hiding this comment.
The rectangle extraction logic is repeated several times using manual MathF.Min/Max chains. Rectangle2.FromCoordinates(...) already encapsulates this and reduces the risk of subtle mistakes (and makes the code much easier to read). Consider replacing these manual constructions with Rectangle2.FromCoordinates for fRect/mfRect/frameRect.
| var fRect = new Rectangle2( | |
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | |
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | |
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | |
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); | |
| var fRect = Rectangle2.FromCoordinates( | |
| frame.TexCoord0, | |
| frame.TexCoord1, | |
| frame.TexCoord2, | |
| frame.TexCoord3); |
| float relX0 = (subRect.X0 - mfRect.X0) / mfRect.Width; | ||
| float relY0 = (subRect.Y0 - mfRect.Y0) / mfRect.Height; | ||
| float relX1 = (subRect.X1 - mfRect.X0) / mfRect.Width; | ||
| float relY1 = (subRect.Y1 - mfRect.Y0) / mfRect.Height; |
There was a problem hiding this comment.
The relative coordinate calculation divides by mfRect.Width / mfRect.Height without guarding against zero-sized frames. If an animated frame has degenerate UVs (width or height == 0), this will produce NaN/Infinity and can poison subsequent lookups. Add a check to skip/continue when mfRect.Width or mfRect.Height is 0 (or within epsilon).
| foreach (var refTex in _referenceAnimTextures) | ||
| { | ||
| // UVRotate and Video animation types are incompatible with sub-area splitting | ||
| // because they rely on specific frame arrangement assumptions (vertical strip scrolling | ||
| // for UVRotate, sequential frame playback for Video) that break when coordinates | ||
| // are transformed to sub-areas. | ||
| if (refTex.Origin.IsUvRotate || refTex.Origin.AnimationType == AnimatedTextureAnimationType.Video) | ||
| continue; | ||
|
|
||
| if (GetTexInfo(fullTexture, refTex.CompiledAnimation, destination, false, blendMode, false, _animTextureLookupMargin).HasValue) | ||
| { | ||
| var origSet = refTex.Origin; | ||
| var parentRect = texture.ParentArea; | ||
| var subRect = texture.GetRect(isForTriangle); | ||
|
|
||
| // Find which frame in the set matches our ParentArea | ||
| AnimatedTextureFrame matchedFrame = null; | ||
|
|
||
| foreach (var frame in origSet.Frames) | ||
| { | ||
| if (frame.Texture != texture.Texture) | ||
| continue; | ||
|
|
||
| var fRect = new Rectangle2( | ||
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | ||
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); | ||
|
|
||
| if (MathC.WithinEpsilon(fRect.X0, parentRect.X0, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.Y0, parentRect.Y0, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.X1, parentRect.X1, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.Y1, parentRect.Y1, _animTextureLookupMargin)) | ||
| { | ||
| matchedFrame = frame; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (matchedFrame == null) | ||
| continue; | ||
|
|
||
| // Calculate relative sub-area position within the matched frame | ||
| var mfRect = new Rectangle2( | ||
| MathF.Min(matchedFrame.TexCoord0.X, MathF.Min(matchedFrame.TexCoord1.X, MathF.Min(matchedFrame.TexCoord2.X, matchedFrame.TexCoord3.X))), | ||
| MathF.Min(matchedFrame.TexCoord0.Y, MathF.Min(matchedFrame.TexCoord1.Y, MathF.Min(matchedFrame.TexCoord2.Y, matchedFrame.TexCoord3.Y))), | ||
| MathF.Max(matchedFrame.TexCoord0.X, MathF.Max(matchedFrame.TexCoord1.X, MathF.Max(matchedFrame.TexCoord2.X, matchedFrame.TexCoord3.X))), | ||
| MathF.Max(matchedFrame.TexCoord0.Y, MathF.Max(matchedFrame.TexCoord1.Y, MathF.Max(matchedFrame.TexCoord2.Y, matchedFrame.TexCoord3.Y)))); | ||
|
|
||
| float relX0 = (subRect.X0 - mfRect.X0) / mfRect.Width; | ||
| float relY0 = (subRect.Y0 - mfRect.Y0) / mfRect.Height; | ||
| float relX1 = (subRect.X1 - mfRect.X0) / mfRect.Width; | ||
| float relY1 = (subRect.Y1 - mfRect.Y0) / mfRect.Height; | ||
|
|
||
| // Create a synthetic AnimatedTextureSet with sub-area frames | ||
| var subSet = origSet.Clone(); | ||
| subSet.Frames.Clear(); | ||
|
|
||
| foreach (var frame in origSet.Frames) | ||
| { | ||
| var frameRect = new Rectangle2( | ||
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | ||
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); | ||
|
|
||
| float fw = frameRect.Width; | ||
| float fh = frameRect.Height; | ||
|
|
||
| var subFrame = frame.Clone(); | ||
| subFrame.TexCoord0 = new Vector2(frameRect.X0 + relX0 * fw, frameRect.Y0 + relY0 * fh); | ||
| subFrame.TexCoord1 = new Vector2(frameRect.X0 + relX0 * fw, frameRect.Y0 + relY1 * fh); | ||
| subFrame.TexCoord2 = new Vector2(frameRect.X0 + relX1 * fw, frameRect.Y0 + relY1 * fh); | ||
| subFrame.TexCoord3 = new Vector2(frameRect.X0 + relX1 * fw, frameRect.Y0 + relY0 * fh); | ||
|
|
||
| subSet.Frames.Add(subFrame); | ||
| } | ||
|
|
||
| // Generate reference lookups for this sub-area animation set | ||
| GenerateAnimLookups(new List<AnimatedTextureSet> { subSet }, destination); | ||
|
|
||
| // Retry - the sub-area coordinates should now match the new reference lookups | ||
| var result = AddTexture(texture, destination, isForTriangle, blendMode); | ||
| result.Animated = true; | ||
| return result; | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var frame in origSet.Frames) | ||
| { | ||
| if (frame.Texture != texture.Texture) | ||
| continue; | ||
|
|
||
| var fRect = new Rectangle2( | ||
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | ||
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); | ||
|
|
||
| if (MathC.WithinEpsilon(fRect.X0, parentRect.X0, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.Y0, parentRect.Y0, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.X1, parentRect.X1, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.Y1, parentRect.Y1, _animTextureLookupMargin)) | ||
| { | ||
| matchedFrame = frame; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var refTex in _referenceAnimTextures) | ||
| { | ||
| // UVRotate and Video animation types are incompatible with sub-area splitting | ||
| // because they rely on specific frame arrangement assumptions (vertical strip scrolling | ||
| // for UVRotate, sequential frame playback for Video) that break when coordinates | ||
| // are transformed to sub-areas. | ||
| if (refTex.Origin.IsUvRotate || refTex.Origin.AnimationType == AnimatedTextureAnimationType.Video) | ||
| continue; | ||
|
|
||
| if (GetTexInfo(fullTexture, refTex.CompiledAnimation, isForRoom, false, false, false, remapAnimatedTextures, _animTextureLookupMargin).HasValue) | ||
| { | ||
| var origSet = refTex.Origin; | ||
| var parentRect = texture.ParentArea; | ||
| var subRect = texture.GetRect(isForTriangle); | ||
|
|
||
| // Find which frame in the set matches our ParentArea | ||
| AnimatedTextureFrame matchedFrame = null; | ||
|
|
||
| foreach (var frame in origSet.Frames) | ||
| { | ||
| if (frame.Texture != texture.Texture) | ||
| continue; | ||
|
|
||
| var fRect = new Rectangle2( | ||
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | ||
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); | ||
|
|
||
| if (MathC.WithinEpsilon(fRect.X0, parentRect.X0, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.Y0, parentRect.Y0, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.X1, parentRect.X1, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.Y1, parentRect.Y1, _animTextureLookupMargin)) | ||
| { | ||
| matchedFrame = frame; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (matchedFrame == null) | ||
| continue; | ||
|
|
||
| // Calculate relative sub-area position within the matched frame | ||
| var mfRect = new Rectangle2( | ||
| MathF.Min(matchedFrame.TexCoord0.X, MathF.Min(matchedFrame.TexCoord1.X, MathF.Min(matchedFrame.TexCoord2.X, matchedFrame.TexCoord3.X))), | ||
| MathF.Min(matchedFrame.TexCoord0.Y, MathF.Min(matchedFrame.TexCoord1.Y, MathF.Min(matchedFrame.TexCoord2.Y, matchedFrame.TexCoord3.Y))), | ||
| MathF.Max(matchedFrame.TexCoord0.X, MathF.Max(matchedFrame.TexCoord1.X, MathF.Max(matchedFrame.TexCoord2.X, matchedFrame.TexCoord3.X))), | ||
| MathF.Max(matchedFrame.TexCoord0.Y, MathF.Max(matchedFrame.TexCoord1.Y, MathF.Max(matchedFrame.TexCoord2.Y, matchedFrame.TexCoord3.Y)))); | ||
|
|
||
| float relX0 = (subRect.X0 - mfRect.X0) / mfRect.Width; | ||
| float relY0 = (subRect.Y0 - mfRect.Y0) / mfRect.Height; | ||
| float relX1 = (subRect.X1 - mfRect.X0) / mfRect.Width; | ||
| float relY1 = (subRect.Y1 - mfRect.Y0) / mfRect.Height; | ||
|
|
||
| // Create a synthetic AnimatedTextureSet with sub-area frames | ||
| var subSet = origSet.Clone(); | ||
| subSet.Frames.Clear(); | ||
|
|
||
| foreach (var frame in origSet.Frames) | ||
| { | ||
| var frameRect = new Rectangle2( | ||
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | ||
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); | ||
|
|
||
| float fw = frameRect.Width; | ||
| float fh = frameRect.Height; | ||
|
|
||
| var subFrame = frame.Clone(); | ||
| subFrame.TexCoord0 = new Vector2(frameRect.X0 + relX0 * fw, frameRect.Y0 + relY0 * fh); | ||
| subFrame.TexCoord1 = new Vector2(frameRect.X0 + relX0 * fw, frameRect.Y0 + relY1 * fh); | ||
| subFrame.TexCoord2 = new Vector2(frameRect.X0 + relX1 * fw, frameRect.Y0 + relY1 * fh); | ||
| subFrame.TexCoord3 = new Vector2(frameRect.X0 + relX1 * fw, frameRect.Y0 + relY0 * fh); | ||
|
|
||
| subSet.Frames.Add(subFrame); | ||
| } | ||
|
|
||
| // Generate reference lookups for this sub-area animation set | ||
| GenerateAnimLookups(new List<AnimatedTextureSet> { subSet }); | ||
|
|
||
| // Retry - the sub-area coordinates should now match the new reference lookups | ||
| return AddTexture(texture, isForRoom, isForTriangle, topmostAndUnpadded); | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var frame in origSet.Frames) | ||
| { | ||
| if (frame.Texture != texture.Texture) | ||
| continue; | ||
|
|
||
| var fRect = new Rectangle2( | ||
| MathF.Min(frame.TexCoord0.X, MathF.Min(frame.TexCoord1.X, MathF.Min(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Min(frame.TexCoord0.Y, MathF.Min(frame.TexCoord1.Y, MathF.Min(frame.TexCoord2.Y, frame.TexCoord3.Y))), | ||
| MathF.Max(frame.TexCoord0.X, MathF.Max(frame.TexCoord1.X, MathF.Max(frame.TexCoord2.X, frame.TexCoord3.X))), | ||
| MathF.Max(frame.TexCoord0.Y, MathF.Max(frame.TexCoord1.Y, MathF.Max(frame.TexCoord2.Y, frame.TexCoord3.Y)))); | ||
|
|
||
| if (MathC.WithinEpsilon(fRect.X0, parentRect.X0, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.Y0, parentRect.Y0, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.X1, parentRect.X1, _animTextureLookupMargin) && | ||
| MathC.WithinEpsilon(fRect.Y1, parentRect.Y1, _animTextureLookupMargin)) | ||
| { | ||
| matchedFrame = frame; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
No description provided.