Implemented model material/texture import and reuse on model/scene load

[Gopmyc]

Description

This PR implements automatic material/texture import and reuse for models, both when adding a model to the scene and when loading a .ovscene.

Main changes:

• Added model material import pipeline in editor actions (Assimp-based).
• Generated assets are stored in per-model folders:
  • Assets/Materials/<ModelName>/
  • Assets/Textures/<ModelName>/
• Reuse existing generated materials/textures instead of regenerating when already present.
• Applied the same generation/reuse logic when loading a scene from disk (.ovscene).
• Refreshed Asset Browser automatically when new assets are generated.
• Synced regenerated materials across all scene actors using the same model (avoids stale pink/error materials).
• Fixed picking pass skinning detection to use drawable skinning descriptors (compatible with IMesh usage).

Related Issue(s)

Fixes #303

Review Guidance

• Drag and drop a model (.fbx, etc.) into the scene:
  • Actor should be created with generated/assigned materials.
  • Materials/textures should be generated under model-specific subfolders.
• Re-import the same model:
  • Existing generated assets should be reused.
• Delete generated materials, then re-add/reload the model:
  • Materials should be regenerated and reassigned.
  • Other actors already using the same model should update as well.
• Load a .ovscene containing models:
  • Missing materials/textures should be generated/reused and assigned automatically.
• Verify picking still works correctly on skinned meshes after the picking-pass adjustment.

Screenshots/GIFs

Checklist

• My code follows the project's code style guidelines
• I have commented my code, particularly in hard-to-understand areas
• I have updated the documentation accordingly
• My changes don't generate new warnings or errors

The reason for the defensive handling is based on Assimp’s own texture semantic docs:

• Assimp explicitly states that mapping texture usages to aiTextureType is a best-effort process across DCC/tools/importers:
  https://github.com/assimp/assimp/blob/master/include/assimp/material.h#L174-L181
• aiTextureType_HEIGHT and aiTextureType_NORMALS are separate semantics in the API, but importers/exporters can still produce ambiguous mappings in practice:
  https://github.com/assimp/assimp/blob/master/include/assimp/material.h#L232-L240

So in this PR I handle it defensively:

• parse normal + height/displacement candidates,
• if height resolves to the same texture as normal (or acts as fallback normal), do not enable parallax,
• only enable parallax when a distinct height/displacement map is actually present._

[adriengivry]

Thanks for submitting this PR!

I can see a few major issues with this work:

1. Assimp is being used directly in OvEditor. Implementation details related to model loading should remain hidden from the application behind the IModelParser interface. Additional model data (i.e., material and texture information) should be fully handled by the AssimpParser.

2. Project file pollution. This PR assumes that the user wants to place automatically generated materials and textures in Materials/{model_name}/ and Models/{model_name}/, respectively.

3. Imported materials and models collision. In some cases, if the user has multiple models with the same name in different folders, only the first imported model will properly generate materials and textures.

Ultimately, my vision for this feature is to offer something similar to what Unity does. I see models (FBX, OBJ, etc.) as “folders” that can be expanded to reveal all their embedded assets, including materials and potentially textures, if any.

These embedded assets would be non-modifiable, as they would be generated when the model is loaded. They could then be dragged and dropped anywhere a material or texture is required. Paths for embedded assets could look like:

path/to/model.fbx:embedded_material.ovmat
path/to/model.fbx:embedded_texture.jpeg

In the model asset properties, we could add a toggle (above the model parsing flags) to determine whether embedded materials and textures should be generated.

This would also require updating the PropagateFileRename implementation to properly handle embedded assets.

Once this is fully implemented, I believe the “Generate materials” option in the model contextual menu could be safely removed.

The final step would be to add support for externally linked textures. Some models reference textures using relative paths, and it would be beneficial to support these as well. Generated materials should be able to reference linked textures correctly.

Given the scope of this work, I understand if it’s not manageable for you at the moment. We’ve postponed this effort for many years because it’s fairly complex and has a lot of dependencies.

[Gopmyc]

Challenge accepted

[Gopmyc]

Thanks for submitting this PR!

I can see a few major issues with this work:

