Add Assimp GLTF/GLB/VRM support back in by tccountus · Pull Request #830 · Overload-Technologies/Overload

tccountus · 2026-08-28T23:16:33Z

Description

This PR adds GLTF/GLB/VRM support back into the engine

The VRM format is a gimmic by Assimp and works without that much extra code, so why not

Related Issue(s)

None

Review Guidance

Only tested on Debian Trixie

Screenshots/GIFs

Klee model: https://hub.vroid.com/en/characters/7993882131090656823/models/2173753627961982484

AI Usage Disclosure

RAPIDJSON_HAS_STDSTRING told me chatgpt, otherwise no AI usage

Checklist

My code follows the project's code style guidelines
When applicable, I have commented my code, particularly in hard-to-understand areas
When applicable, I have updated the documentation accordingly
My changes don't generate new warnings or errors
I have reviewed and take responsibility for all code in this PR (including any AI-assisted contributions)

adriengivry

LGTM! I'm just wondering how embedded materials metallic, roughness, albedo, emissive etc is being interpreted when a model is (for instance) authored in Blender, exported as GLTF, and imported in Overload. I know that different formats store this information differently, and Assimp isn't always the best at resolving those values. (see https://github.com/Overload-Technologies/Overload/blob/main/Sources/OvRendering/src/OvRendering/Resources/Parsers/AssimpParser.cpp).
Approved conditionally that these behaviors work as expected.

Gopmyc · 2026-09-01T17:29:18Z

@@ -77,7 +86,7 @@ project 'assimp'
 		'ASSIMP_BUILD_NO_CSM_IMPORTER',
 		'ASSIMP_BUILD_NO_DXF_IMPORTER',
 		-- 'ASSIMP_BUILD_NO_FBX_IMPORTER',
-		'ASSIMP_BUILD_NO_GLTF_IMPORTER',
+		-- 'ASSIMP_BUILD_NO_GLTF_IMPORTER',


Commenting out ASSIMP_BUILD_NO_GLTF_IMPORTER enables both glTF 1.0 and glTF 2.0, since the importers are guarded by !ASSIMP_BUILD_NO_GLTF_IMPORTER && !ASSIMP_BUILD_NO_GLTF{1,2}_IMPORTER. glTF 1.0 has been superseded since 2017 and nothing exports it anymore, so this pulls in ~5k lines of vendored code (code/AssetLib/glTF/**) and an extra compiled importer for a format we don't need

It also costs us on every glTF import: glTFImporter::CanRead ignores the extension and fully loads and JSON-parses the file before returning false, and glTF1 is registered before glTF2 in ImporterRegistry.cpp

Could we add ASSIMP_BUILD_NO_GLTF1_IMPORTER to the defines and drop code/AssetLib/glTF/** from both the files block and the vendored tree ? glTF2 has no dependency on glTF1, and the only reference is ImporterRegistry.cpp:188, which is already behind that guard

Gopmyc · 2026-09-01T17:35:21Z

@@ -1506,7 +1506,7 @@ bool OvEditor::Core::EditorActions::ImportAsset(const std::string& p_initialDest
 {
 	using namespace OvWindowing::Dialogs;

-	std::string modelFormats = "*.fbx;*.obj;";
+	std::string modelFormats = "*.fbx;*.obj;*.glb;*.gltf;*.vrm;";


Since .gltf is now advertised here : a non-binary .gltf references an external .bin (and often external images), but ImportAsset / ImportAssetAtLocation copy a single file (L1544 and L1618). After importing one through this dialog, ReadFile fails on the missing buffer, LoadModel returns false and ModelLoader::Create returns nullptr with no log, so the asset shows up in the Asset Browser and silently never loads

.glb and .vrm are self-contained and unaffected. Worth copying the files referenced by the .gltf, or at least logging the load failure so the user knows what happened ?

How are we handling that with FBX/OBJ? They can also reference external files

We don't, it's the same single-file copy for all three

The difference is what breaks. For OBJ/FBX the external references are materials and textures, so a missing .mtl just logs an error and the parser carries on (ObjFileParser::getMaterialLib), you still get the mesh. For a text .gltf the .bin holds the geometry itself, so Buffer::Read throws (glTF2Asset.inl:619) and the whole import fails

What bothers me more is that nothing surfaces it : LoadModel returns false, ModelLoader::Create returns nullptr, and the asset just sits in the browser doing nothing. Maybe logging Importer::GetErrorString() there would be enough ? Would help FBX/OBJ too.

oops, never thought about that. Maybe we should ask the user to select the dot bin file or deny adding gltfs using this way completely?

Gopmyc

Built it in Visual Studio on Windows 11, works fine. Integration looks clean overall.

Left two comments inline. One extra thing : glTF packs metallic and roughness into a single texture, but Standard.ovfx samples .r from both maps, while glTF stores roughness in G and metallic in B. Anything with a metallicRoughness map will come out wrong. Worth fixing here, or follow-up ?

adriengivry · 2026-09-01T18:22:55Z

@Gopmyc worth fixing IMO

tccountus · 2026-09-01T19:15:49Z

okay, should we edit the shader or convert the texture? I think converting might be the better way

adriengivry · 2026-09-01T20:29:51Z

okay, should we edit the shader or convert the texture? I think converting might be the better way

If the texture is embedded, and not user-driven (not available through the project files), then we can do whatever we want with it, including converting it. I agree that converting is better than modifying the shaders.

tccountus · 2026-09-02T16:46:45Z

i'm not sure if this fixes it, do you have a test model?

…n file.

Gopmyc · 2026-09-17T17:03:22Z

Khronos has good ones for this : MetalRoughSpheres (metallic on one axis, roughness on the other, so any channel mixup shows right away), plus CompareMetallic and CompareRoughness. All of them have a glTF-Binary version in KhronosGroup/glTF-Sample-Assets, with a reference screenshot to compare against

Had a look at 996a230 and I don't think it kicks in on a .glb yet. assimp keeps GLB images encoded (mHeight == 0, glTF2Importer.cpp:1654), so they go through the EMBEDDED_COMPRESSED branch and the channel id is never used. They only get decoded later in TextureManager.cpp

Also, the cache key in RegisterEmbeddedTexture doesn't include the channel. Metallic registers the texture first with B, then roughness asks for the same path, hits the cache and gets B back. Same for occlusion when it's packed in the same image

Once those are fixed, MetalRoughSpheres should confirm it quickly. A Blender export would be worth a check too, since that's the case Adrien asked about

adriengivry · 2026-09-18T17:05:45Z

@@ -125,7 +125,7 @@ OvTools::Utils::PathParser::EFileType OvTools::Utils::PathParser::GetFileType(co
 	std::string ext = GetExtension(p_path);
 	std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower);

-	if (ext == "fbx" || ext == "obj") return EFileType::MODEL;
+	if (ext == "fbx" || ext == "obj" || ext == "glb" || ext == "gltf" || ext == "vrm") return EFileType::MODEL;


Is gltf expected to remain here? or should it be removed?

yes, we can read it, but we cannot import it

tccountus added 2 commits August 28, 2026 23:06

Add Assimp GLTF support back in

7a45484

Fix Model selection

f20b201

adriengivry added the Graphics Graphical feature label Sep 1, 2026

adriengivry approved these changes Sep 1, 2026

View reviewed changes

Gopmyc reviewed Sep 1, 2026

View reviewed changes

Try to fix GLTF roughness textures

996a230

Remove gltf from importer because gltf files reference an external bi…

a5518d8

…n file.

adriengivry reviewed Sep 18, 2026

View reviewed changes

adriengivry merged commit d013cf0 into Overload-Technologies:main Sep 20, 2026
6 checks passed

Conversation

tccountus commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Review Guidance

Screenshots/GIFs

AI Usage Disclosure

Checklist

Uh oh!

adriengivry left a comment

Choose a reason for hiding this comment

Uh oh!

Gopmyc Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gopmyc Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

adriengivry Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Gopmyc Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

tccountus Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Gopmyc left a comment

Choose a reason for hiding this comment

Uh oh!

adriengivry commented Sep 1, 2026

Uh oh!

tccountus commented Sep 1, 2026

Uh oh!

adriengivry commented Sep 1, 2026

Uh oh!

tccountus commented Sep 2, 2026

Uh oh!

Gopmyc commented Sep 17, 2026

Uh oh!

adriengivry Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

tccountus Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

tccountus commented Aug 28, 2026 •

edited

Loading

Gopmyc Sep 1, 2026 •

edited

Loading