-
Notifications
You must be signed in to change notification settings - Fork 20
Develop #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…mCouldNotBeFound Adjusted Materials Logic
…terRevitCloses Allow to Install When Closing Revit
…portedFeatures Fix Image Gamma Unsupported By glTF
…sToDraco 86b7ejgm6/add properties to draco
…Logic 86b7gwka0/decouple gltf logic
WalkthroughRemoved legacy GLTF model types and IndexedDictionary, migrated export pipeline to glTF.Manipulator types (Node, BaseMaterial, Buffer, etc.), centralized geometry export in ProcessGeometry, updated binary/image handling and JSON serialization, and adjusted material/texture processing and UI installer flow. Changes
Sequence Diagram(s)sequenceDiagram
participant ExportContext as GlTFExportContext
participant ProcGeo as ProcessGeometry
participant BinUtils as GLTFBinaryDataUtils
participant Materials as RevitMaterials
participant FileExport
participant GltfJson
participant BinFile
Note over ExportContext: New export flow (high level)
ExportContext->>ExportContext: Start() initialize Node, BaseMaterial collections
ExportContext->>ProcGeo: OnElementEnd -> ProcessNode(element)
ProcGeo->>BinUtils: ExportVertices/Normals/UVs/Faces (append to globalBuffer)
BinUtils->>ProcGeo: return Accessor indices
ProcGeo->>Materials: Request/assign BaseMaterial for primitives
ExportContext->>FileExport: Finish() -> Run(buffers, scenes, nodes, meshes, materials, globalBuffer)
FileExport->>GltfJson: Build JSON (transformMaterials, cleanImages)
FileExport->>BinFile: Create(outputPath, globalBuffer)
BinFile->>BinFile: Write globalBuffer.ToArray() to disk
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Revit_glTF_Exporter_2025/Revit_glTF_Exporter_2025.csproj (1)
86-95: Hardcoded local paths will break CI and team builds.The
HintPathvalues reference absolute paths on a specific user's desktop (..\..\..\..\Users\User\Desktop\DracoWrapper.dll). This will fail for other developers and CI environments.Consider moving these DLLs to a project-relative location (e.g.,
lib/folder) or packaging them as NuGet packages.
🟡 Minor comments (6)
Common_glTF_Exporter/Export/Draco.cs-38-39 (1)
38-39: Include parameter name inArgumentNullException.Throwing
ArgumentNullExceptionwithout specifying which parameter is null makes debugging harder.if (string.IsNullOrEmpty(originalPath) || string.IsNullOrEmpty(tempPath)) - throw new ArgumentNullException(); + throw new ArgumentNullException( + string.IsNullOrEmpty(originalPath) ? nameof(originalPath) : nameof(tempPath));Common_glTF_Exporter/Version/LatestVersion.cs-31-38 (1)
31-38: Based on my verification of the code, the original review comment is valid and identifies a real issue. Let me now generate the rewritten review comment:
Add null guard after
JsonSerializer.Deserializeto prevent silentNullReferenceException
JsonSerializer.Deserialize<Payload>can returnnullif the JSON is empty, malformed, or the deserialization fails. Line 38 accessespayload.Updatewithout checking for null first, which will throwNullReferenceException. While the outercatchblock (lines 49–52) will catch this exception, it only logs it to Console, silently failing without proper error handling.Add a guard clause:
Payload payload = JsonSerializer.Deserialize<Payload>(myContent, options); if (payload == null) { return; }Additionally, consider moving to fully async patterns (
await client.GetAsync()on line 23) and wrappingHttpClient/HttpResponseMessageinusingblocks to avoid resource leaks, though this is a separate refactoring concern.Common_glTF_Exporter/Core/ProcessGeometry.cs-89-92 (1)
89-92: Potential exception if material UUID is not found.
GetIndexFromUUIDthrows an exception if the UUID is not present in the dictionary. Consider adding a safety check.if (preferences.materials != MaterialsEnum.nonematerials) { - primitive.material = materials.GetIndexFromUUID(kvp.Value.MaterialInfo.uuid); + if (kvp.Value.MaterialInfo != null && materials.Contains(kvp.Value.MaterialInfo.uuid)) + { + primitive.material = materials.GetIndexFromUUID(kvp.Value.MaterialInfo.uuid); + } }Common_glTF_Exporter/Materials/MaterialProperties.cs-3-3 (1)
3-3: Remove unused/incorrect importSystem.IO.Ports.
System.IO.Portsis for serial port communication and is not used in this file. This appears to be an erroneous import.-using System.IO.Ports;Common_glTF_Exporter/Export/BinFile.cs-9-10 (1)
9-10: Class is in global namespace - potential naming conflict.
BinFileis now declared without a namespace, which can cause naming conflicts and breaks the existing namespace organization (Common_glTF_Exporter.Export).+namespace Common_glTF_Exporter.Export +{ public static class BinFile { // ... existing code ... } +}Common_glTF_Exporter/Materials/MaterialTextures.cs-64-89 (1)
64-89: Remove unusedopacityparameter.The
opacityparameter is declared but never used within the method body. This appears to be leftover from the refactoring.- private static Texture createTexture(BaseMaterial material, string texturePath, Asset connectedAsset, - Asset theAsset, float opacity, int imageIndex) + private static Texture createTexture(BaseMaterial material, string texturePath, Asset connectedAsset, + Asset theAsset, int imageIndex)Also update the call site on line 54:
- Texture baseTexture = createTexture(material, texturePath, connectedAsset, theAsset, opacity, indexImage); + Texture baseTexture = createTexture(material, texturePath, connectedAsset, theAsset, indexImage);
🧹 Nitpick comments (29)
GltfInstaller/Program.cs (2)
36-37: glTF.Manipulator.dll deployment entries look consistent across all Revit yearsThe added
glTF.Manipulator.dllfile entries for 2019–2026 follow the same pattern as the other binaries and use consistent year-specific paths, which is good. No correctness issues stand out in the installer configuration here.You might optionally reduce repetition by extracting the common DLL list per year into a small helper (e.g., constructing
Direntries programmatically instead of hardcoding each year block) if this file is going to grow further, but that’s not required for this change.Also applies to: 47-48, 58-59, 69-70, 80-81, 91-92, 102-103, 115-116
36-37: Consider adding glTF.Manipulator.dll to the file‑in‑use check to avoid install/upgrade failuresYou now deploy
glTF.Manipulator.dllfor every year, butCheckFilesAreOverwritableonly checksLeia_glTF_Exporter.dll. If Revit (or another process) keepsglTF.Manipulator.dlllocked, the MSI might still hit a file‑in‑use error even after killing Revit, because you don’t pre‑check that DLL before proceeding.To make upgrades more robust, consider updating
CheckFilesAreOverwritableto coverglTF.Manipulator.dllas well (and optionally the other helper DLLs):- private static ActionResult CheckFilesAreOverwritable(string basePath) - { - basePath = Environment.ExpandEnvironmentVariables(basePath); - - for (int year = 2019; year <= 2026; year++) - { - string filePath = System.IO.Path.Combine(basePath, year.ToString(), "Leia_glTF_Exporter.dll"); - - if (!WaitForFilesToBeOverwritable(filePath)) - { - MessageBox.Show($"The file Leia_glTF_Exporter.dll {year} is still in use", "Warning"); - return ActionResult.Failure; - } - } - - return ActionResult.Success; - } + private static ActionResult CheckFilesAreOverwritable(string basePath) + { + basePath = Environment.ExpandEnvironmentVariables(basePath); + + for (int year = 2019; year <= 2026; year++) + { + string yearFolder = System.IO.Path.Combine(basePath, year.ToString()); + + foreach (var fileName in new[] { "Leia_glTF_Exporter.dll", "glTF.Manipulator.dll" }) + { + string filePath = System.IO.Path.Combine(yearFolder, fileName); + + if (!WaitForFilesToBeOverwritable(filePath)) + { + MessageBox.Show($"The file {fileName} {year} is still in use", "Warning"); + return ActionResult.Failure; + } + } + } + + return ActionResult.Success; + }This keeps the behavior for the main exporter assembly and extends the same safety to the new runtime dependency.
Also applies to: 47-48, 58-59, 69-70, 80-81, 91-92, 102-103, 115-116, 214-227
Common_glTF_Exporter/Windows/ProgressBarWindow.xaml.cs (1)
34-39: Remove unnecessary variable duplication.The
maxValueDuplicatedvariable is an exact copy of themaxValueparameter with no transformation applied. This duplication adds no value and reduces code clarity.Apply this diff to use
maxValuedirectly:- double maxValueDuplicated = maxValue; - ProgressBarWindow.ViewModel.ProgressBarGraphicValue = maxValueDuplicated * 0.07; + ProgressBarWindow.ViewModel.ProgressBarGraphicValue = maxValue * 0.07; ProgressBarWindow.ViewModel.ProgressBarValue = currentValue; ProgressBarWindow.ViewModel.Message = message; ProgressBarWindow.ViewModel.Action = "Cancel"; - ProgressBarWindow.ViewModel.ProgressBarMax = maxValueDuplicated; + ProgressBarWindow.ViewModel.ProgressBarMax = maxValue;Common_glTF_Exporter/Materials/BitmapsUtils.cs (3)
39-53: Add input validation and document exception behavior.The method lacks validation for:
pathparameter (null, empty, or non-existent file)imageFormatparameter (null or unsupported format)Without validation, callers will receive cryptic exceptions like
ArgumentNullExceptionorFileNotFoundException. Consider adding validation or clearly documenting that callers must ensure valid inputs.Apply this diff to add basic validation:
public static byte[] CleanGamma(string path, ImageFormat imageFormat) { + if (string.IsNullOrWhiteSpace(path)) + throw new ArgumentException("Path cannot be null or empty", nameof(path)); + if (!File.Exists(path)) + throw new FileNotFoundException("Image file not found", path); + if (imageFormat == null) + throw new ArgumentNullException(nameof(imageFormat)); + using (var original = new Bitmap(path))
45-45: Document alpha channel loss.The use of
PixelFormat.Format24bppRgbdiscards the alpha channel from the original image. If this is intentional (e.g., glTF base color textures don't need alpha), document this behavior in the XML comments to inform callers. If alpha preservation is needed, consider usingPixelFormat.Format32bppArgbinstead.
46-48: Consider setting Graphics quality parameters.The Graphics object uses default quality settings. For image processing, you may want to specify interpolation mode and compositing quality to ensure high-quality output:
g.InterpolationMode = System.Drawing.Drawing2D.InterpolationMode.HighQualityBicubic; g.CompositingQuality = System.Drawing.Drawing2D.CompositingQuality.HighQuality;This ensures optimal rendering quality when the image is redrawn.
Common_glTF_Exporter/Export/Draco.cs (5)
13-23: Consider using properties instead of public fields.Public mutable fields reduce encapsulation. Auto-properties would be more idiomatic C#.
internal sealed class GlbData { - public string Json; - public byte[] Bin; + public string Json { get; } + public byte[] Bin { get; } public GlbData(string json, byte[] bin) { Json = json; Bin = bin ?? new byte[0]; } }
374-381: Empty catch block silently ignores file deletion failures.While this may be intentional for cleanup, consider logging the failure for debugging purposes in case files aren't cleaned up as expected.
513-516: UsePath.ChangeExtensioninstead of string concatenation for file extensions.String concatenation for paths is fragile.
Path.ChangeExtensionhandles edge cases more robustly.-fileToCompress = preferences.path + ".gltf"; -fileToCompressTemp = preferences.path + "Temp.gltf"; +fileToCompress = Path.ChangeExtension(preferences.path, ".gltf"); +fileToCompressTemp = Path.ChangeExtension(preferences.path + "Temp", ".gltf"); -files.Add(preferences.path + ".bin"); +files.Add(Path.ChangeExtension(preferences.path, ".bin"));
564-574: Empty catch block hides file deletion failures.If file deletion fails (e.g., file locked), the operation continues without warning. Consider logging these failures for troubleshooting.
169-248: Duplicated logic for mergingextensionsUsedandextensionsRequired.The two blocks (lines 171-207 and 209-245) have identical structure. Consider extracting to a helper method.
private static void MergeStringArray(JsonObject src, JsonObject dst, string arrayName) { if (!src.TryGetPropertyValue(arrayName, out JsonNode srcNode)) return; if (srcNode is not JsonArray srcArr || srcArr.Count == 0) return; JsonArray dstArr; if (dst.TryGetPropertyValue(arrayName, out JsonNode dstNode) && dstNode is JsonArray) dstArr = (JsonArray)dstNode; else { dstArr = new JsonArray(); dst[arrayName] = dstArr; } var set = new HashSet<string>(StringComparer.Ordinal); foreach (JsonNode t in dstArr) if (t != null) set.Add(t.GetValue<string>()); foreach (JsonNode t in srcArr) { if (t == null) continue; string name = t.GetValue<string>(); if (set.Add(name)) dstArr.Add(name); } }Common_glTF_Exporter/Materials/RevitMaterials.cs (2)
10-12: Remove unused imports.
System.IO.Ports,System.Windows.Controls, andSystem.Windows.Media.Media3Dappear to be unused in this file.using Common_glTF_Exporter.Materials; using Common_glTF_Exporter.Model; -using System.IO.Ports; -using System.Windows.Controls; -using System.Windows.Media.Media3D; using Material = Autodesk.Revit.DB.Material;
72-81: Redundant null check and duplicate assignment ofbaseColorFactor.
The
revitMaterial != nullcheck on line 72 is always true here since this method is only called from line 47 whererevitMaterialwas already validated as non-null.Line 75 sets
baseColorFactorto the default, but lines 78-81 immediately checkhasTextureand set it to the same default value again. The logic can be simplified.- if (revitMaterial != null && preferences.materials == MaterialsEnum.textures) + if (preferences.materials == MaterialsEnum.textures) { baseNTintColour = MaterialTextures.SetMaterialTextures(revitMaterial, material, doc, opacity, textures, images); - material.baseColorFactor = MaterialProperties.GetDefaultColour(opacity); } if (material.hasTexture) { material.baseColorFactor = MaterialProperties.GetDefaultColour(opacity); } else { material.baseColorFactor = MaterialProperties.SetMaterialColour(node, opacity, baseNTintColour.Item1, baseNTintColour.Item2); }Common_glTF_Exporter/Windows/VersionWindow.xaml (1)
114-137: Two-button update layout is wired correctly; consider minor style consistencyThe new
StackPaneland its two buttons are correctly hooked toButton_UpdateOnExitandButton_ClickAsync, and alignment/margins look reasonable. You may optionally align styling between the two buttons (e.g., setForeground/Backgroundconsistently) so they render identically apart from content.Common_glTF_Exporter/ExternalApplication.cs (1)
8-8: Installer-on-exit flow looks sound; optionally clearInstallerRouteafter useThe new
InstallerRoutefield andOnShutdowncheck integrate cleanly withRunLocalFile.Action, and thestring.IsNullOrEmptyguard plus internalFile.Existsavoid runtime errors if the file is missing. You might optionally setInstallerRoute = nullafter callingRunLocalFile.Actionto make the one-shot intent explicit, but it is not required for correctness givenOnShutdownruns once per add-in lifetime.Also applies to: 29-29, 60-67
Common_glTF_Exporter/Materials/MaterialTexture.cs (1)
1-11: MaterialTexture DTO is fine; consider naming/namespace polishThis simple data holder is valid as-is. Two optional cleanups you might consider:
- Rename
Fadevalue→FadeValuefor standard PascalCase.- Either move the class under a
Common_glTF_Exporter.Materialsnamespace or add a brief comment explaining why it lives in.Corewhile the file sits inMaterials/, to avoid confusion for future readers.Common_glTF_Exporter/Export/GlbFile.cs (1)
9-9: New GLB assembly path looks correct; drop leftover debug variableThe switch to a single
GLTFBinaryDatabuffer andGlbBinInfo.Get(binaryFileData)matches the new global-buffer design and is consistent with the referencedGlbBinInfoimplementation.
int lenggg = jsonChunk.Length;is now unused and can be safely removed to keep the method clean:- byte[] jsonChunk = GlbJsonInfo.Get(json); - int lenggg = jsonChunk.Length; - byte[] binChunk = GlbBinInfo.Get(binaryFileData); + byte[] jsonChunk = GlbJsonInfo.Get(json); + byte[] binChunk = GlbBinInfo.Get(binaryFileData);Also ensure all callers were updated to pass a single
GLTFBinaryDatainstance instead of a list.Also applies to: 15-20
Common_glTF_Exporter/Windows/VersionWindow.xaml.cs (1)
33-40: Installer folder helper and update-on-exit flow look correct; duplication could be reduced
VersionFolder()centralizes the temp path logic nicely, and both flows (“Update Now” vs “Update on Exit”) correctly:
- Hide the window,
- Ensure/clean
%TEMP%\LeiaInstaller\<version>,- Download the installer there,
- Either run it immediately or stash the path in
ExternalApplication.InstallerRoute.Because
DownloadFile.FromServerandfileLocationconstruction are duplicated between the two handlers, you might consider a small private helper likeasync Task<string> DownloadInstallerAsync()that returnsfileLocationso the two event handlers only decide whether to callRunLocalFile.Actionor setInstallerRoute. Not required for correctness, just for DRYness.Also applies to: 43-52, 54-62
Common_glTF_Exporter/Export/BufferConfig.cs (1)
5-7: BufferView/Buffer migration keeps existing layout behavior; consider tiny cleanupUsing
List<BufferView>andList<Buffer>fromglTF.Manipulator.Schemawhile preserving the existing offset/length math keeps the previous behavior intact and matches the standard glTF schema (buffer index, byteOffset, byteLength).Two optional cleanups:
currentBufferis never updated, so the checkif (view.buffer != currentBuffer)is effectivelyview.buffer != 0given the earlierEquals(0)guard; you could inline that and dropcurrentBufferfor clarity.- If you no longer need multiple buffers, a brief comment explaining that all data are collapsed into a single buffer (index 0) would help future maintainers.
Also applies to: 14-47
Common_glTF_Exporter/Export/GlbBinInfo.cs (1)
13-14: Consider using English for code comments.The comments are in Spanish. For consistency and broader team accessibility, consider using English throughout.
Also applies to: 16-17, 19-20, 28-29, 32-33, 37-38
Common_glTF_Exporter/Core/ProcessGeometry.cs (3)
11-13: Remove unused imports.
System.Text,System.Windows.Forms, andSystem.Xml.Linqare not used in this file.using System; using System.Collections.Generic; using System.Linq; -using System.Text; -using System.Windows.Forms; -using System.Xml.Linq;
66-66: Redundant variable assignment.
gLTFBinaryDataElementis just an alias forglobalBufferand is never reassigned. UseglobalBufferdirectly.- MeshPrimitive primitive = new MeshPrimitive(); - GLTFBinaryData gLTFBinaryDataElement = globalBuffer; + MeshPrimitive primitive = new MeshPrimitive();Then replace all occurrences of
gLTFBinaryDataElementwithglobalBufferin lines 70, 73, 77, 82, and 85.
19-21: Method has too many parameters (10).This method signature has 10 parameters, which makes it difficult to use and maintain. Consider grouping related parameters into a context or options object.
Common_glTF_Exporter/Export/BinFile.cs (1)
19-24: Simplify file writing - BufferedStream and explicit Flush are unnecessary.When writing a single byte array,
BufferedStreamprovides no benefit. TheDispose()call (viausing) already flushes, so explicitFlush()is redundant.- using (FileStream f = File.Create(filename)) - using (var writer = new BinaryWriter(new BufferedStream(f))) - { - writer.Write(data); - writer.Flush(); - } + File.WriteAllBytes(filename, data);Common_glTF_Exporter/Utils/MaterialUtils.cs (1)
30-30: Wasteful allocation when material is null.A new
BaseMaterialis created on line 30 but immediately replaced on line 38 when the Revit material is null. Consider deferring the allocation.- BaseMaterial gl_mat = new BaseMaterial(); + BaseMaterial gl_mat; Autodesk.Revit.DB.Material material = GetMeshMaterial(doc, mesh); if (preferences.materials == MaterialsEnum.materials || preferences.materials == MaterialsEnum.textures) { if (material == null) { gl_mat = GLTFExportUtils.GetGLTFMaterial(materials); } else { - gl_mat.doubleSided = doubleSided; + gl_mat = new BaseMaterial(); + gl_mat.doubleSided = doubleSided; // ... rest of initialization } } + else + { + gl_mat = new BaseMaterial(); + } return gl_mat;Also applies to: 38-38
Common_glTF_Exporter/Materials/MaterialTextures.cs (1)
91-117: Consider combining existence check and index lookup.The current implementation iterates the list twice—once with
Any()and again withFindIndex(). For better efficiency, useFindIndexdirectly and check if the result is-1.- bool checkIfImageExists = images.Any(x => x.uuid == texturePath); - - if (checkIfImageExists) - { - int index = images.FindIndex(x => x.uuid == texturePath); - - return index; - } - else + int existingIndex = images.FindIndex(x => x.uuid == texturePath); + if (existingIndex >= 0) + { + return existingIndex; + }Common_glTF_Exporter/Export/GltfJson.cs (1)
103-142: Minor formatting inconsistency - extra indentation.The method logic is correct for transforming
BaseMaterialto glTFMaterial, properly handling PBR properties and texture extensions. However, lines 115-139 have inconsistent indentation (extra indent level).- PBR pbrMetallicRoughness = new PBR(); - pbrMetallicRoughness.metallicFactor = baseMat.metallicFactor; + PBR pbrMetallicRoughness = new PBR(); + pbrMetallicRoughness.metallicFactor = baseMat.metallicFactor; // ... same for remaining lines in the blockCommon_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs (2)
66-67: Remove unused class-level constants.These constants (
VEC3_STR,POSITION_STR) are shadowed by identical local declarations inExportVertices(lines 76-77) and never used. The same applies toNORMAL_STRat line 132.- - const string VEC3_STR = "VEC3"; - const string POSITION_STR = "POSITION";Also remove line 132:
- const string NORMAL_STR = "NORMALS";
153-192: Inconsistent comment language.Several comments are in Spanish while the rest of the codebase uses English. Consider translating for consistency:
- Line 153: "Min/Max SOLO para estas normales" → "Min/Max for these normals only"
- Line 159: "Append normals al buffer global" → "Append normals to global buffer"
- Line 163: "Crear bufferView" → "Create bufferView"
- Line 175: "Crear accessor" → "Create accessor"
- Line 192: "Asignar al primitive" → "Assign to primitive"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
Common_glTF_Exporter/Common_glTF_Exporter.projitems(1 hunks)Common_glTF_Exporter/Core/GLTFAccessor.cs(0 hunks)Common_glTF_Exporter/Core/GLTFAttribute.cs(0 hunks)Common_glTF_Exporter/Core/GLTFBinaryData.cs(0 hunks)Common_glTF_Exporter/Core/GLTFBuffer.cs(0 hunks)Common_glTF_Exporter/Core/GLTFBufferView.cs(0 hunks)Common_glTF_Exporter/Core/GLTFExtras.cs(0 hunks)Common_glTF_Exporter/Core/GLTFImage.cs(0 hunks)Common_glTF_Exporter/Core/GLTFMaterial.cs(0 hunks)Common_glTF_Exporter/Core/GLTFMesh.cs(0 hunks)Common_glTF_Exporter/Core/GLTFMeshPrimitive.cs(0 hunks)Common_glTF_Exporter/Core/GLTFNode.cs(0 hunks)Common_glTF_Exporter/Core/GLTFScene.cs(0 hunks)Common_glTF_Exporter/Core/GLTFTextureInfo.cs(0 hunks)Common_glTF_Exporter/Core/GLTFVersion.cs(0 hunks)Common_glTF_Exporter/Core/GlTFExportContext.cs(10 hunks)Common_glTF_Exporter/Core/IndexedDictionary.cs(2 hunks)Common_glTF_Exporter/Core/ProcessGeometry.cs(1 hunks)Common_glTF_Exporter/Core/glTF.cs(0 hunks)Common_glTF_Exporter/Core/glTFPBR.cs(0 hunks)Common_glTF_Exporter/EportUtils/ElementValidations.cs(1 hunks)Common_glTF_Exporter/EportUtils/GLTFNodeActions.cs(0 hunks)Common_glTF_Exporter/Export/BinFile.cs(1 hunks)Common_glTF_Exporter/Export/BufferConfig.cs(2 hunks)Common_glTF_Exporter/Export/Draco.cs(3 hunks)Common_glTF_Exporter/Export/FileExport.cs(2 hunks)Common_glTF_Exporter/Export/GlbBinInfo.cs(1 hunks)Common_glTF_Exporter/Export/GlbFile.cs(1 hunks)Common_glTF_Exporter/Export/GltfJson.cs(3 hunks)Common_glTF_Exporter/Export/RevitGrids.cs(3 hunks)Common_glTF_Exporter/ExternalApplication.cs(3 hunks)Common_glTF_Exporter/Materials/BitmapsUtils.cs(1 hunks)Common_glTF_Exporter/Materials/MaterialProperties.cs(1 hunks)Common_glTF_Exporter/Materials/MaterialTexture.cs(1 hunks)Common_glTF_Exporter/Materials/MaterialTextures.cs(1 hunks)Common_glTF_Exporter/Materials/RevitMaterials.cs(2 hunks)Common_glTF_Exporter/Model/GeometryDataObject.cs(0 hunks)Common_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs(1 hunks)Common_glTF_Exporter/Utils/MaterialUtils.cs(1 hunks)Common_glTF_Exporter/Utils/glTFExportUtils.cs(2 hunks)Common_glTF_Exporter/Version/LatestVersion.cs(2 hunks)Common_glTF_Exporter/ViewModel/ProgressBarWindowViewModel.cs(1 hunks)Common_glTF_Exporter/Windows/MainWindow.xaml.cs(2 hunks)Common_glTF_Exporter/Windows/ProgressBarWindow.xaml.cs(1 hunks)Common_glTF_Exporter/Windows/VersionWindow.xaml(1 hunks)Common_glTF_Exporter/Windows/VersionWindow.xaml.cs(2 hunks)GltfInstaller/Program.cs(8 hunks)Revit_glTF_Exporter_2019/Revit_glTF_Exporter_2019.csproj(1 hunks)Revit_glTF_Exporter_2020/Revit_glTF_Exporter_2020.csproj(1 hunks)Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj(1 hunks)Revit_glTF_Exporter_2022/Revit_glTF_Exporter_2022.csproj(1 hunks)Revit_glTF_Exporter_2023/Revit_glTF_Exporter_2023.csproj(1 hunks)Revit_glTF_Exporter_2024/Revit_glTF_Exporter_2024.csproj(1 hunks)Revit_glTF_Exporter_2025/Revit_glTF_Exporter_2025.csproj(1 hunks)Revit_glTF_Exporter_2026/Revit_glTF_Exporter_2026.csproj(1 hunks)
💤 Files with no reviewable changes (18)
- Common_glTF_Exporter/Core/GLTFScene.cs
- Common_glTF_Exporter/Core/GLTFExtras.cs
- Common_glTF_Exporter/Core/GLTFBufferView.cs
- Common_glTF_Exporter/Core/GLTFMeshPrimitive.cs
- Common_glTF_Exporter/Core/GLTFBuffer.cs
- Common_glTF_Exporter/Core/GLTFAccessor.cs
- Common_glTF_Exporter/Core/GLTFTextureInfo.cs
- Common_glTF_Exporter/Core/GLTFBinaryData.cs
- Common_glTF_Exporter/Core/GLTFVersion.cs
- Common_glTF_Exporter/Core/GLTFAttribute.cs
- Common_glTF_Exporter/Model/GeometryDataObject.cs
- Common_glTF_Exporter/Core/GLTFMesh.cs
- Common_glTF_Exporter/Core/GLTFMaterial.cs
- Common_glTF_Exporter/EportUtils/GLTFNodeActions.cs
- Common_glTF_Exporter/Core/GLTFNode.cs
- Common_glTF_Exporter/Core/GLTFImage.cs
- Common_glTF_Exporter/Core/glTF.cs
- Common_glTF_Exporter/Core/glTFPBR.cs
🧰 Additional context used
🧬 Code graph analysis (18)
Common_glTF_Exporter/ExternalApplication.cs (1)
Common_glTF_Exporter/Version/RunLocalFile.cs (2)
RunLocalFile(6-15)Action(8-14)
Common_glTF_Exporter/Export/GlbFile.cs (5)
Common_glTF_Exporter/Export/BinFile.cs (1)
Create(14-25)Common_glTF_Exporter/Windows/MainWindow/Preferences.cs (1)
Preferences(26-60)Common_glTF_Exporter/Export/GlbJsonInfo.cs (2)
GlbJsonInfo(9-44)Get(11-43)Common_glTF_Exporter/Export/GlbBinInfo.cs (2)
Get(9-41)GlbBinInfo(7-42)Common_glTF_Exporter/Export/GlbHeaderInfo.cs (1)
Get(11-22)
Common_glTF_Exporter/Materials/MaterialTexture.cs (1)
Common_glTF_Exporter/Materials/MaterialTextures.cs (1)
Autodesk(20-62)
Common_glTF_Exporter/Windows/VersionWindow.xaml.cs (3)
Common_glTF_Exporter/Version/DownloadFile.cs (1)
DownloadFile(13-124)Common_glTF_Exporter/ExternalApplication.cs (1)
ExternalApplication(20-198)Common_glTF_Exporter/Utils/DirectoryUtils.cs (3)
DirectoryUtils(5-42)CreateDirectoryIfNotExists(10-21)DeleteFilesInDirectoyy(34-41)
Common_glTF_Exporter/EportUtils/ElementValidations.cs (3)
Common_glTF_Exporter/Materials/MaterialTextures.cs (1)
Autodesk(20-62)Common_glTF_Exporter/Export/RevitGrids.cs (1)
Export(22-74)GltfInstaller/Dialogs/FeaturesDialog.xaml.cs (1)
Node(47-57)
Common_glTF_Exporter/Export/GlbBinInfo.cs (2)
Common_glTF_Exporter/Export/GlbJsonInfo.cs (1)
Get(11-43)Common_glTF_Exporter/Model/GlbComponents.cs (3)
GlbBin(37-48)ChunkType(28-31)ChunkType(41-44)
Common_glTF_Exporter/Windows/MainWindow.xaml.cs (2)
Common_glTF_Exporter/Utils/ExportLog.cs (2)
ExportLog(7-56)Write(26-37)Common_glTF_Exporter/Windows/ProgressBarWindow.xaml.cs (3)
ProgressBarWindow(13-87)ProgressBarWindow(15-21)ProgressBarWindow(29-44)
Common_glTF_Exporter/Utils/MaterialUtils.cs (3)
Common_glTF_Exporter/Materials/MaterialTextures.cs (1)
Autodesk(20-62)Common_glTF_Exporter/Utils/glTFExportUtils.cs (3)
BaseMaterial(20-23)BaseMaterial(25-37)GLTFExportUtils(14-148)Common_glTF_Exporter/Materials/MaterialProperties.cs (3)
List(26-52)List(55-58)List(70-77)
Common_glTF_Exporter/Core/ProcessGeometry.cs (3)
Common_glTF_Exporter/Core/IndexedDictionary.cs (4)
IndexedDictionary(12-180)IndexedDictionary(163-179)AddOrUpdateCurrent(70-83)GetIndexFromUUID(128-142)Common_glTF_Exporter/Utils/Util.cs (2)
Util(10-344)ElementDescription(167-211)Common_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs (6)
GLTFBinaryDataUtils(11-345)ExportUVs(253-314)ExportVertices(69-128)ExportNormals(134-196)ExportBatchId(198-250)ExportFaces(13-63)
Common_glTF_Exporter/Materials/MaterialTextures.cs (3)
Common_glTF_Exporter/Materials/AssetProperties.cs (8)
Autodesk(62-84)Autodesk(173-199)Asset(25-44)GetTexturePath(86-117)GetFade(201-211)GetScale(132-150)GetOffset(152-171)GetRotationRadians(119-130)Common_glTF_Exporter/Materials/RevitMaterials.cs (2)
BaseMaterial(25-53)BaseMaterial(59-88)Common_glTF_Exporter/Materials/BitmapsUtils.cs (4)
ImageFormat(14-31)BitmapsUtils(12-164)CleanGamma(39-53)BlendImageWithColor(55-142)
Common_glTF_Exporter/Core/IndexedDictionary.cs (1)
Common_glTF_Exporter/Export/GltfJson.cs (2)
List(86-101)List(103-142)
Common_glTF_Exporter/Version/LatestVersion.cs (1)
Common_glTF_Exporter/Model/Payload.cs (1)
Payload(9-18)
Common_glTF_Exporter/Materials/RevitMaterials.cs (7)
Common_glTF_Exporter/Utils/glTFExportUtils.cs (2)
BaseMaterial(20-23)BaseMaterial(25-37)Common_glTF_Exporter/Service/RevitCollectorService.cs (1)
Document(23-26)Common_glTF_Exporter/Core/IndexedDictionary.cs (4)
IndexedDictionary(12-180)IndexedDictionary(163-179)Contains(118-121)AddOrUpdateCurrentMaterial(92-111)Common_glTF_Exporter/Materials/MaterialProperties.cs (5)
List(26-52)List(55-58)List(70-77)MaterialProperties(14-95)SetProperties(18-24)Common_glTF_Exporter/Materials/MaterialTextures.cs (3)
Texture(64-89)Autodesk(20-62)MaterialTextures(17-118)Common_glTF_Exporter/Materials/AssetProperties.cs (2)
Autodesk(62-84)Autodesk(173-199)Common_glTF_Exporter/Utils/Collectors.cs (1)
Material(9-19)
Common_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs (4)
Common_glTF_Exporter/Materials/MaterialProperties.cs (3)
List(26-52)List(55-58)List(70-77)Common_glTF_Exporter/Export/GltfJson.cs (2)
List(86-101)List(103-142)Common_glTF_Exporter/Utils/Util.cs (5)
List(219-241)Util(10-344)GetScalarMinMax(98-102)GetVec3MinMax(89-96)GetVec2MinMax(314-331)Common_glTF_Exporter/Utils/GeometryUtils.cs (1)
List(9-24)
Common_glTF_Exporter/Export/GltfJson.cs (7)
Common_glTF_Exporter/Materials/MaterialTextures.cs (2)
Autodesk(20-62)Texture(64-89)Common_glTF_Exporter/Materials/MaterialProperties.cs (3)
List(26-52)List(55-58)List(70-77)Common_glTF_Exporter/Transform/ModelTraslation.cs (1)
List(10-44)Common_glTF_Exporter/Transform/ModelRotation.cs (1)
List(9-19)Common_glTF_Exporter/Transform/ModelScale.cs (1)
List(12-16)Common_glTF_Exporter/Materials/RevitMaterials.cs (2)
BaseMaterial(25-53)BaseMaterial(59-88)Common_glTF_Exporter/Utils/glTFExportUtils.cs (2)
BaseMaterial(20-23)BaseMaterial(25-37)
Common_glTF_Exporter/Export/RevitGrids.cs (2)
Common_glTF_Exporter/Core/IndexedDictionary.cs (2)
IndexedDictionary(12-180)IndexedDictionary(163-179)Common_glTF_Exporter/Utils/Util.cs (2)
Dictionary(254-312)Util(10-344)
Common_glTF_Exporter/Utils/glTFExportUtils.cs (5)
Common_glTF_Exporter/Utils/MaterialUtils.cs (2)
Autodesk(14-26)BaseMaterial(28-53)Common_glTF_Exporter/Materials/MaterialTextures.cs (1)
Autodesk(20-62)Common_glTF_Exporter/Materials/RevitMaterials.cs (2)
BaseMaterial(25-53)BaseMaterial(59-88)Common_glTF_Exporter/Core/IndexedDictionary.cs (3)
IndexedDictionary(12-180)IndexedDictionary(163-179)AddOrUpdateCurrent(70-83)Common_glTF_Exporter/Export/GltfJson.cs (2)
List(86-101)List(103-142)
Common_glTF_Exporter/Core/GlTFExportContext.cs (4)
Common_glTF_Exporter/Materials/RevitMaterials.cs (3)
BaseMaterial(25-53)BaseMaterial(59-88)RevitMaterials(21-89)Common_glTF_Exporter/Utils/glTFExportUtils.cs (3)
BaseMaterial(20-23)BaseMaterial(25-37)GLTFExportUtils(14-148)Common_glTF_Exporter/Core/IndexedDictionary.cs (4)
IndexedDictionary(12-180)IndexedDictionary(163-179)AddOrUpdateCurrentMaterial(92-111)Reset(155-161)Common_glTF_Exporter/Core/ProcessGeometry.cs (2)
ProcessGeometry(17-104)ProcessNode(19-103)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (37)
Common_glTF_Exporter/Windows/MainWindow.xaml.cs (1)
94-97: LGTM: Consistent use of element count.Introducing the
elemInViewvariable improves consistency throughout the method by using the same value for logging, progress bar initialization, and final progress updates. This also avoids repeated property access toelementsInView.Count.Also applies to: 115-115
Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj (1)
73-87: Based on my verification, the serialization migration from Newtonsoft.Json to System.Text.Json has been completed successfully across the codebase:Verification Results:
- Zero Newtonsoft.Json references found anywhere in the codebase (no imports, no API usage, no comments)
- System.Text.Json properly implemented in:
Common_glTF_Exporter/Export/GltfJson.cs– Core serialization withJsonSerializerOptionsandJsonIgnoreCondition.WhenWritingNullCommon_glTF_Exporter/Version/LatestVersion.cs– API payload deserializationCommon_glTF_Exporter/Export/Draco.cs– JSON node manipulation- glTF.Manipulator extensively integrated for glTF schema types (Mesh, Material, Buffer, Node, Version) across 30+ files, handling domain-specific serialization
- .NET Framework 4.8 compatibility confirmed – System.Text.Json is built-in and the codebase uses only supported features (
JsonIgnoreCondition.WhenWritingNull, etc.)The migration is complete with no actionable items remaining.
Revit_glTF_Exporter_2024/Revit_glTF_Exporter_2024.csproj (1)
69-71: Newtonsoft.Json removal has been successfully completed across the codebase.Verification confirms:
- ✓ No remaining
Newtonsoft.JsonNuGet references in any .csproj files- ✓ No stale
using Newtonsoft.Jsondirectives in source files- ✓ No usage of
JsonConvert(Newtonsoft.Json API)- ✓ System.Text.Json is actively used throughout the codebase (verified in LatestVersion.cs, Draco.cs, GltfJson.cs, and others)
Revit_glTF_Exporter_2026/Revit_glTF_Exporter_2026.csproj (1)
78-78: The glTF.Manipulator package is maintained by E-verse as a private/internal package and is not available on the public NuGet registry.The version
2025.11.24.2122is consistently used across all project files (2023–2026) and cannot be verified through public channels. This is expected behavior for private organizational packages. The version is beyond the knowledge cutoff (March 2025), so current stability cannot be independently assessed without access to E-verse's private NuGet feed. Verification of this package's stability and .NET 8.0 compatibility requires internal E-verse testing and validation processes rather than public registry checks.Revit_glTF_Exporter_2020/Revit_glTF_Exporter_2020.csproj (1)
126-128: glTF.Manipulator package version 2025.11.24.2122 is verified as compatible with .NET Framework 4.8.Based on .nuspec metadata:
- Package targets
.NETStandard2.0andnet8.0, making it compatible with .NET Framework 4.8 (Standard 2.0 supports .NET Framework 4.6.2+)- Dependency:
System.Text.Json 8.0.4is compatible with .NET Framework 4.8 via NuGet package- Published by e-verse (the maintaining organization), MIT-licensed
- Version 2025.11.24.2122 is a stable release (not pre-release), dated Nov 24, 2025
- No security advisories or known critical issues found
The package dependency is appropriate for the System.Text.Json migration outlined in this PR.
Common_glTF_Exporter/Export/Draco.cs (1)
52-75: No error handling for file operations and JSON parsing.If
originalGltfortempGltfdon't exist, or contain malformed JSON, this will throw unhandled exceptions. Consider validating file existence or wrapping in try-catch with meaningful error messages.Revit_glTF_Exporter_2022/Revit_glTF_Exporter_2022.csproj (1)
76-78: The original review contains an incorrect claim that can be removed.The glTF.Manipulator package version
2025.11.24.2122is published on NuGet (MIT licensed, by e-verse) and properly integrated. Verification confirms:
- No Newtonsoft.Json references exist in any project files (the original review's claim about removal is unfounded)
- System.Text.Json is already in use in the shared Common_glTF_Exporter project for glTF JSON serialization
- glTF.Manipulator is explicitly included in the installer deployment (GltfInstaller/Program.cs)
- No version conflicts with existing dependencies (Revit API 2022.1.0, Draco_transcoder 3.3.4, MeshOptimizer_glTFPack 1.1.0)
The package addition aligns with the stated PR objective to migrate to the glTF.Manipulator library types.
Common_glTF_Exporter/Core/IndexedDictionary.cs (1)
163-179: LGTM!The
Clonemethod correctly iterates over the internal dictionary, applies the clone function to each element, and preserves the key-to-element mapping in the newIndexedDictionary.Common_glTF_Exporter/Materials/RevitMaterials.cs (1)
25-53: LGTM onProcessMaterialstructure.The caching logic correctly checks for existing materials before creating new ones, and properly delegates to the
Exportmethod or retrieves the default material as needed.Common_glTF_Exporter/Common_glTF_Exporter.projitems (1)
12-13: LGTM!The new compile items for
ProcessGeometry.csandMaterialTexture.csalign with the refactoring to centralize geometry processing and material handling using the newglTF.Manipulatortypes.Common_glTF_Exporter/Core/GlTFExportContext.cs (5)
1-13: LGTM on import updates.The updated imports and the
Buffertype alias correctly reflect the migration toglTF.Manipulatorschema types.
53-67: LGTM on field type updates.The field declarations are correctly updated to use the new
glTF.Manipulator.Schematypes (Node,BaseMaterial,Texture,glTFImage,Scene,Mesh,Buffer,BufferView,Accessor), aligning with the library migration.
132-146: LGTM on buffer creation and image export.The buffer setup correctly creates a new
Buffer, conditionally exports image data when textures are enabled, and sets thebyteLengthfrom the global buffer's current offset.
214-218: LGTM on geometry processing delegation.The delegation to
ProcessGeometry.ProcessNodecentralizes the node/mesh creation logic appropriately. The reset ofcurrentGeometryandcurrentVerticesafter processing is correct.
273-281: LGTM on UV coordinate handling.The UV flipping logic (
1.0 - uv.V) is correctly applied, and the newGltfUVtype is properly instantiated with the transformed coordinates.Revit_glTF_Exporter_2025/Revit_glTF_Exporter_2025.csproj (1)
78-78: I'll verify the glTF.Manipulator package availability on NuGet and examine the context of the changes in the csproj file.
<function_calls>
#!/bin/bashCheck if glTF.Manipulator package exists on NuGet
echo "=== Checking glTF.Manipulator package on NuGet ==="
curl -s "https://api.nuget.org/v3-flatcontainer/gltf.manipulator/index.json" | jq '.' 2>/dev/nullif [ $? -ne 0 ]; then
echo "Failed to retrieve package. Trying alternative approach..."Try the official NuGet search API
curl -s "https://azuresearch-usnc.nuget.org/query?q=gltf.manipulator&skip=0&take=20" | jq '.data[] | {id, version: .version}' 2>/dev/null
fi
</function_calls>Now let me examine the csproj file to see the full context of the changes:
<function_calls>
#!/bin/bashRead the csproj file to see context around line 78
cat -n "Revit_glTF_Exporter_2025/Revit_glTF_Exporter_2025.csproj" | sed -n '70,100p'
</function_calls><output_format>
Common_glTF_Exporter/EportUtils/ElementValidations.cs (1)
1-3: Node type migration inShouldSkipElementis behaviorally neutral; verify call sitesSwitching
IndexedDictionary<GLTFNode>toIndexedDictionary<Node>while keeping the logic tonodes.Contains(currentElement.UniqueId)preserves behavior and aligns this utility with the new glTF.Manipulator-basedNodemodel.Please double-check all callers of
ShouldSkipElementwere updated to pass anIndexedDictionary<Node>from the new pipeline to avoid type mismatches at compile time.Also applies to: 9-15, 21-39
Common_glTF_Exporter/Core/ProcessGeometry.cs (1)
97-101: Mutating input data is a side effect that may cause issues.Clearing the geometry data modifies the input
dataObjectentries. This could cause issues if the caller expects the data to remain intact. Consider documenting this behavior or making a defensive copy earlier.Verify that callers of
ProcessNodedo not rely on the geometry data after this method returns.Common_glTF_Exporter/Materials/MaterialProperties.cs (2)
18-24: LGTM - Clean refactor to useBaseMaterial.The migration from GLTF-specific types to
BaseMaterialis well-implemented. The property assignments are clear and follow glTF PBR conventions.
26-51: LGTM - Clean implementation of color computation.The refactored
SetMaterialColournow properly returns aList<float>instead of mutating GLTF material properties directly, making it more reusable. The null handling forbaseColorandtintColoris appropriate.Common_glTF_Exporter/Utils/MaterialUtils.cs (1)
14-26: LGTM!The method correctly retrieves the Revit material from a mesh element. The null handling is appropriate.
Common_glTF_Exporter/Export/RevitGrids.cs (2)
1-6: LGTM - Clean imports for the type migration.The imports are well-organized with the new
glTF.Manipulator.Schematypes andSystem.Globalizationfor proper culture-invariant formatting.
22-72: LGTM - Well-structured migration to Node and Extras types.The migration correctly:
- Uses
Nodeinstead ofGLTFNode- Stores grid metadata in a
Dictionary<string, string>viaExtras.parameters- Applies
CultureInfo.InvariantCulturefor consistent coordinate formattingCommon_glTF_Exporter/Materials/MaterialTextures.cs (2)
1-13: LGTM - Imports aligned with library migration.The imports correctly bring in
glTF.Manipulator.GenericSchemaandglTF.Manipulator.Schemafor the new texture/image types.
20-62: LGTM - Clean refactor of texture processing.The method correctly:
- Returns color tuple for downstream processing
- Delegates image caching to
createOrGetBaseImage- Sets
material.hasTextureandmaterial.textureIndexappropriatelyThe removal of
File.Existscheck on line 50 is safe sinceAssetPropertiesUtils.GetTexturePath(per relevant snippets) already validates file existence and returnsnullif not found.Common_glTF_Exporter/Export/GltfJson.cs (3)
1-13: LGTM - Clean migration to System.Text.Json.The imports correctly bring in
System.Text.Jsonand related types, replacing Newtonsoft.Json serialization.
31-84: LGTM - Correct System.Text.Json configuration.The serializer options are well-configured:
PropertyNamingPolicy = nullpreserves property casing (important for glTF spec compliance)DefaultIgnoreCondition.WhenWritingNullomits null properties for cleaner outputWriteIndented = falseminimizes file size for production
86-101: LGTM - Proper sanitization for glTF output.The method correctly strips internal properties (
uuid,imageData) that shouldn't appear in the final glTF JSON, keeping only spec-compliantbufferViewandmimeType.Common_glTF_Exporter/Utils/glTFExportUtils.cs (3)
1-14: LGTM - Imports and namespace updated for library migration.The imports correctly bring in the new
glTF.Manipulatortypes with appropriate aliasing forBuffer.
54-67: Key format change from UniqueId to Id.The vertex key construction changed from
element.UniqueId + material.UniqueIdtoelement.Id.ToString() + "_" + material.uuid. This is a valid simplification, but note:
UniqueIdis a GUID (globally unique), whileIdis an integer (unique within a document)- The underscore separator improves key parsing if needed
This should be fine for single-document exports but verify there are no cross-document scenarios.
69-147: LGTM - Normal calculation methods unchanged.The
AddRPCNormalsandAddNormalsmethods correctly handle polymesh normal distribution cases (AtEachPoint, OnePerFace, OnEachFacet).Common_glTF_Exporter/Export/FileExport.cs (2)
1-13: LGTM - Imports and type aliases correctly configured.The
BufferandMaterialaliases prevent naming conflicts with the library types. Imports are well-organized.
21-56: LGTM - Clean migration with consistent export paths.The method correctly handles both formats:
- gltf: Creates separate
.binand.gltffiles with proper UTF-8 encoding (no BOM)- glb: Uses combined binary format
The change from
List<GLTFBinaryData>to singleGLTFBinaryDatasimplifies the API while maintaining functionality.Common_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs (4)
13-63: LGTM!The index buffer export logic is correct. The method properly handles the empty case, creates appropriate BufferView and Accessor, and returns the accessor index.
69-128: LGTM!The vertex export logic correctly converts vertices to float array, computes min/max bounds, and creates the appropriate BufferView and Accessor.
253-314: LGTM!UV export correctly handles the 2-component texture coordinates, creates appropriate BufferView and Accessor with VEC2 type, and assigns to the primitive's TEXCOORD_0 attribute.
316-344: LGTM!Image buffer export correctly handles the image embedding workflow: appending raw bytes, creating BufferView without a target (appropriate for non-vertex/index data), and updating the image's bufferView reference.
| { | ||
| currentMaterial = MaterialUtils.GetGltfMeshMaterial(currentDocument, preferences, mesh, materials, true); | ||
| materials.AddOrUpdateCurrentMaterial(currentMaterial.UniqueId, currentMaterial, true); | ||
| materials.AddOrUpdateCurrentMaterial(currentMaterial.name, currentMaterial, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent key used for material lookup.
Here currentMaterial.name is used as the key for AddOrUpdateCurrentMaterial, but in other locations (e.g., line 116, line 50 in RevitMaterials.cs), currentMaterial.uuid is used. This inconsistency could cause:
- Duplicate materials if
name != uuid - Lookup failures when retrieving the material by
uuid
- materials.AddOrUpdateCurrentMaterial(currentMaterial.name, currentMaterial, true);
+ materials.AddOrUpdateCurrentMaterial(currentMaterial.uuid, currentMaterial, true);🤖 Prompt for AI Agents
In Common_glTF_Exporter/Core/GlTFExportContext.cs around line 385, the call uses
currentMaterial.name as the key for AddOrUpdateCurrentMaterial which is
inconsistent with other places that use currentMaterial.uuid; change the key to
currentMaterial.uuid (or a null/empty-safe fallback like uuid ?? name if uuid
can be missing) so materials are indexed consistently, and ensure any call sites
and lookups expect uuid keys.
| if (elem is glTF.Manipulator.Schema.Material) | ||
| { | ||
| var mat = this.GetElement(uuid) as GLTFMaterial; | ||
| var mat = this.GetElement(uuid) as glTF.Manipulator.Schema.Material; | ||
| mat.doubleSided = doubleSided; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Script executed:
#!/bin/bash
# Find BaseMaterial class definition
ast-grep --pattern 'class BaseMaterial $$$'Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
Revit_glTF_Exporter_2026/Program.cs:9: internal class Program
Revit_glTF_Exporter_2025/Program.cs:9: internal class Program
GltfInstaller/Dialogs/WelcomeDialog.xaml.cs:23: public partial class WelcomeDialog : WpfDialog, IWpfDialog, IDialog
GltfInstaller/Dialogs/WelcomeDialog.xaml.cs:106: internal class WelcomeDialogModel : Caliburn.Micro.Screen
GltfInstaller/Dialogs/SetupTypeDialog.xaml.cs:18: public partial class SetupTypeDialog : WpfDialog, IWpfDialog
GltfInstaller/Dialogs/SetupTypeDialog.xaml.cs:44: internal class SetupTypeDialogModel : Caliburn.Micro.Screen
GltfInstaller/Dialogs/Resources.xaml.cs:8: public partial class Resources : ResourceDictionary
Revit_glTF_Exporter_2019/Properties/Resources.Designer.cs:25: internal class Resources {
Revit_glTF_Exporter_2023/Properties/Resources.Designer.cs:25: internal class Resources {
Revit_glTF_Exporter_2022/Properties/Resources.Designer.cs:25: internal class Resources {
Revit_glTF_Exporter_2021/Properties/Resources.Designer.cs:25: internal class Resources {
Revit_glTF_Exporter_2020/Properties/Resources.Designer.cs:25: internal class Resources {
GltfInstaller/Dialogs/ProgressDialog.xaml.cs:23: public partial class ProgressDialog : WpfDialog, IWpfDialog, IProgressDialog
GltfInstaller/Dialogs/ProgressDialog.xaml.cs:170: internal class ProgressDialogModel : Caliburn.Micro.Screen
Common_glTF_Exporter/Service/UIApplicationExtension.cs:10: internal static class UIApplicationExtension
Common_glTF_Exporter/Service/RevitCollectorService.cs:10: public class RevitCollectorService
GltfInstaller/Program.cs:17: internal class Program
GltfInstaller/Program.cs:161: public class CustomActions
Common_glTF_Exporter/Export/RevitGrids.cs:13: public static class RevitGrids
GltfInstaller/Dialogs/MaintenanceTypeDialog.xaml.cs:20: public partial class MaintenanceTypeDialog : WpfDialog, IWpfDialog
GltfInstaller/Dialogs/MaintenanceTypeDialog.xaml.cs:103: internal class MaintenanceTypeDialogModel : Caliburn.Micro.Screen
Common_glTF_Exporter/Model/VertexLookupIntObject.cs:10: public class VertexLookupIntObject : Dictionary<PointIntObject, int>
Common_glTF_Exporter/Model/VertexLookupIntObject.cs:45: public class PointIntEqualityComparer : IEqualityComparer<PointIntObject>
GltfInstaller/Dialogs/ExitDialog.xaml.cs:23: public partial class ExitDialog : WpfDialog, IWpfDialog
GltfInstaller/Dialogs/ExitDialog.xaml.cs:142: internal class ExitDialogModel : Caliburn.Micro.Screen
GltfInstaller/Dialogs/LicenseAgreementDialog.xaml.cs:35: public partial class LicenceDialog : WpfDialog, IWpfDialog
GltfInstaller/Dialogs/LicenseAgreementDialog.xaml.cs:123: internal class LicenseDialogModel : Caliburn.Micro.Screen
GltfInstaller/Dialogs/FeaturesDialog.xaml.cs:23: public partial class FeaturesDialog : WpfDialog, IWpfDialog
GltfInstaller/Dialogs/FeaturesDialog.xaml.cs:47: internal class Node : PropertyChangedBase
GltfInstaller/Dialogs/FeaturesDialog.xaml.cs:65: internal class FeaturesDialogModel : Caliburn.Micro.Screen
GltfInstaller/Properties/Resources.Designer.cs:25: internal class Resources {
Common_glTF_Exporter/Export/MeshOpt.cs:11: public static class MeshOpt
Common_glTF_Exporter/Export/GlbHeaderInfo.cs:9: public static class GlbHeaderInfo
GltfInstaller/Dialogs/InstallDirDialog.xaml.cs:18: public partial class InstallDirDialog : WpfDialog, IWpfDialog
GltfInstaller/Dialogs/InstallDirDialog.xaml.cs:43: internal class InstallDirDialogModel : Caliburn.Micro.Screen
Common_glTF_Exporter/EportUtils/ElementValidations.cs:19: public static class ElementValidations
Common_glTF_Exporter/Export/GlbJsonInfo.cs:9: public static class GlbJsonInfo
Common_glTF_Exporter/Export/Compression.cs:8: public class Compression
Common_glTF_Exporter/Model/UnitObject.cs:5: public class UnitObject
Common_glTF_Exporter/Export/BinFile.cs:9:public static class BinFile
Common_glTF_Exporter/ExternalApplication.cs:20: public class ExternalApplication : IExternalApplication
Common_glTF_Exporter/Export/GltfJson.cs:16: public static class GltfJson
Common_glTF_Exporter/Model/Room.cs:6: public class Room
Common_glTF_Exporter/Export/BufferConfig.cs:10: public static class BufferConfig
Common_glTF_Exporter/Model/RevitGridParametersObject.cs:5: public class RevitGridParametersObject
Common_glTF_Exporter/Export/ZIP.cs:8: public static class ZIP
Common_glTF_Exporter/Export/GlbFile.cs:13: internal class GlbFile
Common_glTF_Exporter/Core/ProcessGeometry.cs:17: public static class ProcessGeometry
Common_glTF_Exporter/Model/PointIntObject.cs:13: public class PointIntObject : IComparable<PointIntObject>
Common_glTF_Exporter/Export/FileExport.cs:16: public static class FileExport
Common_glTF_Exporter/Model/Payload.cs:9: public class Payload
Common_glTF_Exporter/Windows/VersionWindow.xaml.cs:16: public partial class VersionWindow : Window
Common_glTF_Exporter/Export/GlbBinInfo.cs:7: public static class GlbBinInfo
Common_glTF_Exporter/Model/GlbComponents.cs:8: public class GlbHeader
Common_glTF_Exporter/Model/GlbComponents.cs:24: public class GlbJson
Common_glTF_Exporter/Model/GlbComponents.cs:37: public class GlbBin
Common_glTF_Exporter/Model/MovableObjects.cs:7: public class MovableObjects
Common_glTF_Exporter/Model/FixedObject.cs:5: public class FixedObject : IObject
Common_glTF_Exporter/Model/MovableObject.cs:5: public class MovableObject : IObject
Common_glTF_Exporter/ExternalCommand.cs:12: public class ExternalCommand : IExternalCommand
Common_glTF_Exporter/ExternalCommand.cs:52: public class AboutUs : IExternalCommand
Common_glTF_Exporter/Model/FixedObjects.cs:7: public class FixedObjects
Common_glTF_Exporter/Model/LoadContext.cs:6: public class NonCollectibleAssemblyLoadContext : AssemblyLoadContext
Common_glTF_Exporter/Model/MaterialCacheDTO.cs:7: public class MaterialCacheDTO
Common_glTF_Exporter/Model/Links.cs:9: public class Links
Common_glTF_Exporter/Core/IndexedDictionary.cs:12: public class IndexedDictionary<T>
Common_glTF_Exporter/Windows/ProgressBarWindow.xaml.cs:13: public partial class ProgressBarWindow : Window
Common_glTF_Exporter/ViewModel/ProgressBarWindowViewModel.cs:8: public class ProgressBarWindowViewModel : INotifyPropertyChanged
Common_glTF_Exporter/Materials/RevitMaterials.cs:21: public static class RevitMaterials
Common_glTF_Exporter/ViewModel/ViewModelBase.cs:8: public class ViewModelBase : INotifyPropertyChanged
Common_glTF_Exporter/ViewModel/UnitsViewModel.cs:7: public class UnitsViewModel : ViewModelBase
Common_glTF_Exporter/Materials/BitmapsUtils.cs:12: public static class BitmapsUtils
Common_glTF_Exporter/Materials/TextureLocation.cs:8: public static class TextureLocation
Common_glTF_Exporter/Version/LatestVersion.cs:11: public static class LatestVersion
Common_glTF_Exporter/Windows/ErrorWindow.xaml.cs:11: public partial class ErrorWindow : Window
Common_glTF_Exporter/Windows/MainWindow/UpdateForm.cs:16: public static class UpdateForm
Common_glTF_Exporter/Version/VersionValidate.cs:9: public static class VersionValidation
Common_glTF_Exporter/Materials/MaterialTextures.cs:17: public static class MaterialTextures
Common_glTF_Exporter/Version/RunLocalFile.cs:6: public static class RunLocalFile
Common_glTF_Exporter/Materials/MaterialTexture.cs:3: public class MaterialTexture
Common_glTF_Exporter/Windows/MainWindow.xaml.cs:27: public partial class MainWindow : Window
Common_glTF_Exporter/Version/InternetConnection.cs:8: public static class InternetConnection
Common_glTF_Exporter/Windows/Resources.xaml.cs:10: public partial class Resources : ResourceDictionary
Common_glTF_Exporter/Windows/ErrorReportWindow.xaml.cs:14: public partial class ErrorReportWindow : Window
Common_glTF_Exporter/Materials/AssetProperties.cs:10: public static class AssetPropertiesUtils
Common_glTF_Exporter/Windows/AboutUsWindow.xaml.cs:13: public partial class AboutUsWindow : Window
Common_glTF_Exporter/Windows/MessageWindow.xaml.cs:11: public partial class MessageWindow : Window
Common_glTF_Exporter/Version/DownloadFile.cs:13: public static class DownloadFile
Common_glTF_Exporter/Transform/ModelRotation.cs:7: internal class ModelRotation
Common_glTF_Exporter/Transform/ModelTraslation.cs:8: public static class ModelTraslation
Common_glTF_Exporter/Materials/MaterialProperties.cs:14: public static class MaterialProperties
Common_glTF_Exporter/Transform/ModelScale.cs:10: public static class ModelScale
Common_glTF_Exporter/Utils/Hyperlink.cs:8: public static class Hyperlink
Common_glTF_Exporter/Windows/FeedbackWindow.xaml.cs:13: public partial class FeedbackWindow : Window
Common_glTF_Exporter/Utils/DirectoryUtils.cs:5: public static class DirectoryUtils
Common_glTF_Exporter/Utils/GeometryUtils.cs:7: public class GeometryUtils
Common_glTF_Exporter/Utils/Theme.cs:11: public static class Theme
Common_glTF_Exporter/Windows/MainWindow/LabelVersion.cs:15: public class LabelVersion
Common_glTF_Exporter/Utils/FilesHelper.cs:8: internal class FilesHelper
Common_glTF_Exporter/Utils/glTFExportUtils.cs:14: public class GLTFExportUtils
Common_glTF_Exporter/Utils/Collectors.cs:7: internal class Collectors
Common_glTF_Exporter/Windows/MainWindow/ComboUnits.cs:11: public static class ComboUnits
Common_glTF_Exporter/Core/GlTFExportContext.cs:20: public class GLTFExportContext : IExportContext
Common_glTF_Exporter/Utils/ExportLog.cs:7: public class ExportLog
Common_glTF_Exporter/Utils/MaterialUtils.cs:12: public class MaterialUtils
Common_glTF_Exporter/Windows/MainWindow/Preferences.cs:26: public class Preferences
Common_glTF_Exporter/Windows/MainWindow/Settings.cs:8: public static class Settings
Common_glTF_Exporter/Utils/ClickUpFormSender.cs:8:public static class ClickUpFormSender
Common_glTF_Exporter/Utils/RevitIniReader.cs:11: public static class RevitIniReader
Common_glTF_Exporter/Export/Draco.cs:13: internal sealed class GlbData
Common_glTF_Exporter/Export/Draco.cs:25: internal static class GltfExtrasPatcher
Common_glTF_Exporter/Export/Draco.cs:503: public static class Draco
Common_glTF_Exporter/Utils/LogConfiguration.cs:7: public static class LogConfiguration
Common_glTF_Exporter/Utils/SettingsConfig.cs:10: public static class SettingsConfig
Common_glTF_Exporter/Utils/Analytics.cs:10: public class Analytics
Common_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs:11: public class GLTFBinaryDataUtils
Common_glTF_Exporter/Utils/Util.cs:10: public class Util
</shell_result>
Script executed:
#!/bin/bash
# Search for BaseMaterial class definition
rg -n "class BaseMaterial" --type cs
# Also search for Material class
rg -n "class Material" --type cs | head -20
# Search for materials field in GlTFExportContext
rg -n "materials" Common_glTF_Exporter/Core/GlTFExportContext.cs | head -20Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
Common_glTF_Exporter/Utils/MaterialUtils.cs:12: public class MaterialUtils
Common_glTF_Exporter/Model/MaterialCacheDTO.cs:7: public class MaterialCacheDTO
Common_glTF_Exporter/Materials/MaterialTextures.cs:17: public static class MaterialTextures
Common_glTF_Exporter/Materials/MaterialProperties.cs:14: public static class MaterialProperties
Common_glTF_Exporter/Materials/MaterialTexture.cs:3: public class MaterialTexture
57: public IndexedDictionary<BaseMaterial> materials = new IndexedDictionary<BaseMaterial>();
116: materials.AddOrUpdateCurrentMaterial(currentMaterial.uuid, currentMaterial, false);
135: if (preferences.materials == MaterialsEnum.textures)
153: scenes, nodes, meshes, materials, accessors, textures, images);
202: /// We do this at OnElementEnd because it signals no more meshes or materials are coming for
215: globalBuffer, bufferViews, accessors, materials, currentGeometry, currentElement);
229: if (preferences.materials == MaterialsEnum.materials || preferences.materials == MaterialsEnum.textures)
233: currentMaterial = GLTFExportUtils.GetGLTFMaterial(materials);
237: currentMaterial = RevitMaterials.ProcessMaterial(node, preferences, currentDocument, materials, textures, images);
246: /// element, materials can be changed and repeated in unknown order).
273: if (preferences.materials == MaterialsEnum.textures && currentMaterial.hasTexture)
382: if (preferences.materials == MaterialsEnum.materials || preferences.materials == MaterialsEnum.textures)
384: currentMaterial = MaterialUtils.GetGltfMeshMaterial(currentDocument, preferences, mesh, materials, true);
385: materials.AddOrUpdateCurrentMaterial(currentMaterial.name, currentMaterial, true);
</shell_result>
Script executed:
#!/bin/bash
# Read IndexedDictionary.cs to see the full context around lines 104-108
cat -n Common_glTF_Exporter/Core/IndexedDictionary.cs | sed -n '95,115p'
# Also check the using statements
cat -n Common_glTF_Exporter/Core/IndexedDictionary.cs | head -15Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
95 {
96 this.List.Add(elem);
97 this.dict.Add(uuid, this.List.Count - 1);
98 this.CurrentKey = uuid;
99 return true;
100 }
101
102 this.CurrentKey = uuid;
103
104 if (elem is glTF.Manipulator.Schema.Material)
105 {
106 var mat = this.GetElement(uuid) as glTF.Manipulator.Schema.Material;
107 mat.doubleSided = doubleSided;
108 }
109
110 return false;
111 }
112
113 /// <summary>
114 /// Check if the container already has an item with this key.
115 /// </summary>
1 namespace Revit_glTF_Exporter
2 {
3 using System;
4 using System.Collections.Generic;
5 using Common_glTF_Exporter.Core;
6
7 /// <summary>
8 /// Container for holding a strict set of items
9 /// that is also addressable by a unique ID.
10 /// </summary>
11 /// <typeparam name="T">The type of item contained.</typeparam>
12 public class IndexedDictionary<T>
13 {
14 private Dictionary<string, int> dict = new Dictionary<string, int>();
15
</shell_result>
Script executed:
#!/bin/bash
# Read more context to see the AddOrUpdateCurrentMaterial method signature
cat -n Common_glTF_Exporter/Core/IndexedDictionary.cs | sed -n '75,110p'
# Search for BaseMaterial in the entire codebase
rg -n "BaseMaterial" --type cs | head -30
# Search for where Material is defined (full qualified name)
rg -n "glTF\.Manipulator\.Schema\.Material" --type cs | head -20Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
75 this.dict.Add(uuid, this.List.Count - 1);
76 this.CurrentKey = uuid;
77 return true;
78 }
79
80 this.CurrentKey = uuid;
81
82 return false;
83 }
84
85 /// <summary>
86 /// Add a new gltfMaterial to the list, if it already exists then the current item will be set to this item.
87 /// </summary>
88 /// <param name="uuid">Unique identifier for the item.</param>
89 /// <param name="elem">The item to add.</param>
90 /// <param name="doubleSided">Identify if the material is double sided.</param>
91 /// <returns>true if item did not already exist.</returns>
92 public bool AddOrUpdateCurrentMaterial(string uuid, T elem, bool doubleSided)
93 {
94 if (!this.dict.ContainsKey(uuid))
95 {
96 this.List.Add(elem);
97 this.dict.Add(uuid, this.List.Count - 1);
98 this.CurrentKey = uuid;
99 return true;
100 }
101
102 this.CurrentKey = uuid;
103
104 if (elem is glTF.Manipulator.Schema.Material)
105 {
106 var mat = this.GetElement(uuid) as glTF.Manipulator.Schema.Material;
107 mat.doubleSided = doubleSided;
108 }
109
110 return false;
Common_glTF_Exporter/Utils/MaterialUtils.cs:28: public static BaseMaterial GetGltfMeshMaterial(Document doc, Preferences preferences, Autodesk.Revit.DB.Mesh mesh, IndexedDictionary<BaseMaterial> materials, bool doubleSided)
Common_glTF_Exporter/Utils/MaterialUtils.cs:30: BaseMaterial gl_mat = new BaseMaterial();
Common_glTF_Exporter/Utils/glTFExportUtils.cs:20: public static BaseMaterial GetGLTFMaterial(IndexedDictionary<BaseMaterial> Materials)
Common_glTF_Exporter/Utils/glTFExportUtils.cs:25: public static BaseMaterial CreateDefaultGLTFMaterial(int materialOpacity, bool doubleSided)
Common_glTF_Exporter/Utils/glTFExportUtils.cs:27: BaseMaterial baseMaterial = new BaseMaterial();
Common_glTF_Exporter/Utils/glTFExportUtils.cs:58: BaseMaterial material)
Common_glTF_Exporter/Materials/MaterialProperties.cs:18: public static void SetProperties(MaterialNode node, float opacity, ref BaseMaterial material)
Common_glTF_Exporter/Materials/MaterialTextures.cs:20: public static (Autodesk.Revit.DB.Color, Autodesk.Revit.DB.Color) SetMaterialTextures(Material revitMaterial, BaseMaterial material,
Common_glTF_Exporter/Materials/MaterialTextures.cs:64: private static Texture createTexture(BaseMaterial material, string texturePath, Asset connectedAsset,
Common_glTF_Exporter/Materials/RevitMaterials.cs:25: public static BaseMaterial ProcessMaterial(MaterialNode node,
Common_glTF_Exporter/Materials/RevitMaterials.cs:26: Preferences preferences, Document doc, IndexedDictionary<BaseMaterial> materials,
Common_glTF_Exporter/Materials/RevitMaterials.cs:29: BaseMaterial material = new BaseMaterial();
Common_glTF_Exporter/Materials/RevitMaterials.cs:59: public static BaseMaterial Export(MaterialNode node,
Common_glTF_Exporter/Materials/RevitMaterials.cs:62: List<glTFImage> images, BaseMaterial material)
Common_glTF_Exporter/Export/GltfJson.cs:22: List<BaseMaterial> materials,
Common_glTF_Exporter/Export/GltfJson.cs:103: public static List<glTF.Manipulator.Schema.Material> transformMaterials(List<BaseMaterial> baseMaterials)
Common_glTF_Exporter/Export/GltfJson.cs:106: foreach (BaseMaterial baseMat in baseMaterials)
Common_glTF_Exporter/Export/FileExport.cs:29: IndexedDictionary<BaseMaterial> materials,
Common_glTF_Exporter/Core/ProcessGeometry.cs:20: Preferences preferences, GLTFBinaryData globalBuffer, List<BufferView> bufferViews, List<Accessor> accessors, IndexedDictionary<BaseMaterial> materials,
Common_glTF_Exporter/Core/GlTFExportContext.cs:35: private BaseMaterial currentMaterial;
Common_glTF_Exporter/Core/GlTFExportContext.cs:57: public IndexedDictionary<BaseMaterial> materials = new IndexedDictionary<BaseMaterial>();
Common_glTF_Exporter/Export/GltfJson.cs:103: public static List<glTF.Manipulator.Schema.Material> transformMaterials(List<BaseMaterial> baseMaterials)
Common_glTF_Exporter/Export/GltfJson.cs:105: List<glTF.Manipulator.Schema.Material> materials = new List<glTF.Manipulator.Schema.Material>();
Common_glTF_Exporter/Export/GltfJson.cs:108: glTF.Manipulator.Schema.Material mat = new glTF.Manipulator.Schema.Material();
Common_glTF_Exporter/Core/IndexedDictionary.cs:104: if (elem is glTF.Manipulator.Schema.Material)
Common_glTF_Exporter/Core/IndexedDictionary.cs:106: var mat = this.GetElement(uuid) as glTF.Manipulator.Schema.Material;
Common_glTF_Exporter/Export/FileExport.cs:12:using Material = glTF.Manipulator.Schema.Material;
</shell_result>
Script executed:
#!/bin/bash
# Find BaseMaterial class definition
rg -n "class BaseMaterial" --type cs -A 5
# Also check if BaseMaterial inherits from Material
rg -n "class BaseMaterial.*:" --type csRepository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 1
</shell_result>
Script executed:
#!/bin/bash
# Find where BaseMaterial is imported or defined
rg -n "using.*BaseMaterial" --type cs
# Check for any imports of glTF.Manipulator
rg -n "using.*glTF" --type cs | head -10
# Check the file that uses BaseMaterial most
rg -n "BaseMaterial" Common_glTF_Exporter/Utils/MaterialUtils.cs -B 10 | head -30Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
Common_glTF_Exporter/ExternalApplication.cs:6: using Common_glTF_Exporter;
Common_glTF_Exporter/ExternalApplication.cs:7: using Common_glTF_Exporter.Service;
Common_glTF_Exporter/ExternalApplication.cs:8: using Common_glTF_Exporter.Version;
Common_glTF_Exporter/Windows/VersionWindow.xaml.cs:2:using Common_glTF_Exporter.Utils;
Common_glTF_Exporter/Windows/VersionWindow.xaml.cs:3:using Common_glTF_Exporter.Version;
Common_glTF_Exporter/Windows/VersionWindow.xaml.cs:9:using Common_glTF_Exporter;
Common_glTF_Exporter/Windows/ProgressBarWindow.xaml.cs:6: using Common_glTF_Exporter.Utils;
Common_glTF_Exporter/Windows/ProgressBarWindow.xaml.cs:7: using Common_glTF_Exporter.ViewModel;
Common_glTF_Exporter/Windows/ProgressBarWindow.xaml.cs:8: using Common_glTF_Exporter.Core;
Common_glTF_Exporter/Windows/MainWindow.xaml.cs:12:using Common_glTF_Exporter;
18- if (materialId != null)
19- {
20- return doc.GetElement(materialId) as Autodesk.Revit.DB.Material;
21- }
22- else
23- {
24- return null;
25- }
26- }
27-
28: public static BaseMaterial GetGltfMeshMaterial(Document doc, Preferences preferences, Autodesk.Revit.DB.Mesh mesh, IndexedDictionary<BaseMaterial> materials, bool doubleSided)
29- {
30: BaseMaterial gl_mat = new BaseMaterial();
</shell_result>
Script executed:
#!/bin/bash
# Read MaterialUtils.cs header to find imports and see if BaseMaterial is defined there
head -30 Common_glTF_Exporter/Utils/MaterialUtils.cs
# Also search for "BaseMaterial" definition more carefully
rg -n "^.*BaseMaterial" --type cs Common_glTF_Exporter/Utils/MaterialUtils.cs | head -5Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
namespace Common_glTF_Exporter.Utils
{
using Autodesk.Revit.DB;
using Common_glTF_Exporter.Core;
using Common_glTF_Exporter.Model;
using Common_glTF_Exporter.Windows.MainWindow;
using glTF.Manipulator.GenericSchema;
using glTF.Manipulator.Schema;
using Revit_glTF_Exporter;
using System.Collections.Generic;
public class MaterialUtils
{
public static Autodesk.Revit.DB.Material GetMeshMaterial(Document doc, Autodesk.Revit.DB.Mesh mesh)
{
ElementId materialId = mesh.MaterialElementId;
if (materialId != null)
{
return doc.GetElement(materialId) as Autodesk.Revit.DB.Material;
}
else
{
return null;
}
}
public static BaseMaterial GetGltfMeshMaterial(Document doc, Preferences preferences, Autodesk.Revit.DB.Mesh mesh, IndexedDictionary<BaseMaterial> materials, bool doubleSided)
{
BaseMaterial gl_mat = new BaseMaterial();
28: public static BaseMaterial GetGltfMeshMaterial(Document doc, Preferences preferences, Autodesk.Revit.DB.Mesh mesh, IndexedDictionary<BaseMaterial> materials, bool doubleSided)
30: BaseMaterial gl_mat = new BaseMaterial();
</shell_result>
Script executed:
#!/bin/bash
# Find BaseMaterial definition
fd -e cs | xargs grep -l "class BaseMaterial" 2>/dev/null
# Or search in Common_glTF_Exporter.Model namespace
find . -path "*/Common_glTF_Exporter/Model*" -name "*.cs" -exec grep -l "BaseMaterial" {} \;
# Search entire repo for "class.*BaseMaterial"
find . -name "*.cs" -exec grep -n "class.*BaseMaterial" {} + 2>/dev/nullRepository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 1
</shell_result>
Script executed:
#!/bin/bash
# Search for any reference to glTF.Manipulator in imports
rg -n "glTF\.Manipulator" --type cs | grep using | head -20
# Search all files for any typedef or alias of BaseMaterial
rg -n "BaseMaterial\s*=" --type cs
# Check if there's a class definition anywhere with inheritance
rg -n "BaseMaterial.*{" --type cs | head -10Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
Common_glTF_Exporter/Core/GlTFExportContext.cs:8:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Core/GlTFExportContext.cs:9:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Core/GlTFExportContext.cs:13:using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Core/ProcessGeometry.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Core/ProcessGeometry.cs:6:using glTF.Manipulator.Schema;
Common_glTF_Exporter/EportUtils/ElementValidations.cs:9:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/BufferConfig.cs:5:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/BufferConfig.cs:6:using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Export/GlbFile.cs:9:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GlbBinInfo.cs:3:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GltfJson.cs:6:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GltfJson.cs:7:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/GltfJson.cs:12:using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Export/RevitGrids.cs:3:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/FileExport.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/FileExport.cs:6:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/FileExport.cs:11:using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Export/FileExport.cs:12:using Material = glTF.Manipulator.Schema.Material;
Common_glTF_Exporter/Export/BinFile.cs:7:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Materials/RevitMaterials.cs:15:using glTF.Manipulator.Schema;
stderr:
regex parse error:
BaseMaterial.*{
^
error: unclosed counted repetition
</shell_result>
Script executed:
#!/bin/bash
# Look at how BaseMaterial is used in context to understand where it comes from
rg -B 5 "new BaseMaterial()" --type cs | head -30
# Check if there's a namespace alias for BaseMaterial
rg -n "using.*GenericSchema" --type cs
# Look at the glTF.Manipulator package more carefully
rg -n "GenericSchema\." --type cs | head -10Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
Common_glTF_Exporter/Utils/glTFExportUtils.cs- return Materials.GetElement(DEF_UNIQUEL_ID);
Common_glTF_Exporter/Utils/glTFExportUtils.cs- }
Common_glTF_Exporter/Utils/glTFExportUtils.cs-
Common_glTF_Exporter/Utils/glTFExportUtils.cs- public static BaseMaterial CreateDefaultGLTFMaterial(int materialOpacity, bool doubleSided)
Common_glTF_Exporter/Utils/glTFExportUtils.cs- {
Common_glTF_Exporter/Utils/glTFExportUtils.cs: BaseMaterial baseMaterial = new BaseMaterial();
--
Common_glTF_Exporter/Utils/MaterialUtils.cs- }
Common_glTF_Exporter/Utils/MaterialUtils.cs- }
Common_glTF_Exporter/Utils/MaterialUtils.cs-
Common_glTF_Exporter/Utils/MaterialUtils.cs- public static BaseMaterial GetGltfMeshMaterial(Document doc, Preferences preferences, Autodesk.Revit.DB.Mesh mesh, IndexedDictionary<BaseMaterial> materials, bool doubleSided)
Common_glTF_Exporter/Utils/MaterialUtils.cs- {
Common_glTF_Exporter/Utils/MaterialUtils.cs: BaseMaterial gl_mat = new BaseMaterial();
--
Common_glTF_Exporter/Materials/RevitMaterials.cs-
Common_glTF_Exporter/Materials/RevitMaterials.cs- public static BaseMaterial ProcessMaterial(MaterialNode node,
Common_glTF_Exporter/Materials/RevitMaterials.cs- Preferences preferences, Document doc, IndexedDictionary<BaseMaterial> materials,
Common_glTF_Exporter/Materials/RevitMaterials.cs- List<Texture> textures, List<glTFImage> images)
Common_glTF_Exporter/Materials/RevitMaterials.cs- {
Common_glTF_Exporter/Materials/RevitMaterials.cs: BaseMaterial material = new BaseMaterial();
Common_glTF_Exporter/Utils/glTFExportUtils.cs:10: using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Utils/MaterialUtils.cs:7: using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs:3:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GltfJson.cs:6:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GlbFile.cs:9:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Materials/RevitMaterials.cs:16:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Materials/MaterialTextures.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Materials/MaterialProperties.cs:9:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GlbBinInfo.cs:3:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/FileExport.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/BinFile.cs:7:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Core/ProcessGeometry.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Core/GlTFExportContext.cs:8:using glTF.Manipulator.GenericSchema;
</shell_result>
Perfect! I've verified the issue. Let me now generate the final rewritten review comment:
Type check will never execute when IndexedDictionary<BaseMaterial> is used.
The type check elem is glTF.Manipulator.Schema.Material will always fail because BaseMaterial (from glTF.Manipulator.GenericSchema) and glTF.Manipulator.Schema.Material are different types from different namespaces. Since materials is instantiated as IndexedDictionary<BaseMaterial> in GlTFExportContext.cs, the conditional block at lines 104-108 is unreachable dead code.
🤖 Prompt for AI Agents
In Common_glTF_Exporter/Core/IndexedDictionary.cs around lines 104-108, the
runtime type check uses glTF.Manipulator.Schema.Material but the dictionary is
instantiated as IndexedDictionary<BaseMaterial>, so that check is unreachable;
fix by either (A) change the dictionary's element type where it's instantiated
(e.g., in GlTFExportContext.cs) to glTF.Manipulator.Schema.Material so the
existing check is valid, or (B) change this conditional to check for the actual
BaseMaterial type (glTF.Manipulator.GenericSchema.BaseMaterial) and then obtain
or map to the underlying Schema.Material instance before setting doubleSided
(e.g., cast to BaseMaterial and use its schema/material accessor), ensuring the
types match at compile/runtime.
| int indexAcessor = GLTFBinaryDataUtils.ExportFaces(kvp.Value, gLTFBinaryDataElement, bufferViews, accessors); | ||
|
|
||
| primitive.indices = indexAcessor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle failure case when ExportFaces returns -1.
ExportFaces returns -1 when there are no faces to export. Setting primitive.indices = -1 will produce invalid glTF output as accessor indices must be non-negative.
int indexAcessor = GLTFBinaryDataUtils.ExportFaces(kvp.Value, gLTFBinaryDataElement, bufferViews, accessors);
- primitive.indices = indexAcessor;
+ if (indexAcessor < 0)
+ {
+ // Skip this primitive if no faces exist
+ continue;
+ }
+
+ primitive.indices = indexAcessor;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| int indexAcessor = GLTFBinaryDataUtils.ExportFaces(kvp.Value, gLTFBinaryDataElement, bufferViews, accessors); | |
| primitive.indices = indexAcessor; | |
| int indexAcessor = GLTFBinaryDataUtils.ExportFaces(kvp.Value, gLTFBinaryDataElement, bufferViews, accessors); | |
| if (indexAcessor < 0) | |
| { | |
| // Skip this primitive if no faces exist | |
| continue; | |
| } | |
| primitive.indices = indexAcessor; |
🤖 Prompt for AI Agents
In Common_glTF_Exporter/Core/ProcessGeometry.cs around lines 85–87, ExportFaces
can return -1 when there are no faces; currently the code unconditionally sets
primitive.indices = -1 which produces invalid glTF. Change the logic to check
the return value and only assign primitive.indices when indexAcessor >= 0; if it
is -1, do not assign the indices field (or set it to null/leave it unset per the
glTF model) and optionally emit a debug/warning log indicating no face accessor
was created.
| private static GlbData ReadGlb(string path) | ||
| { | ||
| using (FileStream fs = File.OpenRead(path)) | ||
| using (BinaryReader br = new BinaryReader(fs)) | ||
| { | ||
| uint magic = br.ReadUInt32(); // 'glTF' | ||
| uint version = br.ReadUInt32(); // 2 | ||
| uint length = br.ReadUInt32(); | ||
|
|
||
| // chunk 0: JSON | ||
| uint chunkLen0 = br.ReadUInt32(); | ||
| uint chunkType0 = br.ReadUInt32(); // 'JSON' | ||
| byte[] jsonBytes = br.ReadBytes((int)chunkLen0); | ||
| string json = System.Text.Encoding.UTF8.GetString(jsonBytes); | ||
|
|
||
| // chunk 1 (opcional): BIN | ||
| byte[] bin = new byte[0]; | ||
| if (fs.Position + 8 <= fs.Length) | ||
| { | ||
| uint chunkLen1 = br.ReadUInt32(); | ||
| uint chunkType1 = br.ReadUInt32(); // 'BIN\0' | ||
| bin = br.ReadBytes((int)chunkLen1); | ||
| } | ||
|
|
||
| return new GlbData(json, bin); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing GLB header validation.
The method reads magic, version, and length values but doesn't validate them. Processing a non-GLB file or corrupted file could lead to unexpected behavior.
uint magic = br.ReadUInt32(); // 'glTF'
uint version = br.ReadUInt32(); // 2
uint length = br.ReadUInt32();
+if (magic != 0x46546C67)
+ throw new InvalidDataException($"Invalid GLB magic number: 0x{magic:X8}");
+if (version != 2)
+ throw new InvalidDataException($"Unsupported GLB version: {version}");
+
// chunk 0: JSON📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static GlbData ReadGlb(string path) | |
| { | |
| using (FileStream fs = File.OpenRead(path)) | |
| using (BinaryReader br = new BinaryReader(fs)) | |
| { | |
| uint magic = br.ReadUInt32(); // 'glTF' | |
| uint version = br.ReadUInt32(); // 2 | |
| uint length = br.ReadUInt32(); | |
| // chunk 0: JSON | |
| uint chunkLen0 = br.ReadUInt32(); | |
| uint chunkType0 = br.ReadUInt32(); // 'JSON' | |
| byte[] jsonBytes = br.ReadBytes((int)chunkLen0); | |
| string json = System.Text.Encoding.UTF8.GetString(jsonBytes); | |
| // chunk 1 (opcional): BIN | |
| byte[] bin = new byte[0]; | |
| if (fs.Position + 8 <= fs.Length) | |
| { | |
| uint chunkLen1 = br.ReadUInt32(); | |
| uint chunkType1 = br.ReadUInt32(); // 'BIN\0' | |
| bin = br.ReadBytes((int)chunkLen1); | |
| } | |
| return new GlbData(json, bin); | |
| } | |
| } | |
| private static GlbData ReadGlb(string path) | |
| { | |
| using (FileStream fs = File.OpenRead(path)) | |
| using (BinaryReader br = new BinaryReader(fs)) | |
| { | |
| uint magic = br.ReadUInt32(); // 'glTF' | |
| uint version = br.ReadUInt32(); // 2 | |
| uint length = br.ReadUInt32(); | |
| if (magic != 0x46546C67) | |
| throw new InvalidDataException($"Invalid GLB magic number: 0x{magic:X8}"); | |
| if (version != 2) | |
| throw new InvalidDataException($"Unsupported GLB version: {version}"); | |
| // chunk 0: JSON | |
| uint chunkLen0 = br.ReadUInt32(); | |
| uint chunkType0 = br.ReadUInt32(); // 'JSON' | |
| byte[] jsonBytes = br.ReadBytes((int)chunkLen0); | |
| string json = System.Text.Encoding.UTF8.GetString(jsonBytes); | |
| // chunk 1 (opcional): BIN | |
| byte[] bin = new byte[0]; | |
| if (fs.Position + 8 <= fs.Length) | |
| { | |
| uint chunkLen1 = br.ReadUInt32(); | |
| uint chunkType1 = br.ReadUInt32(); // 'BIN\0' | |
| bin = br.ReadBytes((int)chunkLen1); | |
| } | |
| return new GlbData(json, bin); | |
| } | |
| } |
| private static void File_MoveOverwrite(string src, string dst) | ||
| { | ||
| if (File.Exists(dst)) | ||
| { | ||
| try { File.Delete(dst); } | ||
| catch { /* ignore */ } | ||
| } | ||
| File.Move(src, dst); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File_MoveOverwrite can fail if delete is ignored but file still exists.
If File.Delete(dst) throws and is caught, the subsequent File.Move(src, dst) will fail because the destination still exists. This defeats the purpose of the overwrite helper.
private static void File_MoveOverwrite(string src, string dst)
{
if (File.Exists(dst))
{
- try { File.Delete(dst); }
- catch { /* ignore */ }
+ File.Delete(dst);
}
File.Move(src, dst);
}Alternatively, on .NET Core 3.0+, use File.Move(src, dst, overwrite: true).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static void File_MoveOverwrite(string src, string dst) | |
| { | |
| if (File.Exists(dst)) | |
| { | |
| try { File.Delete(dst); } | |
| catch { /* ignore */ } | |
| } | |
| File.Move(src, dst); | |
| } | |
| private static void File_MoveOverwrite(string src, string dst) | |
| { | |
| if (File.Exists(dst)) | |
| { | |
| File.Delete(dst); | |
| } | |
| File.Move(src, dst); | |
| } |
🤖 Prompt for AI Agents
In Common_glTF_Exporter/Export/Draco.cs around lines 601 to 609, the current
File_MoveOverwrite swallows exceptions from File.Delete then calls File.Move
which will fail if the destination still exists; change the method to either
call File.Move(src, dst, overwrite: true) when targeting .NET Core 3.0+ or, if
that overload is not available, remove the empty catch and let the delete
exception bubble (or detect File.Exists(dst) after the catch and throw a clear
IOException) so you don't attempt File.Move into an existing file; ensure you
only catch and handle expected exceptions and preserve/propagate errors instead
of ignoring them.
| double percentageProgress = (value / ProgressBarMax) * 100; | ||
| if (percentageProgress > 99) | ||
| { | ||
| ProgressBarPercentage = 100; | ||
| } | ||
| else | ||
| { | ||
| ProgressBarPercentage = (value / ProgressBarMax) * 100; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add division-by-zero protection.
The calculation (value / ProgressBarMax) * 100 will throw a DivideByZeroException if ProgressBarMax is zero. This could occur during initialization or if the maximum value is incorrectly set to zero.
Apply this diff to add a guard clause:
+ if (ProgressBarMax == 0)
+ {
+ ProgressBarPercentage = 0;
+ this.OnPropertyChanged();
+ return;
+ }
+
double percentageProgress = (value / ProgressBarMax) * 100;
if (percentageProgress > 99)
{
ProgressBarPercentage = 100;
}
else
{
ProgressBarPercentage = percentageProgress;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| double percentageProgress = (value / ProgressBarMax) * 100; | |
| if (percentageProgress > 99) | |
| { | |
| ProgressBarPercentage = 100; | |
| } | |
| else | |
| { | |
| ProgressBarPercentage = (value / ProgressBarMax) * 100; | |
| } | |
| if (ProgressBarMax == 0) | |
| { | |
| ProgressBarPercentage = 0; | |
| this.OnPropertyChanged(); | |
| return; | |
| } | |
| double percentageProgress = (value / ProgressBarMax) * 100; | |
| if (percentageProgress > 99) | |
| { | |
| ProgressBarPercentage = 100; | |
| } | |
| else | |
| { | |
| ProgressBarPercentage = percentageProgress; | |
| } |
🤖 Prompt for AI Agents
In Common_glTF_Exporter/ViewModel/ProgressBarWindowViewModel.cs around lines 64
to 72, the code computes (value / ProgressBarMax) * 100 and will throw a
DivideByZeroException if ProgressBarMax is zero; add a guard that checks if
ProgressBarMax is zero (or effectively zero) before dividing and set
ProgressBarPercentage to 0 (or to 100 only when value >= ProgressBarMax and
ProgressBarMax > 0), otherwise compute percentage once and reuse it for the >99
check and assignment to avoid repeated division.
| <PackageReference Include="glTF.Manipulator"> | ||
| <Version>2025.11.24.2122</Version> | ||
| </PackageReference> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Script executed:
#!/bin/bash
# Verify glTF.Manipulator usage and ensure complete migration from Newtonsoft.Json
# 1. Find all .csproj files in the repository
echo "=== All .csproj files in the repository ==="
fd -e csproj
# 2. Search for remaining Newtonsoft.Json references in all .csproj files
echo -e "\n=== Checking for remaining Newtonsoft.Json package references in .csproj files ==="
fd -e csproj -x rg -n "Newtonsoft\.Json" {}
# 3. Search for glTF.Manipulator references in all .csproj files
echo -e "\n=== Checking for glTF.Manipulator package reference in .csproj files ==="
fd -e csproj -x rg -n "glTF\.Manipulator" {}
# 4. Search for remaining using statements for Newtonsoft.Json in C# files
echo -e "\n=== Checking for remaining Newtonsoft.Json using statements in C# files ==="
rg -n "using Newtonsoft" --type=cs
# 5. Search for Newtonsoft.Json API usage (JsonConvert, JObject, JArray, etc.)
echo -e "\n=== Checking for Newtonsoft.Json API usage (JsonConvert, JObject, JArray, etc.) ==="
rg -n "JsonConvert|JObject|JArray|JsonProperty|JsonSerializerSettings" --type=cs | head -30
# 6. Count occurrences to get a sense of scope
echo -e "\n=== Count of potential Newtonsoft.Json usages ==="
rg "JsonConvert|JObject|JArray" --type=cs | wc -lRepository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
=== All .csproj files in the repository ===
GltfInstaller/GltfInstaller.csproj
Revit_glTF_Exporter_2019/Revit_glTF_Exporter_2019.csproj
Revit_glTF_Exporter_2020/Revit_glTF_Exporter_2020.csproj
Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj
Revit_glTF_Exporter_2022/Revit_glTF_Exporter_2022.csproj
Revit_glTF_Exporter_2023/Revit_glTF_Exporter_2023.csproj
Revit_glTF_Exporter_2024/Revit_glTF_Exporter_2024.csproj
Revit_glTF_Exporter_2025/Revit_glTF_Exporter_2025.csproj
Revit_glTF_Exporter_2026/Revit_glTF_Exporter_2026.csproj
=== Checking for remaining Newtonsoft.Json package references in .csproj files ===
=== Checking for glTF.Manipulator package reference in .csproj files ===
78: <PackageReference Include="glTF.Manipulator" Version="2025.11.24.2122" />
78: <PackageReference Include="glTF.Manipulator" Version="2025.11.24.2122" />
126: <PackageReference Include="glTF.Manipulator">
76: <PackageReference Include="glTF.Manipulator">
78: <PackageReference Include="glTF.Manipulator">
76: <PackageReference Include="glTF.Manipulator">
76: <PackageReference Include="glTF.Manipulator">
69: <PackageReference Include="glTF.Manipulator">
=== Checking for remaining Newtonsoft.Json using statements in C# files ===
=== Checking for Newtonsoft.Json API usage (JsonConvert, JObject, JArray, etc.) ===
Common_glTF_Exporter/Export/Draco.cs:65: InlineExternalImagesIntoBin_JObject(dst, baseDir, tempBinFileName, ref binBytes, false, true);
Common_glTF_Exporter/Export/Draco.cs:91: InlineExternalImagesIntoBin_JObject(dst, baseDir, null, ref glbBin, true, true);
Common_glTF_Exporter/Export/Draco.cs:275: private static void InlineExternalImagesIntoBin_JObject(
=== Count of potential Newtonsoft.Json usages ===
3
</shell_result>
glTF.Manipulator 2025.11.24 NuGet package stability release notes
<web_search_result>
I couldn't find a NuGet package named "glTF.Manipulator" or release notes for version/date 2025.11.24. Do you want me to (pick one):
- search NuGet.org and related repos again for that exact package and date, or
- look for similarly named packages (e.g., SharpGLTF, glTFLoader) instead?
</web_search_result>
Script executed:
#!/bin/bash
# Investigate NuGet feed configuration and package source
# 1. Look for nuget.config files
echo "=== Looking for nuget.config or .nuget configuration ==="
find . -name "nuget.config" -o -name "NuGet.Config" 2>/dev/null
# 2. Check for any .nuget directory
echo -e "\n=== Looking for .nuget directory ==="
find . -type d -name ".nuget" 2>/dev/null
# 3. Look at one of the .csproj files to see full context of glTF.Manipulator reference
echo -e "\n=== Full context of glTF.Manipulator reference in 2019 project ==="
rg -n -B2 -A2 "glTF\.Manipulator" "Revit_glTF_Exporter_2019/Revit_glTF_Exporter_2019.csproj"
# 4. Check git diff to see what changed in .csproj files
echo -e "\n=== Git diff summary for .csproj files ==="
git diff --name-only HEAD~1 2>/dev/null | grep -i ".csproj"
# 5. Try to search web for glTF.Manipulator package more broadly
echo -e "\n=== Checking package name variations ==="
rg "glTF|gltf" --type=cs -n | grep -i "import\|using\|package" | head -20Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
=== Looking for nuget.config or .nuget configuration ===
=== Looking for .nuget directory ===
=== Full context of glTF.Manipulator reference in 2019 project ===
74- <Version>3.3.4</Version>
75- </PackageReference>
76: <PackageReference Include="glTF.Manipulator">
77- <Version>2025.11.24.2122</Version>
78- </PackageReference>
=== Git diff summary for .csproj files ===
=== Checking package name variations ===
Common_glTF_Exporter/Core/ProcessGeometry.cs:2:using Common_glTF_Exporter.Model;
Common_glTF_Exporter/Core/ProcessGeometry.cs:3:using Common_glTF_Exporter.Utils;
Common_glTF_Exporter/Core/ProcessGeometry.cs:4:using Common_glTF_Exporter.Windows.MainWindow;
Common_glTF_Exporter/Core/ProcessGeometry.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Core/ProcessGeometry.cs:6:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Core/ProcessGeometry.cs:7:using Revit_glTF_Exporter;
Common_glTF_Exporter/ExternalCommand.cs:8: using Common_glTF_Exporter.Utils;
Common_glTF_Exporter/Core/IndexedDictionary.cs:5: using Common_glTF_Exporter.Core;
Common_glTF_Exporter/Export/RevitGrids.cs:2:using Common_glTF_Exporter.Windows.MainWindow;
Common_glTF_Exporter/Export/RevitGrids.cs:3:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/RevitGrids.cs:4:using Revit_glTF_Exporter;
Common_glTF_Exporter/Export/GlbFile.cs:7:using Common_glTF_Exporter.Core;
Common_glTF_Exporter/Export/GlbFile.cs:8:using Common_glTF_Exporter.Windows.MainWindow;
Common_glTF_Exporter/Export/GlbFile.cs:9:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GltfJson.cs:2:using Common_glTF_Exporter.Core;
Common_glTF_Exporter/Export/GltfJson.cs:3:using Common_glTF_Exporter.Model;
Common_glTF_Exporter/Export/GltfJson.cs:4:using Common_glTF_Exporter.Utils;
Common_glTF_Exporter/Export/GltfJson.cs:5:using Common_glTF_Exporter.Windows.MainWindow;
Common_glTF_Exporter/Export/GltfJson.cs:6:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GltfJson.cs:7:using glTF.Manipulator.Schema;
</shell_result>
glTF.Manipulator NuGet package
<web_search_result>
I couldn't find a NuGet package named exactly "glTF.Manipulator". Searches of nuget.org and common repositories show no package by that name. If you meant a C# glTF library, common NuGet alternatives are:
- glTFLoader — C# glTF loader (nuget.org). [1]
- glTF2Loader — Khronos C# glTF2 loader (nuget.org). [2]
- SharpGLTF — popular C# glTF read/write library (community/package listings). [3]
- Microsoft.glTF.CPP — Microsoft native glTF SDK package (nuget.org; note Microsoft recommends the GitHub repo for latest). [4]
If you want, tell me whether you meant a different package name or whether you want help finding functionality (e.g., editing/manipulating glTF files) and I’ll search for the best match or show usage examples.
</web_search_result>
"glTF.Manipulator" C# library
<web_search_result>
I couldn’t find any C# library named exactly "glTF.Manipulator" (no matching GitHub repos or NuGet packages). If you meant a C# glTF manipulation library, common alternatives are:
- SharpGLTF — a mature .NET library for reading/writing/manipulating glTF 2.0. [1]
- Khronos glTF C# reference loader (glTF Loader NuGet / reference implementation). [2]
- UnityGLTF — a runtime importer/exporter for Unity (pure C#). [3]
If you have a link or more context (NuGet id, GitHub account, or where you saw "glTF.Manipulator"), share it and I’ll look specifically.
Sources: [1] SharpGLTF (GitHub), [2] Khronos glTF C# loader announcement, [3] UnityGLTF (GitHub).
</web_search_result>
Critical: Package dependency cannot be resolved from NuGet registry.
The repository has a blocking issue. The codebase actively imports glTF.Manipulator (found in ProcessGeometry.cs, RevitGrids.cs, GlbFile.cs, GltfJson.cs and elsewhere), and version 2025.11.24.2122 has been added to all 9 project files. However, this package does not exist on the public NuGet.org registry, and no private NuGet feed is configured in the repository.
Build will fail when attempting to restore packages unless:
- The package name or version is incorrect
- A private NuGet feed needs to be configured (missing nuget.config)
- The package has not yet been published to NuGet.org
Verify the package name, version, and NuGet feed configuration before merging.
Positive findings:
- ✓ Newtonsoft.Json has been completely removed from all .csproj files and C# code
- ✓ glTF.Manipulator is consistently referenced across all 9 projects
| <PackageReference Include="glTF.Manipulator"> | ||
| <Version>2025.11.24.2122</Version> | ||
| </PackageReference> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Verification inconclusive
glTF.Manipulator NuGet package version 2025.11.24.2122
<web_search_result>
I searched public NuGet and the web but couldn’t find any public package named "glTF.Manipulator" or the specific version "2025.11.24.2122". It appears not to be published to nuget.org (or may be in a private feed). Do you want me to:
- search GitHub/GitLab for a repository/release with that name and version, or
- check a private/organization feed if you can share its URL, or
- help you inspect a local .nupkg or project to confirm the version?
</web_search_result>
Script executed:
#!/bin/bash
# First, let's check the full context of the .csproj file around lines 76-78
cat -n "Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj" | sed -n '70,85p'Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
70 </EmbeddedResource>
71 </ItemGroup>
72 <ItemGroup>
73 <PackageReference Include="Draco_transcoder">
74 <Version>3.3.4</Version>
75 </PackageReference>
76 <PackageReference Include="glTF.Manipulator">
77 <Version>2025.11.24.2122</Version>
78 </PackageReference>
79 <PackageReference Include="MeshOptimizer_glTFPack">
80 <Version>1.1.0</Version>
81 </PackageReference>
82 <PackageReference Include="Revit_All_Main_Versions_API_x64">
83 <Version>2021.1.4</Version>
84 <PrivateAssets>all</PrivateAssets>
85 <ExcludeAssets>runtime</ExcludeAssets>
</shell_result>
Script executed:
#!/bin/bash
# Look for any references to glTF.Manipulator in the codebase
rg -n "glTF\.Manipulator" --type csRepository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
GltfInstaller/Program.cs:36: new File(@"..\Revit_glTF_Exporter_2019\bin\Release\glTF.Manipulator.dll",
GltfInstaller/Program.cs:47: new File(@"..\Revit_glTF_Exporter_2020\bin\Release\glTF.Manipulator.dll",
GltfInstaller/Program.cs:58: new File(@"..\Revit_glTF_Exporter_2021\bin\Release\glTF.Manipulator.dll",
GltfInstaller/Program.cs:69: new File(@"..\Revit_glTF_Exporter_2022\bin\Release\glTF.Manipulator.dll",
GltfInstaller/Program.cs:80: new File(@"..\Revit_glTF_Exporter_2023\bin\Release\glTF.Manipulator.dll",
GltfInstaller/Program.cs:91: new File(@"..\Revit_glTF_Exporter_2024\bin\Release\glTF.Manipulator.dll",
GltfInstaller/Program.cs:102: new File(@"..\Revit_glTF_Exporter_2025\bin\Release\glTF.Manipulator.dll",
GltfInstaller/Program.cs:115: new File(@"..\Revit_glTF_Exporter_2026\bin\Release\glTF.Manipulator.dll",
Common_glTF_Exporter/Utils/glTFExportUtils.cs:10: using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Utils/glTFExportUtils.cs:11: using glTF.Manipulator.Schema;
Common_glTF_Exporter/Utils/glTFExportUtils.cs:13: using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Utils/MaterialUtils.cs:7: using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Utils/MaterialUtils.cs:8: using glTF.Manipulator.Schema;
Common_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs:3:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Utils/GLTFBinaryDataUtils.cs:4:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Materials/RevitMaterials.cs:15:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Materials/RevitMaterials.cs:16:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Materials/MaterialTextures.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Materials/MaterialTextures.cs:6:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Materials/MaterialProperties.cs:9:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Materials/MaterialProperties.cs:10:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/RevitGrids.cs:3:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/GltfJson.cs:6:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GltfJson.cs:7:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/GltfJson.cs:12:using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Export/GltfJson.cs:21: List<glTF.Manipulator.Schema.Mesh> meshes,
Common_glTF_Exporter/Export/GltfJson.cs:31: glTF.Manipulator.Schema.Version version = new glTF.Manipulator.Schema.Version
Common_glTF_Exporter/Export/GltfJson.cs:103: public static List<glTF.Manipulator.Schema.Material> transformMaterials(List<BaseMaterial> baseMaterials)
Common_glTF_Exporter/Export/GltfJson.cs:105: List<glTF.Manipulator.Schema.Material> materials = new List<glTF.Manipulator.Schema.Material>();
Common_glTF_Exporter/Export/GltfJson.cs:108: glTF.Manipulator.Schema.Material mat = new glTF.Manipulator.Schema.Material();
Common_glTF_Exporter/Export/GlbFile.cs:9:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/GlbBinInfo.cs:3:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/FileExport.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Export/FileExport.cs:6:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/FileExport.cs:11:using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Export/FileExport.cs:12:using Material = glTF.Manipulator.Schema.Material;
Common_glTF_Exporter/Export/FileExport.cs:28: IndexedDictionary<glTF.Manipulator.Schema.Mesh> meshes,
Common_glTF_Exporter/Export/BufferConfig.cs:5:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Export/BufferConfig.cs:6:using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Export/BinFile.cs:7:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/EportUtils/ElementValidations.cs:9:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Core/ProcessGeometry.cs:5:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Core/ProcessGeometry.cs:6:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Core/ProcessGeometry.cs:19: public static void ProcessNode(IndexedDictionary<Node> nodes, Node rootNode, IndexedDictionary<glTF.Manipulator.Schema.Mesh> meshes,
Common_glTF_Exporter/Core/ProcessGeometry.cs:54: glTF.Manipulator.Schema.Mesh newMesh = new glTF.Manipulator.Schema.Mesh
Common_glTF_Exporter/Core/IndexedDictionary.cs:104: if (elem is glTF.Manipulator.Schema.Material)
Common_glTF_Exporter/Core/IndexedDictionary.cs:106: var mat = this.GetElement(uuid) as glTF.Manipulator.Schema.Material;
Common_glTF_Exporter/Core/GlTFExportContext.cs:8:using glTF.Manipulator.GenericSchema;
Common_glTF_Exporter/Core/GlTFExportContext.cs:9:using glTF.Manipulator.Schema;
Common_glTF_Exporter/Core/GlTFExportContext.cs:13:using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Core/GlTFExportContext.cs:61: public IndexedDictionary<glTF.Manipulator.Schema.Mesh> meshes { get; } = new IndexedDictionary<glTF.Manipulator.Schema.Mesh>();
</shell_result>
Script executed:
#!/bin/bash
# Check for Newtonsoft.Json references to verify migration
rg -n "Newtonsoft" --type cs -C 2Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 1
</shell_result>
Script executed:
#!/bin/bash
# Search for System.Text.Json usage to verify the migration
rg -n "System\.Text\.Json" --type cs -C 2Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
Common_glTF_Exporter/Version/LatestVersion.cs-3-using System;
Common_glTF_Exporter/Version/LatestVersion.cs-4-using System.Net.Http;
Common_glTF_Exporter/Version/LatestVersion.cs:5:using System.Text.Json;
Common_glTF_Exporter/Version/LatestVersion.cs-6-using System.Threading.Tasks;
Common_glTF_Exporter/Version/LatestVersion.cs-7-
--
Common_glTF_Exporter/Export/Draco.cs-3-using System.IO;
Common_glTF_Exporter/Export/Draco.cs-4-using System.Reflection;
Common_glTF_Exporter/Export/Draco.cs:5:using System.Text.Json;
Common_glTF_Exporter/Export/Draco.cs:6:using System.Text.Json.Nodes;
Common_glTF_Exporter/Export/Draco.cs-7-using Common_glTF_Exporter.Model;
Common_glTF_Exporter/Export/Draco.cs-8-using Common_glTF_Exporter.Windows.MainWindow;
--
Common_glTF_Exporter/Export/GltfJson.cs-8-using System.Collections.Generic;
Common_glTF_Exporter/Export/GltfJson.cs-9-using System.Linq;
Common_glTF_Exporter/Export/GltfJson.cs:10:using System.Text.Json;
Common_glTF_Exporter/Export/GltfJson.cs:11:using System.Text.Json.Serialization;
Common_glTF_Exporter/Export/GltfJson.cs-12-using Buffer = glTF.Manipulator.Schema.Buffer;
Common_glTF_Exporter/Export/GltfJson.cs-13-
--
Common_glTF_Exporter/Export/GltfJson.cs-79- };
Common_glTF_Exporter/Export/GltfJson.cs-80-
Common_glTF_Exporter/Export/GltfJson.cs:81: string serializedModel = System.Text.Json.JsonSerializer.Serialize(model, options);
Common_glTF_Exporter/Export/GltfJson.cs-82-
Common_glTF_Exporter/Export/GltfJson.cs-83- return serializedModel;
</shell_result>
Script executed:
#!/bin/bash
# Look for NuGet.config which might define private feeds
find . -name "NuGet.config" -o -name "nuget.config" 2>/dev/nullRepository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
</shell_result>
Script executed:
#!/bin/bash
# Check if there's a packages.config or any local packages directory
fd -t f "packages\.config|\.nupkg" | head -20Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
</shell_result>
Script executed:
#!/bin/bash
# Look for any build output or package generation related files
rg -l "glTF\.Manipulator" --type json --type xml | grep -E "(package|nuget|build|config)" | head -10Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
</shell_result>
Script executed:
#!/bin/bash
# Check git log to see if this package was recently added and from what branch
git log --oneline -n 20 -- "Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj" 2>/dev/null | head -10Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
9974b83 Merge pull request #141 from EverseDevelopment/86b7gwka0/DecoupleGltfLogic
</shell_result>
Script executed:
#!/bin/bash
# Check the PR or branch for any build/restore configuration
find . -name "global.json" -o -name "Directory.Build.props" -o -name "Directory.Build.targets" 2>/dev/nullRepository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
</shell_result>
Based on my thorough verification, I can now rewrite the review comment:
glTF.Manipulator package version cannot be verified on public NuGet and will cause build failures without proper package source configuration.
The package version 2025.11.24.2122 is not available on nuget.org. The repository has no NuGet.config to define alternative package sources, which means the build will fail unless this package is available in the builder's environment. Confirm:
- Whether glTF.Manipulator is sourced from a private feed (if so, add
NuGet.configor document the requirement) - Whether the version number is correct
- Whether this package should be publicly available or requires organization-specific setup
🤖 Prompt for AI Agents
In Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj around lines 76-78,
the PackageReference for glTF.Manipulator (version 2025.11.24.2122) is not found
on nuget.org and will break CI/builds; either confirm the version and point to
the correct public package, or add a NuGet.config in the repo that declares the
private feed hosting this package (and include credentials/restore instructions
in CI), or change the PackageReference to a verifiable public version; update
project file or add NuGet.config and documentation accordingly so builds can
restore the package reliably.
| <PackageReference Include="glTF.Manipulator"> | ||
| <Version>2025.11.24.2122</Version> | ||
| </PackageReference> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
glTF.Manipulator NuGet package version 2025.11.24.2122
<web_search_result>
I couldn't find a public NuGet release named glTF.Manipulator version 2025.11.24.2122. The package owner "e-verse" shows glTF.Manipulator with latest public version 2025.11.17.2022 (last updated Nov 17, 2025). [1]
If you want to confirm or install the published version, check the package on nuget.org or run:
dotnet add package glTF.Manipulator --version 2025.11.17.2022
Sources:
[1] e-verse / glTF.Manipulator listing on nuget.org profile.
</web_search_result>
Script executed:
#!/bin/bash
# Search for any remaining Newtonsoft.Json references in the codebase
rg -i "newtonsoft" --type=cs --type=csproj -n
# Also search for System.Text.Json usage
rg "System\.Text\.Json" --type=cs -n | head -20Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
Common_glTF_Exporter/Version/LatestVersion.cs:5:using System.Text.Json;
Common_glTF_Exporter/Export/GltfJson.cs:10:using System.Text.Json;
Common_glTF_Exporter/Export/GltfJson.cs:11:using System.Text.Json.Serialization;
Common_glTF_Exporter/Export/GltfJson.cs:81: string serializedModel = System.Text.Json.JsonSerializer.Serialize(model, options);
Common_glTF_Exporter/Export/Draco.cs:5:using System.Text.Json;
Common_glTF_Exporter/Export/Draco.cs:6:using System.Text.Json.Nodes;
stderr:
unrecognized file type: csproj
</shell_result>
Script executed:
#!/bin/bash
# Check the full .csproj file to see all package references and target framework
cat -n "Revit_glTF_Exporter_2024/Revit_glTF_Exporter_2024.csproj" | head -100Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
stdout:
1 <?xml version="1.0" encoding="utf-8"?>
2 <Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3 <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
4 <PropertyGroup>
5 <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
6 <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
7 <ProjectGuid>{C7A8980F-6129-4C0B-9CF1-1CB1023197E7}</ProjectGuid>
8 <OutputType>Library</OutputType>
9 <AppDesignerFolder>Properties</AppDesignerFolder>
10 <RootNamespace>Revit_glTF_Exporter</RootNamespace>
11 <AssemblyName>Leia_glTF_Exporter</AssemblyName>
12 <TargetFrameworkVersion>v4.8</TargetFrameworkVersion>
13 <FileAlignment>512</FileAlignment>
14 <Deterministic>true</Deterministic>
15 <NuGetPackageImportStamp>
16 </NuGetPackageImportStamp>
17 </PropertyGroup>
18 <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
19 <DebugSymbols>true</DebugSymbols>
20 <DebugType>full</DebugType>
21 <Optimize>false</Optimize>
22 <OutputPath>bin\Debug\</OutputPath>
23 <DefineConstants>TRACE;DEBUG;REVIT2024</DefineConstants>
24 <ErrorReport>prompt</ErrorReport>
25 <WarningLevel>4</WarningLevel>
26 </PropertyGroup>
27 <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
28 <DebugType>none</DebugType>
29 <Optimize>true</Optimize>
30 <OutputPath>bin\Release\</OutputPath>
31 <DefineConstants>TRACE;DEBUG;REVIT2024</DefineConstants>
32 <ErrorReport>prompt</ErrorReport>
33 <WarningLevel>4</WarningLevel>
34 <CodeAnalysisRuleSet>e-verse.ruleset</CodeAnalysisRuleSet>
35 </PropertyGroup>
36 <ItemGroup>
37 <Reference Include="PresentationCore" />
38 <Reference Include="PresentationFramework" />
39 <Reference Include="System" />
40 <Reference Include="System.ComponentModel.DataAnnotations" />
41 <Reference Include="System.Configuration" />
42 <Reference Include="System.Core" />
43 <Reference Include="System.Drawing" />
44 <Reference Include="System.Drawing.Design" />
45 <Reference Include="System.IO.Compression" />
46 <Reference Include="System.IO.Compression.FileSystem" />
47 <Reference Include="System.Numerics" />
48 <Reference Include="System.Numerics.Vectors, Version=4.1.4.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
49 <Private>False</Private>
50 </Reference>
51 <Reference Include="System.Windows" />
52 <Reference Include="System.Windows.Forms" />
53 <Reference Include="System.Xaml" />
54 <Reference Include="System.Xml.Linq" />
55 <Reference Include="System.Data.DataSetExtensions" />
56 <Reference Include="Microsoft.CSharp" />
57 <Reference Include="System.Data" />
58 <Reference Include="System.Net.Http" />
59 <Reference Include="System.Xml" />
60 <Reference Include="WindowsBase" />
61 </ItemGroup>
62 <ItemGroup>
63 <Compile Include="Properties\AssemblyInfo.cs" />
64 </ItemGroup>
65 <ItemGroup>
66 <PackageReference Include="Draco_transcoder">
67 <Version>3.3.4</Version>
68 </PackageReference>
69 <PackageReference Include="glTF.Manipulator">
70 <Version>2025.11.24.2122</Version>
71 </PackageReference>
72 <PackageReference Include="MeshOptimizer_glTFPack">
73 <Version>1.1.0</Version>
74 </PackageReference>
75 <PackageReference Include="Revit_All_Main_Versions_API_x64">
76 <Version>2024.0.0</Version>
77 <PrivateAssets>all</PrivateAssets>
78 <ExcludeAssets>runtime</ExcludeAssets>
79 </PackageReference>
80 </ItemGroup>
81 <ItemGroup>
82 <Folder Include="Resources\" />
83 </ItemGroup>
84 <Import Project="..\Common_glTF_Exporter\Common_glTF_Exporter.projitems" Label="Shared" />
85 <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
86 <PropertyGroup>
87 <PostBuildEvent>if $(ConfigurationName) == Debug (
88 mkdir "$(APPDATA)\Autodesk\ApplicationPlugins\leia.bundle\Contents\2024\"
89 mkdir "$(APPDATA)\Autodesk\ApplicationPlugins\leia.bundle\Contents\Resources\"
90 copy "$(SolutionDir)Common_glTF_Exporter\*.xml" "$(APPDATA)\Autodesk\ApplicationPlugins\leia.bundle\"
91 copy "$(ProjectDir)$(OutputPath)*.dll" "$(APPDATA)\Autodesk\ApplicationPlugins\leia.bundle\Contents\2024\"
92 copy "$(SolutionDir)Common_glTF_Exporter\*.addin" "$(APPDATA)\Autodesk\ApplicationPlugins\leia.bundle\Contents\2024\"
93 )
94 If "%25CI%25"=="true" (
95 "C:\Users\runneradmin\.nuget\packages\signtool\10.0.17763.132\tools\signtool.exe" sign /f $(CERTIFICATE_LOCATION) /p $(CERTIFICATE_KEY) /t http://timestamp.digicert.com "$(ProjectDir)$(OutputPath)Leia_glTF_Exporter.dll"
96 )</PostBuildEvent>
97 </PropertyGroup>
98 </Project>
</shell_result>
Script executed:
#!/bin/bash
# Look for NuGet configuration files
fd -name "NuGet.config" -o -name "nuget.config"
# Also check for any references to custom package sources in project files
rg "packageSources|nugetSource" --type=xml -nRepository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 1
stderr:
error: Found argument '-n' which wasn't expected, or isn't valid in this context
If you tried to supply '-n' as a value rather than a flag, use '-- -n'
Usage: fd [OPTIONS] [pattern] [path]...
For more information try '--help'
</shell_result>
Script executed:
#!/bin/bash
# Check if there's any documentation about the package version
find . -name "*.md" -o -name "*.txt" | xargs grep -l "2025.11.24.2122" 2>/dev/null | head -5Repository: EverseDevelopment/revit-glTF-exporter
<shell_result>
Repository: EverseDevelopment/revit-glTF-exporter
Exit code: 0
</shell_result>
Urgent: glTF.Manipulator version 2025.11.24.2122 does not exist on public NuGet.
The specified version 2025.11.24.2122 is not available in public NuGet registries. The latest public release from e-verse is 2025.11.17.2022 (Nov 17, 2025). This version mismatch will cause build failures for anyone without access to a private feed. Verify and correct the version to either:
2025.11.17.2022(latest public stable release), or- Document the private NuGet feed configuration if an internal version is intentional
Additionally, add a NuGet.config file to the repository root if a private feed is required, so other developers can access it.
🤖 Prompt for AI Agents
In Revit_glTF_Exporter_2024/Revit_glTF_Exporter_2024.csproj around lines 69 to
71 the PackageReference pins glTF.Manipulator to version 2025.11.24.2122 which
does not exist on public NuGet and will break builds; update the Version to the
public release 2025.11.17.2022 unless you intentionally rely on a private
feed—if so, revert the version change and add a NuGet.config at the repo root
pointing to the private feed and document the feed credentials/access in README
so other developers can restore packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (7)
Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj (1)
76-78: glTF.Manipulator package version remains unverifiable and will cause build failures without proper feed configuration.The package version
2025.11.27.1900still cannot be confirmed on nuget.org (prior review confirmed2025.11.24.2122was not publicly available). The repo has noNuGet.configdefining alternative package sources, so builds will fail unless this package is in the builder's environment cache or a private feed is configured.Before merge, you must either:
- Verify the package is publicly available on nuget.org (or the version number is correct), or
- Add a
NuGet.configin the repo root that declares the private feed and include restore instructions in CI setup, or- Confirm this is sourced from a private organization feed and document the CI environment setup requirement
#!/bin/bash # Verify glTF.Manipulator 2025.11.27.1900 is publicly available and package can be restored curl -s "https://api.nuget.org/v3-flatcontainer/gltf.manipulator/index.json" | jq '.versions | map(select(. == "2025.11.27.1900"))' # Also check for any NuGet.config find . -iname "nuget.config" -o -iname "packages.config"Revit_glTF_Exporter_2019/Revit_glTF_Exporter_2019.csproj (1)
76-78: Verify glTF.Manipulator package availability on NuGet registry.Same package availability concern as in other project files. The package
glTF.Manipulatorversion2025.11.27.1900needs verification before merging.Revit_glTF_Exporter_2026/Revit_glTF_Exporter_2026.csproj (1)
78-78: Verify glTF.Manipulator package availability.Same package availability concern applies to this project file.
Common_glTF_Exporter/Core/ProcessGeometry.cs (1)
86-88: Handle ExportFaces returning -1 to avoid invalid glTF output.
ExportFacesreturns-1when there are no faces to export. Settingprimitive.indices = -1produces invalid glTF output, as accessor indices must be non-negative. The primitive should either be skipped or the indices field left unset when no faces exist.Apply this diff to handle the failure case:
int indexAcessor = GLTFBinaryDataUtils.ExportFaces(kvp.Value, gLTFBinaryDataElement, bufferViews, accessors); - primitive.indices = indexAcessor; + if (indexAcessor < 0) + { + // Skip this primitive if no faces exist + continue; + } + + primitive.indices = indexAcessor;Common_glTF_Exporter/Utils/MaterialUtils.cs (1)
47-47: Color values should be converted to linear space for glTF compliance.This issue was previously flagged: glTF requires
baseColorFactorto be in linear color space, but this code uses sRGB values directly. TheMaterialProperties.SrgbToLinearmethod exists for this conversion but isn't used here.Apply this diff to fix the color space conversion:
- gl_mat.baseColorFactor = new List<float>(4) { material.Color.Red / 255f, material.Color.Green / 255f, material.Color.Blue / 255f, opacity }; + float lr = MaterialProperties.SrgbToLinear(material.Color.Red / 255f); + float lg = MaterialProperties.SrgbToLinear(material.Color.Green / 255f); + float lb = MaterialProperties.SrgbToLinear(material.Color.Blue / 255f); + gl_mat.baseColorFactor = new List<float>(4) { lr, lg, lb, opacity };Common_glTF_Exporter/Utils/glTFExportUtils.cs (1)
26-38: Parameter "materialOpacity" is misnamed; formula produces fully transparent default material.This issue was previously flagged: the method treats
materialOpacityas transparency (formulaopacity = 1 - materialOpacity), but the parameter name suggests opacity. When called withCreateDefaultGLTFMaterial(1, true), this producesopacity = 0(fully transparent), making the default material invisible.Apply this diff to fix the parameter semantics:
- public static BaseMaterial CreateDefaultGLTFMaterial(int materialOpacity, bool doubleSided) + public static BaseMaterial CreateDefaultGLTFMaterial(float materialOpacity, bool doubleSided) { BaseMaterial baseMaterial = new BaseMaterial(); baseMaterial.doubleSided = doubleSided; - float opacity = 1 - (float)materialOpacity; + float opacity = materialOpacity; baseMaterial.name = DEF_MATERIAL_NAME; baseMaterial.baseColorFactor = new List<float>(4) { 1f, 1f, 1f, opacity }; baseMaterial.metallicFactor = 0f; baseMaterial.roughnessFactor = 1f; baseMaterial.uuid = DEF_UNIQUEL_ID; return baseMaterial; }And update the caller in
GlTFExportContext.csline 115:- currentMaterial = GLTFExportUtils.CreateDefaultGLTFMaterial(1, true); + currentMaterial = GLTFExportUtils.CreateDefaultGLTFMaterial(1f, true);Common_glTF_Exporter/Core/GlTFExportContext.cs (1)
386-386: Inconsistent material key: uses name instead of uuid.This issue was previously flagged: line 386 uses
currentMaterial.nameas the key forAddOrUpdateCurrentMaterial, but line 117 (and other locations like RevitMaterials.cs line 50) usecurrentMaterial.uuid. This inconsistency could cause duplicate materials ifname != uuidor lookup failures when retrieving materials by uuid.Apply this diff:
- materials.AddOrUpdateCurrentMaterial(currentMaterial.name, currentMaterial, true); + materials.AddOrUpdateCurrentMaterial(currentMaterial.uuid, currentMaterial, true);
🧹 Nitpick comments (5)
Common_glTF_Exporter/Export/BinFile.cs (1)
11-11: Use English for code comments.The inline comment is in Spanish. For consistency and maintainability in an international codebase, please use English for all code comments.
Apply this diff:
- // Obtiene los bytes finales del MemoryStream + // Get the final bytes from the MemoryStreamCommon_glTF_Exporter/Materials/RevitMaterials.cs (4)
30-36: Consider lazy initialization to avoid unnecessary allocation.A new
BaseMaterialis created unconditionally at line 30, but if the material is already cached (line 34), this allocation is immediately discarded when the cached material is retrieved (line 36).Apply this diff to defer allocation until needed:
- BaseMaterial material = new BaseMaterial(); + BaseMaterial material; string materialId = node.MaterialId.ToString(); - material.uuid = materialId; if (materials.Contains(materialId)) { material = materials.GetElement(materialId); } else { + material = new BaseMaterial(); + material.uuid = materialId; Autodesk.Revit.DB.Material revitMaterial = doc.GetElement(node.MaterialId) as Autodesk.Revit.DB.Material;
66-66: Consider validating the Transparency value range.The
node.Transparencyvalue is used to calculate opacity without validation. IfTransparencycan be outside the [0, 1] range, this could result in invalid opacity values.Apply this diff to add validation:
- float opacity = ONEINTVALUE - (float)node.Transparency; + float transparency = Math.Max(0f, Math.Min(1f, (float)node.Transparency)); + float opacity = ONEINTVALUE - transparency;
71-71: Tighten variable scope for better readability.The
baseNTintColourvariable is declared outside the conditional block where it's used (line 75), making its purpose less clear.Apply this diff to move the declaration closer to its usage:
- (Autodesk.Revit.DB.Color, Autodesk.Revit.DB.Color) baseNTintColour = (null, null); - if (revitMaterial != null && preferences.materials == MaterialsEnum.textures) { + (Autodesk.Revit.DB.Color, Autodesk.Revit.DB.Color) baseNTintColour = (null, null); baseNTintColour = MaterialTextures.SetMaterialTextures(revitMaterial, material, doc, opacity, textures, images);Note: Adjust line 85 accordingly if
baseNTintColouris moved into the conditional scope.
73-73: Redundant null check on required parameter.The method checks
revitMaterial != nullat line 73, but based on the calling context inProcessMaterial(line 48), this method is only called whenrevitMaterialis confirmed to be non-null. Consider removing this check or documenting why it's necessary for defensive programming.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
Common_glTF_Exporter/Common_glTF_Exporter.projitems(1 hunks)Common_glTF_Exporter/Core/GlTFExportContext.cs(10 hunks)Common_glTF_Exporter/Core/IndexedDictionary.cs(0 hunks)Common_glTF_Exporter/Core/ProcessGeometry.cs(1 hunks)Common_glTF_Exporter/EportUtils/ElementValidations.cs(1 hunks)Common_glTF_Exporter/Export/BinFile.cs(1 hunks)Common_glTF_Exporter/Export/Draco.cs(2 hunks)Common_glTF_Exporter/Export/FileExport.cs(2 hunks)Common_glTF_Exporter/Export/GltfJson.cs(3 hunks)Common_glTF_Exporter/Export/RevitGrids.cs(3 hunks)Common_glTF_Exporter/Materials/RevitMaterials.cs(2 hunks)Common_glTF_Exporter/Utils/MaterialUtils.cs(1 hunks)Common_glTF_Exporter/Utils/glTFExportUtils.cs(2 hunks)Common_glTF_Exporter/Version/LatestVersion.cs(2 hunks)GltfInstaller/Program.cs(8 hunks)Revit_glTF_Exporter_2019/Revit_glTF_Exporter_2019.csproj(1 hunks)Revit_glTF_Exporter_2020/Revit_glTF_Exporter_2020.csproj(1 hunks)Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj(1 hunks)Revit_glTF_Exporter_2022/Revit_glTF_Exporter_2022.csproj(1 hunks)Revit_glTF_Exporter_2023/Revit_glTF_Exporter_2023.csproj(1 hunks)Revit_glTF_Exporter_2024/Revit_glTF_Exporter_2024.csproj(1 hunks)Revit_glTF_Exporter_2025/Revit_glTF_Exporter_2025.csproj(1 hunks)Revit_glTF_Exporter_2026/Revit_glTF_Exporter_2026.csproj(1 hunks)
💤 Files with no reviewable changes (1)
- Common_glTF_Exporter/Core/IndexedDictionary.cs
🚧 Files skipped from review as they are similar to previous changes (6)
- Revit_glTF_Exporter_2020/Revit_glTF_Exporter_2020.csproj
- Common_glTF_Exporter/Version/LatestVersion.cs
- Revit_glTF_Exporter_2023/Revit_glTF_Exporter_2023.csproj
- Common_glTF_Exporter/Export/Draco.cs
- Revit_glTF_Exporter_2025/Revit_glTF_Exporter_2025.csproj
- Revit_glTF_Exporter_2024/Revit_glTF_Exporter_2024.csproj
🧰 Additional context used
🧬 Code graph analysis (5)
Common_glTF_Exporter/Export/RevitGrids.cs (2)
GltfInstaller/Dialogs/FeaturesDialog.xaml.cs (1)
Node(47-57)Common_glTF_Exporter/Utils/Util.cs (2)
Dictionary(254-312)Util(10-344)
Common_glTF_Exporter/Export/FileExport.cs (4)
Common_glTF_Exporter/Materials/MaterialProperties.cs (3)
List(26-52)List(55-58)List(70-77)Common_glTF_Exporter/Utils/glTFExportUtils.cs (2)
BaseMaterial(21-24)BaseMaterial(26-38)Common_glTF_Exporter/Export/BufferConfig.cs (2)
BufferConfig(10-48)Run(14-47)Common_glTF_Exporter/Export/BinFile.cs (2)
BinFile(4-21)Create(9-20)
Common_glTF_Exporter/Utils/MaterialUtils.cs (2)
Common_glTF_Exporter/Utils/glTFExportUtils.cs (3)
BaseMaterial(21-24)BaseMaterial(26-38)GLTFExportUtils(15-149)Common_glTF_Exporter/Export/GltfJson.cs (2)
List(82-97)List(99-139)
Common_glTF_Exporter/EportUtils/ElementValidations.cs (2)
Common_glTF_Exporter/Export/RevitGrids.cs (1)
Export(23-75)GltfInstaller/Dialogs/FeaturesDialog.xaml.cs (1)
Node(47-57)
Common_glTF_Exporter/Utils/glTFExportUtils.cs (3)
Common_glTF_Exporter/Utils/MaterialUtils.cs (2)
Autodesk(15-27)BaseMaterial(29-54)Common_glTF_Exporter/Materials/MaterialTextures.cs (1)
Autodesk(20-62)Common_glTF_Exporter/Materials/RevitMaterials.cs (2)
BaseMaterial(26-54)BaseMaterial(60-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (25)
Revit_glTF_Exporter_2021/Revit_glTF_Exporter_2021.csproj (1)
82-84: Newtonsoft.Json formatting is acceptable.The change from inline
Version="13.0.1"attribute to nested<Version>13.0.1</Version>element is a style normalization. Version 13.0.1 is publicly available and stable. No action needed here.GltfInstaller/Program.cs (2)
36-37: glTF.Manipulator.dll added consistently for 2019–2024The new
glTF.Manipulator.dllentries for 2019–2024 use the expected per-year Revit exporter paths and match the existing pattern (same directory, sameFilePermissionas other DLLs). This looks correct and keeps the payload symmetrical across versions.Please just confirm that:
- Each
Revit_glTF_Exporter_20XXproject actually producesglTF.Manipulator.dllintobin\Release, and- The runtime target of
glTF.Manipulator.dllis compatible with the corresponding Revit/Leia exporter version.Also applies to: 49-50, 62-63, 75-76, 88-89, 101-102
114-119: 2025/2026 payload: glTF.Manipulator + .deps.json + Newtonsoft.Json orderingFor 2025 and 2026 you now ship:
glTF.Manipulator.dllLeia_glTF_Exporter.deps.jsonNewtonsoft.Json.dllAll three are in the same
bin\Releasefolder with consistentFilePermissionusage, and the 2025/2026 blocks are symmetrical with each other, which is good.A couple of points to double-check:
- That the
.deps.jsonfile name and location exactly match what the Leia exporter build produces (case and path sensitive for the installer).- That there isn’t a second, stale
Newtonsoft.Json.dllentry left elsewhere in the project definition for 2025/2026 (to avoid duplicate component/wix id issues at build time).If those hold, the changes look sound.
Also applies to: 129-134
Common_glTF_Exporter/Export/BinFile.cs (1)
9-19: LGTM! Simplified binary write implementation.The refactored Create method is cleaner and more efficient. Migrating from multi-buffer writing to a single
globalBuffer.ToArray()write operation simplifies the logic and improves maintainability. The use ofBufferedStreamis appropriate for performance.Common_glTF_Exporter/EportUtils/ElementValidations.cs (1)
23-23: LGTM! Type migration aligned with refactor.The parameter type change from
IndexedDictionary<GLTFNode>toIndexedDictionary<Node>correctly aligns with the broader migration toglTF.Manipulator.Schematypes throughout the codebase.Common_glTF_Exporter/Export/GltfJson.cs (3)
72-78: Inconsistency: AI summary incorrectly states System.Text.Json is used.The AI summary claims "Switches serialization from Json.NET to System.Text.Json," but the code clearly uses
Newtonsoft.Json.JsonConvertfor serialization (lines 5, 72-78). The implementation is correct, but the summary is misleading.
14-80: LGTM! Clean refactor to glTF.Manipulator types.The migration from legacy GLTF types to
glTF.Manipulator.Schematypes (Scene, Node, Mesh, BaseMaterial, Buffer, BufferView, Accessor, Texture, glTFImage) is well-executed. The conditional emission of materials and textures based on preferences is correctly implemented.
82-139: LGTM! Well-structured helper methods.The
cleanImagesandtransformMaterialshelper methods are clean and focused. They correctly transform the internal model types to glTF-compliant output structures, including proper handling of texture extensions (KHR_texture_transform).Common_glTF_Exporter/Core/ProcessGeometry.cs (1)
20-104: LGTM! Centralized geometry processing with proper version handling.The
ProcessNodemethod effectively centralizes geometry export logic, including node/mesh creation, primitive assembly, attribute export, and material assignment. The conditional compilation for different Revit versions (lines 46-50) is correctly implemented.Revit_glTF_Exporter_2022/Revit_glTF_Exporter_2022.csproj (1)
76-78: The glTF.Manipulator package version 2025.11.27.1900 is publicly available on NuGet.org—the original concern is unfounded.Verification confirms the package exists on NuGet.org with the specified version present in the public registry. No private NuGet feed configuration is required, and the absence of a
nuget.configfile is not problematic since NuGet defaults to the official NuGet.org registry. The package reference will restore successfully during build.Likely an incorrect or invalid review comment.
Revit_glTF_Exporter_2026/Revit_glTF_Exporter_2026.csproj (1)
81-81: Security concern is unfounded; however, the downgrade removes functional bug fixes.The downgrade to Newtonsoft.Json 13.0.1 does not introduce security vulnerabilities. All three versions (13.0.1, 13.0.2, 13.0.3) include the CVE-2024-21907 DoS vulnerability mitigation. However, downgrading from 13.0.3 loses bug fixes from 13.0.2 and 13.0.3, including:
- MaxDepth fixes when used with ToObject inside JsonConverter
- ARM processor race condition in name table deserialization
- JToken type deserialization/merging fixes
- Decimal trailing zero preservation and negative zero parsing fixes
Verify this downgrade is intentional and consider the impact of these lost improvements.
Common_glTF_Exporter/Common_glTF_Exporter.projitems (1)
12-13: Review comment verification is incorrect — no issues found.The review comment conflates external package types with removed project files.
GLTFBinaryData,IndexedDictionary, andglTFImageare not removed project files; they are classes from the glTF.Manipulator NuGet package (v2025.11.27.1900), which is referenced in all project files.The actual removed legacy GLTF core model classes (GLTFImage, GLTFPBR, GLTFExtras, GLTFAccessor, GLTFBufferView, etc.) are not referenced anywhere in the codebase, confirming they have been cleanly removed without dangling references.
Common_glTF_Exporter/Utils/MaterialUtils.cs (1)
1-27: LGTM! Type migrations are consistent.The using statements and type updates (fully-qualified Autodesk.Revit.DB types and new glTF.Manipulator namespaces) align with the broader refactor from legacy GLTF types to BaseMaterial/Schema types.
Common_glTF_Exporter/Export/RevitGrids.cs (1)
1-77: LGTM! Clean migration to Node/Extras with proper formatting.The type migrations (GLTFNode→Node, GLTFExtras→Extras) are consistent, and the use of
CultureInfo.InvariantCulturefor numeric parameter formatting ensures locale-independent serialization.Common_glTF_Exporter/Utils/glTFExportUtils.cs (2)
1-24: LGTM! Type migrations and API simplification are appropriate.The updated using statements, public constant exposure, and simplified
GetGLTFMaterialsignature align with the BaseMaterial refactor.
40-149: LGTM! BaseMaterial integration is consistent.The
AddOrUpdateCurrentItemsignature update and vertex key construction usingmaterial.uuidalign correctly with the BaseMaterial type system.Common_glTF_Exporter/Export/FileExport.cs (1)
1-59: LGTM! Type migrations and simplified binary data handling.The Run signature updates (BaseMaterial, Node, Scene, etc.) and the shift to a single
GLTFBinaryDatainstance (instead of a list) are consistent with the broader refactor. Type aliases for Buffer and Material appropriately disambiguate from System types.Common_glTF_Exporter/Core/GlTFExportContext.cs (4)
1-121: LGTM! Core type migrations are consistent.The field type updates (GLTFNode→Node, GLTFMaterial→BaseMaterial, etc.) and
Start()initialization usingcurrentMaterial.uuidfor material indexing align with the schema refactor.
122-163: LGTM! Finish() updated to use single global buffer.The Buffer creation and
FileExport.Runcall using a singleglobalBufferinstance (instead of a list) simplifies the export flow and is consistent with changes in FileExport.cs.
164-241: LGTM! Centralized geometry processing improves architecture.The delegation to
ProcessGeometry.ProcessNodeinOnElementEndand the updatedRevitMaterials.ProcessMaterialcall inOnMaterialalign with the centralized geometry export pipeline.
242-291: LGTM! Texture handling updated for BaseMaterial.The switch from
EmbeddedTexturePathtocurrentMaterial.hasTextureis correct, and the UV flipping logic properly converts to glTF's texture coordinate convention.Common_glTF_Exporter/Materials/RevitMaterials.cs (4)
14-17: LGTM: New imports support the migration to glTF.Manipulator types.The added imports are necessary for the new BaseMaterial type and associated utilities used throughout the refactored methods.
51-51: Clarify whether updating the cache for already-cached materials is intentional.Line 51 updates the materials cache even when the material was retrieved from the cache at line 36. If the cached material is immutable or the update is meant to refresh metadata, this is correct. Otherwise, it may be redundant.
79-86: LGTM: Clear logic for setting baseColorFactor.The conditional logic correctly sets
baseColorFactorto a default color when textures are present (allowing the texture to define the color) and uses the material's computed color otherwise.
44-44: Based on my verification of the codebase, I can now generate the rewritten review comment.
GLTFExportUtils.GetGLTFMaterialexists and retrieves a pre-initialized default material, but relies on an implicit initialization guarantee.The method is implemented at
Common_glTF_Exporter/Utils/glTFExportUtils.cs:21and returns aBaseMaterialby callingMaterials.GetElement(DEF_UNIQUEL_ID). The default material withuuid = "8a3c94b3-d9e2-4e57-9189-f9bb6a9a54a4"is created and added to the materials dictionary during export context initialization (GlTFExportContext.cs:115-117). However, the method provides no defensive null-checking; if the default material is not present in the dictionary for any reason,GetElementmay return null, causing aNullReferenceExceptionat the call site (e.g.,MaterialUtils.cs:43accesses.doubleSidedwithout null validation). Consider adding an explicit null check or fallback toCreateDefaultGLTFMaterialto handle edge cases where the dictionary lacks the expected default entry.
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.