1. Assimp is being used directly in OvEditor. Implementation details related to model loading should remain hidden from the application behind the IModelParser interface. Additional model data (i.e., material and texture information) should be fully handled by the AssimpParser.
2. Project file pollution. This PR assumes that the user wants to place automatically generated materials and textures in Materials/{model_name}/ and Models/{model_name}/, respectively.
3. Imported materials and models collision. In some cases, if the user has multiple models with the same name in different folders, only the first imported model will properly generate materials and textures.

Ultimately, my vision for this feature is to offer something similar to what Unity does. I see models (FBX, OBJ, etc.) as “folders” that can be expanded to reveal all their embedded assets, including materials and potentially textures, if any.

These embedded assets would be non-modifiable, as they would be generated when the model is loaded. They could then be dragged and dropped anywhere a `material` or `texture` is required. Paths for embedded assets could look like:

path/to/model.fbx:embedded_material.ovmat
path/to/model.fbx:embedded_texture.jpeg

In the model asset properties, we could add a toggle (above the model parsing flags) to determine whether embedded materials and textures should be generated.

This would also require updating the PropagateFileRename implementation to properly handle embedded assets.

Once this is fully implemented, I believe the “Generate materials” option in the model contextual menu could be safely removed.

The final step would be to add support for externally linked textures. Some models reference textures using relative paths, and it would be beneficial to support these as well. Generated materials should be able to reference linked textures correctly.

Given the scope of this work, I understand if it’s not manageable for you at the moment. We’ve postponed this effort for many years because it’s fairly complex and has a lot of dependencies.

I understand the direction and I agree with the overall idea, especially the Unity-like approach with embedded assets and keeping things clean through the parser.

That said, I don’t think it makes sense to include material generation and the embedded asset view in this PR. The scope gets pretty large and touches a lot of different parts of the engine (asset system, file handling, UI, parser responsibilities, etc.). To keep things clear and easier to review, I think it’s better to split this into multiple smaller PRs.

For now, I’ll focus on loading materials virtually through the parser and assigning them to the models at runtime. This should already improve rendering quality and material support, without polluting the project’s asset folders or introducing persistence/UI concerns too early.

Once that part is solid, we can move forward with separate PRs for embedded asset handling, optional generation, rename propagation, and support for external textures.

I’m going to reopen this PR and continue working on it with this scope.

[adriengivry]

@Gopmyc

Yep, this is a pretty massive change. Totally agree about breaking it down.
Could you explain more in detail what you mean by:

For now, I’ll focus on loading materials virtually through the parser and assigning them to the models at runtime.

[Gopmyc]

@Gopmyc

Yep, this is a pretty massive change. Totally agree about breaking it down. Could you explain more in detail what you mean by:

For now, I’ll focus on loading materials virtually through the parser and assigning them to the models at runtime.

Great question.

By “loading materials virtually through the parser”, I mean:

• no .ovmat / texture files are generated on disk in Assets/;
• AssimpParser extracts embedded material/texture data and stores it in the Model resource in memory;
• these are exposed through virtual paths like model.fbx:embedded_material_0.ovmat and model.fbx:embedded_texture_0.png.

Then, “assigning them at runtime” means MaterialManager / TextureManager resolve those virtual paths on demand, and renderer components assign them automatically when a model is loaded (or scene deserialized), especially for empty/default slots.

[adriengivry]

I didn't look too deep into the implementation that much yet, but I tested it locally, and here is my feedback:

Issues/bugs:

• GENERATE_EMBEDDED_ASSETS doesn't seem to work. Even when I set it to false the materials are still being loaded.
• Embedded textures don't seem to work (e.g. with this ShaderBall.fbx, loading it into Blender or Unity, you'll see a grid pattern on the ball, but you won't in Overload)
• Reloading linked (external) textures doesn't apply changes, but reloading the model after modifying a linked texture does work. This means that linked textures are actually duplicated in GPU memory (loaded twice). You can reproduce that issue with Sponza FBX with linked textures. If you import all the assets into a project, and load the FBX, and then modify a texture and reload it (right click on the texture > reload), you'll notice that the texture isn't updated on the model. To reload it you would have to reload the model instead.
• Parallax mapping seem to be enabled even when no height map is used. This can be reproduced by loading Sponza FBX with linked textures and looking at textures from close, from an angle.
• We should also keep the "Generate Materials" for now, as it's still useful to change the appearance of a model. I know I mentioned we should remove it at some point, but I think it still serves a valid purpose for now.

