Skip to content

Ogre importer: Fix for material sharing#477

Merged
empirephoenix merged 1 commit into
jMonkeyEngine:masterfrom
riccardobl:fix_topic35663_issue
May 18, 2016
Merged

Ogre importer: Fix for material sharing#477
empirephoenix merged 1 commit into
jMonkeyEngine:masterfrom
riccardobl:fix_topic35663_issue

Conversation

@riccardobl
Copy link
Copy Markdown
Member

Fix for the issue reported here: https://hub.jmonkeyengine.org/t/ogre-importer-materials-are-not-shared/35663

Short description of the problem:
We have an Ogre's .scene with two spatials that share the same material.
When this scene is imported in the engine, the two spatials no longer share the material but, instead, they own a new instanced of it.
This happens because the SceneLoader loads the objects through the assetManager as if they were unrelated, the assetManager calls clone(true) on the cached models before returning them, this cause the issue.

This patch loads the models directly with an instance of MeshLoader opportunely extended to have an internal cache that is preserved until the scene is fully loaded.

@riccardobl riccardobl changed the title Ogre importer: Fix material sharing Ogre importer: Fix for material sharing Apr 18, 2016
@empirephoenix
Copy link
Copy Markdown
Contributor

Looks fine to me, merging

@empirephoenix empirephoenix merged commit 2c1fd47 into jMonkeyEngine:master May 18, 2016
@normen
Copy link
Copy Markdown
Member

normen commented May 18, 2016

Whats the purpose of sharing the material? It doesn't accelerate rendering, the size of the material data is negligible because all referenced data like textures and material definitions are shared anyway and this change will cause all instances of the material to be changed when you change one (e.g. you want to tint one model blue and the other red)?

@riccardobl
Copy link
Copy Markdown
Member Author

riccardobl commented May 18, 2016

Whats the purpose of sharing the material?

Batching, instancing, everything that need shared materials to work.

and this change will cause all instances of the material to be changed when you change one (e.g. you want to tint one model blue and the other red)?

This is the expected behaviour for shared materials, if you don't want it, just clone your material like you would do in any other software.

@empirephoenix
Copy link
Copy Markdown
Contributor

I think you two have a misunderstanding.
I can on the ogre side explicitly share a material, but the current jme loader ignores it, and does not share it then. This is useful for stuff like a city with different weather based flower/tree ect textures. This basically only adds the missing feature to jme, it does not share all materials if possible, only if requested on orgre side.

@normen
Copy link
Copy Markdown
Member

normen commented May 19, 2016

Batching and instancing look for the actual parameters, so even if the material is not the same object - if the parameters are the same they will be batched.

@pspeed42
Copy link
Copy Markdown
Contributor

Yes, but the point is that if the imported file specifically has shared materials then the user probably did that for a reason and we shouldn't undo it. Anyway, MPOs sort of get rid of the last reason to leave them shared.

@riccardobl
Copy link
Copy Markdown
Member Author

riccardobl commented May 19, 2016

@pspeed42
Copy link
Copy Markdown
Contributor

pspeed42 commented May 19, 2016

In the second case, normen's comment is true... see:
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/tools/java/jme3tools/optimize/GeometryBatchFactory.java#L323

But yeah, InstancedNode is not as nice this way.

@empirephoenix
Copy link
Copy Markdown
Contributor

There are way more reasons than just batching.

For example a scene of a house, with 3 different textures(atlas) that can be changed by simply switching the diffuse in the material.
If it is shared this is way easier done for complex scenes than with separate ones.
For example something like this https://www.assetstore.unity3d.com/en/#!/content/19469 would be such a case. eg. apocalypse game with before and after doomsday parts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants