Should we change default behavior on Spatial.clone() to share material?

General Discussion
1

As you know calling spatial.clone() internally calls spatial.clone(true) to clone material.

I think the main reason for this was because material sharing was not working for animated models, but now that this issue is already fixed (in Use MPOs for skinning by shadowislord · Pull Request #717 · jMonkeyEngine/jmonkeyengine · GitHub) do you think we better to change it to share material by default ?

1 Like
2

if so, then sure. it might break a lot people games, because they will change material params for one spatial and will not know why others change too. trully i think “true” param should mean that material will be shared.i had not modify material params internally, but if this was working this way, then it was wrong.

also i belive it could save some of memory.

1 Like
3

A few bytes, sure… most of the big stuff in material is still shared, though. So not as much as you might think.

No, I think the main reason for this is because when you load an asset from the asset manager you expect that you can modify it without also modifying every other instance of that asset that you previously loaded.

In the case that you know you want that behavior, then you can clone the spatial with clone(false).

What is the motivation behind making this the default?

1 Like
4

Ah, did not know it. No argument then :slightly_smiling_face:

To eliminate scene graph visitor stuff to replace material in case of instancing. But anyway this was in case that my above assumption was right.

5

then it seems correct. no topic :slight_smile:

6

The AssetManager Usecase definitely prevents us from changing this behavior.
The only problem is with instancing/batching which may rely on “the same” material instead of “equal” (probably to prevent equality checking every frame) (Is this even the case?)

7

Some paths of batching used to already compare materials. I think it was partly broken, though.

Maybe it was fully broken and/or removed?

It’s been a long time since I looked but it used to for sure not just be an == check.

No need to do it every frame… but I guess callers would have to know not to mess with the materials of a batch.