Merge branch 'mdl-io-triage-4'

Penumbra/Import/Models/Export/MaterialExporter.cs

@@ -89,11 +89,15 @@ public class MaterialExporter
             // TODO: handle other textures stored in the mask?
         }
 
+        // Specular extension puts colour on RGB and factor on A. We're already packing like that, so we can reuse the texture.
+        var specularImage = BuildImage(specular, name, "specular");
+
         return BuildSharedBase(material, name)
             .WithBaseColor(BuildImage(baseColor,         name, "basecolor"))
             .WithNormal(BuildImage(operation.Normal,     name, "normal"))
-            .WithSpecularColor(BuildImage(specular,      name, "specular"))
+            .WithEmissive(BuildImage(operation.Emissive, name, "emissive"), Vector3.One, 1)
-            .WithEmissive(BuildImage(operation.Emissive, name, "emissive"), Vector3.One, 1);
+            .WithSpecularFactor(specularImage, 1)
+            .WithSpecularColor(specularImage);
     }
 
     // TODO: It feels a little silly to request the entire normal here when extracting the normal only needs some of the components.
@@ -102,7 +106,7 @@ public class MaterialExporter
     {
         public Image<Rgba32> Normal    { get; } = normal.Clone();
         public Image<Rgba32> BaseColor { get; } = new(normal.Width, normal.Height);
-        public Image<Rgb24>  Specular  { get; } = new(normal.Width, normal.Height);
+        public Image<Rgba32> Specular  { get; } = new(normal.Width, normal.Height);
         public Image<Rgb24>  Emissive  { get; } = new(normal.Width, normal.Height);
 
         private Buffer2D<Rgba32> NormalBuffer
@@ -111,7 +115,7 @@ public class MaterialExporter
         private Buffer2D<Rgba32> BaseColorBuffer
             => BaseColor.Frames.RootFrame.PixelBuffer;
 
-        private Buffer2D<Rgb24> SpecularBuffer
+        private Buffer2D<Rgba32> SpecularBuffer
             => Specular.Frames.RootFrame.PixelBuffer;
 
         private Buffer2D<Rgb24> EmissiveBuffer
@@ -140,7 +144,9 @@ public class MaterialExporter
 
                 // Specular (table)
                 var lerpedSpecularColor = Vector3.Lerp(prevRow.Specular, nextRow.Specular, tableRow.Weight);
-                specularSpan[x].FromVector4(new Vector4(lerpedSpecularColor, 1));
+                // float.Lerp is .NET8 ;-; #TODO
+                var lerpedSpecularFactor = prevRow.SpecularStrength * (1.0f - tableRow.Weight) + nextRow.SpecularStrength * tableRow.Weight;
+                specularSpan[x].FromVector4(new Vector4(lerpedSpecularColor, lerpedSpecularFactor));
 
                 // Emissive (table)
                 var lerpedEmissive = Vector3.Lerp(prevRow.Emissive, nextRow.Emissive, tableRow.Weight);

Penumbra/Import/Models/Export/MeshExporter.cs

@@ -230,10 +230,19 @@ public class MeshExporter
             { "targetNames", shapeNames },
         });
 
-        var attributes = Enumerable.Range(0, 32)
+        string[] attributes = [];
-            .Where(index => ((attributeMask >> index) & 1) == 1)
+        var maxAttribute = 31 - BitOperations.LeadingZeroCount(attributeMask);
-            .Select(index => _mdl.Attributes[index])
+        if (maxAttribute < _mdl.Attributes.Length)
-            .ToArray();
+        {
+            attributes = Enumerable.Range(0, 32)
+                .Where(index => ((attributeMask >> index) & 1) == 1)
+                .Select(index => _mdl.Attributes[index])
+                .ToArray();
+        }
+        else
+        {
+            _notifier.Warning("Invalid attribute data, ignoring.");
+        }
 
         return new MeshData
         {

Penumbra/Import/Models/Export/VertexFragment.cs

@@ -11,7 +11,8 @@ and there's reason to overhaul the export pipeline.
 
 public struct VertexColorFfxiv : IVertexCustom
 {
-    [VertexAttribute("_FFXIV_COLOR", EncodingType.UNSIGNED_BYTE, false)]
+    // NOTE: We only realistically require UNSIGNED_BYTE for this, however Blender 3.6 errors on that (fixed in 4.0).
+    [VertexAttribute("_FFXIV_COLOR", EncodingType.UNSIGNED_SHORT, false)]
     public Vector4 FfxivColor;
 
     public int MaxColors => 0;
@@ -80,7 +81,7 @@ public struct VertexTexture1ColorFfxiv : IVertexCustom
     [VertexAttribute("TEXCOORD_0")]
     public Vector2 TexCoord0;
 
-    [VertexAttribute("_FFXIV_COLOR", EncodingType.UNSIGNED_BYTE, false)]
+    [VertexAttribute("_FFXIV_COLOR", EncodingType.UNSIGNED_SHORT, false)]
     public Vector4 FfxivColor;
 
     public int MaxColors => 0;
@@ -162,7 +163,7 @@ public struct VertexTexture2ColorFfxiv : IVertexCustom
     [VertexAttribute("TEXCOORD_1")]
     public Vector2 TexCoord1;
 
-    [VertexAttribute("_FFXIV_COLOR", EncodingType.UNSIGNED_BYTE, false)]
+    [VertexAttribute("_FFXIV_COLOR", EncodingType.UNSIGNED_SHORT, false)]
     public Vector4 FfxivColor;
 
     public int MaxColors => 0;

Penumbra/UI/AdvancedWindow/ModEditWindow.Models.MdlTab.cs

@@ -2,6 +2,7 @@ using Lumina.Data.Parsing;
 using OtterGui;
 using Penumbra.GameData;
 using Penumbra.GameData.Files;
+using Penumbra.Import.Models;
 using Penumbra.Import.Models.Export;
 using Penumbra.Meta.Manipulations;
 using Penumbra.String.Classes;
@@ -27,8 +28,8 @@ public partial class ModEditWindow
 
         private bool            _dirty;
         public  bool            PendingIo    { get; private set; }
-        public  List<Exception> IoExceptions { get; private set; } = [];
+        public  List<Exception> IoExceptions { get; } = [];
-        public  List<string>    IoWarnings   { get; private set; } = [];
+        public  List<string>    IoWarnings   { get; } = [];
 
         public MdlTab(ModEditWindow edit, byte[] bytes, string path)
         {
@@ -79,7 +80,7 @@ public partial class ModEditWindow
                 return;
             }
 
-            PendingIo = true;
+            BeginIo();
             var task = Task.Run(() =>
             {
                 // TODO: Is it worth trying to order results based on option priorities for cases where more than one match is found?
@@ -91,12 +92,7 @@ public partial class ModEditWindow
                     .ToList();
             });
 
-            task.ContinueWith(t =>
+            task.ContinueWith(t => { GamePaths = FinalizeIo(t); });
-            {
-                RecordIoExceptions(t.Exception);
-                GamePaths = t.Result;
-                PendingIo = false;
-            });
         }
 
         private EstManipulation[] GetCurrentEstManipulations()
@@ -132,33 +128,22 @@ public partial class ModEditWindow
                 return;
             }
 
