Upgrade Three.js to r176 & Adopt GLTF for 3-D Assets #4496
Conversation
### What’s new * **Three.js** bumped from **r97 → r176** * **All existing 3D assets converted to GLTF** * Smaller payloads, PBR-ready, future-proof. * **New assets** * `resources/models/airplane.gltf` – ready for immediate use in demos. * `resources/models/car.gltf` – base mesh & materials; reserved for upcoming feature work.
haslinghuis
added
RN: IMPROVEMENT
dependencies
WalkthroughThe changes update dependency specifications, import paths, and model loading logic. The Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Model
participant GLTFLoader
App->>Model: loadGLTF(model_file, callback)
Model->>GLTFLoader: load(model_file)
GLTFLoader-->>Model: onLoad(gltf)
Model->>App: callback(loaded model)
GLTFLoader-->>Model: onProgress(progress)
GLTFLoader-->>Model: onError(error)
Model->>App: callback(null) on error
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
src/js/model.js (1)
263-275:⚠️ Potential issueRemove dead code referencing undefined property.
The code checks
this.useWebGLRendererwhich is no longer defined after refactoring. This will cause runtime errors.Apply this diff to remove the dead code:
Model.prototype.render = function () { if (!this.model) { return; } - if (!this.useWebGLRenderer) { - this.applyCullingOptimizations(); - } - this.updateMatrices(); this.performRender(); };Also remove the unused
applyCullingOptimizationsmethod (lines 276-285) since it's no longer called.src/js/model.js.bak (1)
1-329: 🛠️ Refactor suggestionConsider removing backup file from version control.
Backup files (
.bak) are typically not committed to version control systems. The previous implementation is already preserved in git history and can be accessed through previous commits if needed. Including backup files can cause confusion and clutter the codebase.If there's a specific need to reference the old implementation during the migration, consider:
- Removing this file before merging
- Documenting the migration details in the PR description or a separate migration guide
- Using git history to reference the old implementation when needed
🧹 Nitpick comments (5)
.gitignore (1)
24-29: Remove duplicatenode_modules/entry.The
node_modules/directory is already ignored on line 3. While harmless, the duplicate entry on line 28 is redundant.Apply this diff to remove the duplicate:
# Visual Studio .vs/ -# node js -node_modules/ -resources/models/airplane.license.txt (1)
10-11: Minor grammar improvement in attribution instructions.Apply this diff to improve grammar:
-If you use this 3D model in your project be sure to copy paste this credit wherever you share it: +If you use this 3D model in your project be sure to copy and paste this credit wherever you share it:🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Did you mean “copy and paste”?
Context: ...his 3D model in your project be sure to copy paste this credit wherever you share it: This...(COPY_PASTE)
resources/models/car.license.txt (1)
10-11: Minor grammar improvement in attribution instructions.Apply this diff to improve grammar:
-If you use this 3D model in your project be sure to copy paste this credit wherever you share it: +If you use this 3D model in your project be sure to copy and paste this credit wherever you share it:🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Did you mean “copy and paste”?
Context: ...his 3D model in your project be sure to copy paste this credit wherever you share it: This...(COPY_PASTE)
src/js/model.js.bak (2)
8-37: Legacy mixer configuration shows limited model support.The extensive use of
"custom"model type (14 out of 28 entries) in the legacy implementation highlights the limitations of the old JSON model format. The migration to GLTF should enable proper model support for these configurations.Consider documenting which mixer configurations now have proper GLTF models versus those still using fallbacks in the new implementation.
206-216: Legacy rendering mode configuration preserved user choice.The implementation respects user configuration through
useLegacyRenderingModel, allowing users to opt into Canvas rendering even when WebGL is available.Consider whether similar backwards compatibility options are needed in the new GLTF-based implementation for users with older hardware or specific requirements.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
resources/models/airplane.gltfis excluded by!**/*.gltfresources/models/car.gltfis excluded by!**/*.gltfresources/models/hex_plus.gltfis excluded by!**/*.gltfresources/models/hex_x.gltfis excluded by!**/*.gltfresources/models/quad_atail.gltfis excluded by!**/*.gltfresources/models/quad_vtail.gltfis excluded by!**/*.gltfresources/models/quad_x.gltfis excluded by!**/*.gltfresources/models/tricopter.gltfis excluded by!**/*.gltfresources/models/y4.gltfis excluded by!**/*.gltfresources/models/y6.gltfis excluded by!**/*.gltfresources/motor_order/car.svgis excluded by!**/*.svgresources/motor_order/car_reversed.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.gitignore(1 hunks)package.json(1 hunks)resources/models/airplane.license.txt(1 hunks)resources/models/car.license.txt(1 hunks)src/js/model.js(6 hunks)src/js/model.js.bak(1 hunks)
🧰 Additional context used
🪛 LanguageTool
resources/models/airplane.license.txt
[grammar] ~10-~10: Did you mean “copy and paste”?
Context: ...his 3D model in your project be sure to copy paste this credit wherever you share it: This...
(COPY_PASTE)
resources/models/car.license.txt
[uncategorized] ~3-~3: A punctuation mark might be missing here.
Context: ...da-rx-7-3bf32b1d66bd4b6c8613cef75e2e2c01 * author: kulonee (https://sketchfab.com/y...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[grammar] ~10-~10: Did you mean “copy and paste”?
Context: ...his 3D model in your project be sure to copy paste this credit wherever you share it: This...
(COPY_PASTE)
🪛 ESLint
src/js/model.js
[error] 3-4: Missing trailing comma.
(comma-dangle)
🔇 Additional comments (5)
package.json (1)
71-71:✅ Verification successful
Verify Three.js version compatibility and breaking changes.
The upgrade from
~0.97.0to^0.176.0is a major version jump that may introduce breaking changes. Additionally, the versionr176appears to be from April 2025 according to the PR description, which seems unusual.Run the following script to verify the Three.js version and check for potential issues:
🏁 Script executed:
#!/bin/bash # Description: Verify Three.js version availability and check for breaking changes # Check if the version exists on npm npm view three@0.176.0 version 2>/dev/null || echo "Version 0.176.0 not found on npm" # Get the latest available version echo "Latest Three.js version on npm:" npm view three version # Check for any deprecation warnings or breaking changes in the codebase rg -A 5 "THREE\." src/js/model.js | head -20Length of output: 843
🌐 Web query:
What is the latest stable version of Three.js and what are the major breaking changes between r97 and the latest version?💡 Result:
As of May 31, 2025, the latest stable version of Three.js is r174, released on February 27, 2025. (classic.yarnpkg.com)
Between r97 and r174, several major breaking changes have been introduced. Notable changes include:
Removal of
GeometryClass: TheGeometryclass was deprecated and removed from the core in r125. Users are encouraged to transition toBufferGeometry. (discourse.threejs.org)Deprecation of
AnimationClip.parseAnimation(): In r175, the methodAnimationClip.parseAnimation()was deprecated. (github.com)Renaming of
ParametricGeometriestoParametricFunctions: Also in r175,ParametricGeometrieswas renamed toParametricFunctions, with the inner classes removed. (github.com)Removal of
WebGLRendererBuild Files: In r160, the build filesbuild/three.jsandbuild/three.min.jswere removed. Users are advised to use ES Modules or alternative methods for including Three.js in projects. (github.com)Deprecation of
MeshGouraudMaterial: In r173,MeshGouraudMaterialwas deprecated in favor ofMeshLambertMaterial. (github.com)Renaming of
TextureNode.uv()toTextureNode.sample(): In r172, the methodTextureNode.uv()was renamed toTextureNode.sample(). (github.com)For a comprehensive list of breaking changes and migration guidance, refer to the Three.js Migration Guide. (github.com)
Citations:
- 1: https://classic.yarnpkg.com/en/package/three?utm_source=openai
- 2: https://discourse.threejs.org/t/three-geometry-will-be-removed-from-core-with-r125/22401?utm_source=openai
- 3: https://github.com/mrdoob/three.js/wiki/Migration-Guide?utm_source=openai
- 4: https://github.com/mrdoob/three.js/wiki/Migration-Guide?utm_source=openai
- 5: https://github.com/mrdoob/three.js/wiki/Migration-Guide?utm_source=openai
- 6: https://github.com/mrdoob/three.js/wiki/Migration-Guide?utm_source=openai
- 7: https://github.com/mrdoob/three.js/wiki/Migration-Guide?utm_source=openai
- 8: https://github.com/mrdoob/three.js/wiki/Migration-Guide?utm_source=openai
Verify Three.js version 0.176.0 on npm and migration impact
- npm confirms
three@0.176.0exists; the very latest is0.177.0.- Major breaking changes since r97 include:
• Removal ofGeometry(r125) – migrate toBufferGeometry
• Removal ofbuild/three.jsfiles (r160) – use ES Modules
• Deprecations/renames in r172–r175 (ParametricGeometries→ParametricFunctions,AnimationClip.parseAnimation(),MeshGouraudMaterial, etc.)- A quick grep of
src/js/model.jsshows only core classes (WebGLRenderer,Scene,Object3D,PerspectiveCamera), none of which have been removed or renamed.Recommendation: proceed with the upgrade, run your full test suite, and review the Three.js Migration Guide for any other affected files.
src/js/model.js.bak (4)
3-5: Legacy Three.js components referenced.This backup file references deprecated Three.js components (
ProjectorandCanvasRenderer) that have been removed from modern Three.js versions. This validates the need for the upgrade to r176 and the migration to WebGLRenderer-only rendering.
69-71: Temporary workaround reveals technical debt.The comment indicates this is a "temporary workaround" that appears to have become permanent in the legacy code. This pattern of unresolved temporary fixes is a good example of why the migration to a cleaner GLTF-based implementation is beneficial.
111-124: Deprecated JSONLoader usage confirms need for migration.This method uses
THREE.JSONLoaderwhich has been deprecated and removed from modern Three.js versions. The GLTF migration in the main PR addresses this by switching toGLTFLoader.Note the commented code on line 119 that suggests incomplete car model handling - another indication of the limitations in the legacy system.
139-204: Complex Canvas renderer optimizations highlight benefits of WebGL-only approach.This 65-line optimization method demonstrates the complexity required to achieve acceptable performance with the Canvas renderer. The manual vertex merging, face filtering, and tolerance-based simplification add significant maintenance burden.
The migration to WebGL-only rendering in the new implementation eliminates this entire optimization path, resulting in cleaner, more maintainable code.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/js/model.js (1)
50-50: Fix misleading comment about antialiasing.The comment is ambiguous - it says "enable or disable" but the code enables antialiasing.
- antialias: true, // enable or disable antialiasing for performance + antialias: true, // Enable antialiasing for better visual quality
🧹 Nitpick comments (2)
src/js/model.js (2)
42-42: Fix typo in comment.- this.detailTolerance = 10; // Default value, can be modifie + this.detailTolerance = 10; // Default value, can be modified
115-119: Remove unused createModel method.This method appears to be legacy code from the previous JSON loader implementation and is no longer used.
-Model.prototype.createModel = function (geometry, materials) { - const model = new THREE.Mesh(geometry, materials); - model.scale.set(15, 15, 15); - return model; -}; -
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/model.js(4 hunks)
🔇 Additional comments (2)
src/js/model.js (2)
22-22: Model updates look good!The airplane model now correctly references the new GLTF asset, and the car model is appropriately reserved for future work.
Also applies to: 36-36
94-113: Excellent GLTF loader implementation!The method correctly handles loading, progress tracking, and errors. The naming is clear and the error handling ensures failures are properly managed.
Remove redundant Three.js imports
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/js/model.js (1)
41-41: Fix typo in comment.- this.detailTolerance = 10; // Default value, can be modifie + this.detailTolerance = 10; // Default value, can be modified
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/model.js(4 hunks)
🔇 Additional comments (1)
src/js/model.js (1)
93-112: Well-implemented GLTF loading with proper error handling!The loadGLTF method correctly handles loading progress and errors, addressing the concerns from previous reviews.
Corrected "modifie" to "modified" in various code comments.
This commit adds the 'resources/models/fallback.gltf' file, which was inadvertently excluded from a previous commit. This GLTF file is a converted version of the legacy 'fallback.json' 3D model.
This commit replaces existing 3D model assets with newly compressed versions. The recompression aims to significantly reduce file sizes without compromising visual quality, thereby improving overall application efficiency and resource loading.
…bility This commit reactivates the legacy 3D renderer, ensuring continued functionality for low-performance devices. This prevents the renderer from becoming a blocking issue for users with less powerful hardware. Additionally, this update incorporates the latest 'Projector' import from the Three.js examples library into the CanvasRenderer, improving compatibility and leveraging recent advancements
|
|
Preview URL: https://d4c92144.betaflight-configurator.pages.dev |
github-project-automation
bot
moved this from App
to Done
in Finalizing Firmware 4.6 Release
1. Overview
This PR modernises the rendering stack by upgrading Three.js from r97 (2018) to r176 (April 2025) and migrating our asset pipeline from legacy Three.js JSON (v3 Geometry) to GLTF 2.0.
The update unlocks new features, restores first-class tooling support, and improves rendering performance, file size, and visual fidelity.
2. Why this change is needed
• Security-related dependency warnings
• Contributors must rely on obsolete tools.
• No PBR support, larger file sizes.
GLTF 2.0 (with optional Draco / Meshopt compression) is now the de-facto interchange standard and resolves all of the above issues.
What’s new
resources/models/airplane.gltf– ready for immediate use in demos.resources/models/car.gltf– reserved for upcoming feature work.Summary by CodeRabbit
New Features
Bug Fixes
Chores