Notes:

• Linked textures seem to work (e.g. Sponza FBX with linked textures, loading the FBX will properly find the textures using the relative path. Since you mentioned this, I thought this was gonna be in another PR, but I'm ok with external textures being supported in this PR as well:

  Once that part is solid, we can move forward with separate PRs for embedded asset handling, optional generation, rename propagation, and support for external textures.

Overall this seem feature complete (once it will be fully fixed). I like the scope of the PR, which I would summarize as:

• Added support to load materials embedded into a model
• Added support to load textures embedded into a model and automatically attaching them to the correct embedded materials
• Added support to attach externally linked textures to embedded materials

[Gopmyc]

I didn't look too deep into the implementation that much yet, but I tested it locally, and here is my feedback:

Issues/bugs:

• GENERATE_EMBEDDED_ASSETS doesn't seem to work. Even when I set it to false the materials are still being loaded.
• Embedded textures don't seem to work (e.g. with this ShaderBall.fbx, loading it into Blender or Unity, you'll see a grid pattern on the ball, but you won't in Overload)
• Reloading linked (external) textures doesn't apply changes, but reloading the model after modifying a linked texture does work. This means that linked textures are actually duplicated in GPU memory (loaded twice). You can reproduce that issue with Sponza FBX with linked textures. If you import all the assets into a project, and load the FBX, and then modify a texture and reload it (right click on the texture > reload), you'll notice that the texture isn't updated on the model. To reload it you would have to reload the model instead.
• Parallax mapping seem to be enabled even when no height map is used. This can be reproduced by loading Sponza FBX with linked textures and looking at textures from close, from an angle.
• We should also keep the "Generate Materials" for now, as it's still useful to change the appearance of a model. I know I mentioned we should remove it at some point, but I think it still serves a valid purpose for now.

Notes:

• Linked textures seem to work (e.g. Sponza FBX with linked textures, loading the FBX will properly find the textures using the relative path. Since you mentioned this, I thought this was gonna be in another PR, but I'm ok with external textures being supported in this PR as well:

  Once that part is solid, we can move forward with separate PRs for embedded asset handling, optional generation, rename propagation, and support for external textures.

Overall this seem feature complete (once it will be fully fixed). I like the scope of the PR, which I would summarize as:

• Added support to load materials embedded into a model
• Added support to load textures embedded into a model and automatically attaching them to the correct embedded materials
• Added support to attach externally linked textures to embedded materials

Thanks for the detailed review, this was very helpful.

I addressed each issue you listed:

• GENERATE_EMBEDDED_ASSETS now behaves correctly: when disabled, embedded materials are no longer applied (fallback is used instead of stale embedded refs).
• Embedded textures are now properly resolved, including named embedded texture references (not only *0, *1, etc.).
• Linked external textures are now bound through their real shared texture resource path, so reloading a linked texture updates model materials correctly and avoids duplicate GPU loads.
• PARALLAX_MAPPING is now enabled only when a valid height map is actually bound.
• I kept the "Generate materials..." model context-menu entry.

I also re-validated with ShaderBall/Sponza-style cases, and the PR scope is now aligned with your summary:

• embedded materials loading
• embedded textures loading and auto-assignment
• externally linked textures attached to embedded materials

[adriengivry]

Thanks for updating the PR! It looks really good, and seem to work in most cases!

One issue I found is with externally linked textures (e.g. Sponza FBX with linked textures) external textures are now properly being used (yay! 🎉) but parallax mapping still look like it's enabled even though no height map is provided.

[Gopmyc]

Thanks for updating the PR! It looks really good, and seem to work in most cases!

One issue I found is with externally linked textures (e.g. Sponza FBX with linked textures) external textures are now properly being used (yay! 🎉) but parallax mapping still look like it's enabled even though no height map is provided.

Thanks for the feedback !

I just pushed a fix for this point, and it now seems corrected on my side :
parallax is no longer enabled for externally linked textures when no real height map is provided (Sponza case).

I adjusted the material extraction/runtime binding logic to avoid treating height/bump data as parallax input in this scenario.