-            PendingIo = true;
+            BeginIo();
             _edit._models.ExportToGltf(ExportConfig, Mdl, sklbPaths, ReadFile, outputPath)
-                .ContinueWith(task =>
+                .ContinueWith(FinalizeIo);
-                {
-                    RecordIoExceptions(task.Exception);
-                    if (task is { IsCompletedSuccessfully: true, Result: not null })
-                        IoWarnings = task.Result.GetWarnings().ToList();
-                    PendingIo = false;
-                });
         }
 
         /// <summary> Import a model from an interchange format. </summary>
         /// <param name="inputPath"> Disk path to load model data from. </param>
         public void Import(string inputPath)
         {
-            PendingIo = true;
+            BeginIo();
             _edit._models.ImportGltf(inputPath)
                 .ContinueWith(task =>
                 {
-                    RecordIoExceptions(task.Exception);
+                    var mdlFile = FinalizeIo(task, result => result.Item1, result => result.Item2);
-                    if (task is { IsCompletedSuccessfully: true, Result: (not null, _) })
+                    if (mdlFile != null)
-                    {
+                        FinalizeImport(mdlFile);
-                        IoWarnings = task.Result.Item2.GetWarnings().ToList();
-                        FinalizeImport(task.Result.Item1);
-                    }
-
-                    PendingIo = false;
                 });
         }
 
@@ -186,7 +171,7 @@ public partial class ModEditWindow
         /// <summary> Merge material configuration from the source onto the target. </summary>
         /// <param name="target"> Model that will be updated. </param>
         /// <param name="source"> Model to copy material configuration from. </param>
-        public void MergeMaterials(MdlFile target, MdlFile source)
+        private static void MergeMaterials(MdlFile target, MdlFile source)
         {
             target.Materials = source.Materials;
 
@@ -201,7 +186,7 @@ public partial class ModEditWindow
         /// <summary> Merge attribute configuration from the source onto the target. </summary>
         /// <param name="target"> Model that will be updated. ></param>
         /// <param name="source"> Model to copy attribute configuration from. </param>
-        public static void MergeAttributes(MdlFile target, MdlFile source)
+        private static void MergeAttributes(MdlFile target, MdlFile source)
         {
             target.Attributes = source.Attributes;
 
@@ -255,14 +240,47 @@ public partial class ModEditWindow
             target.ElementIds = [.. elementIds];
         }
 
+        private void BeginIo()
+        {
+            PendingIo = true;
+            IoWarnings.Clear();
+            IoExceptions.Clear();
+        }
+
+        private void FinalizeIo(Task<IoNotifier> task)
+            => FinalizeIo<IoNotifier, object?>(task, _ => null, notifier => notifier);
+
+        private TResult? FinalizeIo<TResult>(Task<TResult> task)
+            => FinalizeIo(task, result => result, null);
+
+        private TResult? FinalizeIo<TTask, TResult>(Task<TTask> task, Func<TTask, TResult> getResult, Func<TTask, IoNotifier>? getNotifier)
+        {
+            TResult? result = default;
+            RecordIoExceptions(task.Exception);
+            if (task is { IsCompletedSuccessfully: true, Result: not null })
+            {
+                result = getResult(task.Result);
+                if (getNotifier != null)
+                    IoWarnings.AddRange(getNotifier(task.Result).GetWarnings());
+            }
+
+            PendingIo = false;
+
+            return result;
+        }
+
         private void RecordIoExceptions(Exception? exception)
         {
-            IoExceptions = exception switch
+            switch (exception)
             {
-                null                  => [],
+                case null: break;
-                AggregateException ae => [.. ae.Flatten().InnerExceptions],
+                case AggregateException ae:
-                _                     => [exception],
+                    IoExceptions.AddRange(ae.Flatten().InnerExceptions);
-            };
+                    break;
+                default:
+                    IoExceptions.Add(exception);
+                    break;
+            }
         }
 
         /// <summary> Read a file from the active collection or game. </summary>