Handle material for instances

[pjoe]

When using mesh instances with different materials, these are stored in the object - not the mesh.

I've attached a simple file (2.80) showing the issue:

instancing.zip

[emackey]

At first glance this seems good. I didn't test it on my machine yet, will try to do that tomorrow. Might be worth adding a test to the suite to make sure that materials come out the intended way.

[pjoe]

Hmmm it fails roundtrip test 02_shared_mesh. I think it's because the cache_wrapper now sees meshes as different because of the different blender_object param. I originally tried passing down the material list, but had issues that the cache_wrapper couldn't handle list args 🤔

[pjoe]

Updated this to do caching correctly. Also properly handle meshes without material assigned.

Now output will be something like (reusing same attribute accessors for same mesh data):

{
    "meshes" : [
        {
            "name" : "CubeMESH",
            "primitives" : [
                {
                    "attributes" : {
                        "POSITION" : 0,
                        "NORMAL" : 1,
                        "TEXCOORD_0" : 2
                    },
                    "indices" : 3,
                    "material" : 0
                }
            ]
        },
        {
            "name" : "CubeMESH",
            "primitives" : [
                {
                    "attributes" : {
                        "POSITION" : 0,
                        "NORMAL" : 1,
                        "TEXCOORD_0" : 2
                    },
                    "indices" : 3,
                    "material" : 1
                }
            ]
        }
    ]
}

NOTE: from debugging I noticed that cache_wrapper does NOT work properly with bpy.types objects, which is the reason I had to change several places from passing around blender_mesh: bpy.types.Mesh to blender_mesh_name: str. This issue with cache_wrapper is likely to cause problems elsewhere :S

[emackey]

So I have to ask, how far do we want to go to preserve the 2.79 per-mesh double-sided feature, given that the flag was dropped from 2.80? Could we revert back to just using the custom node's double-sided setting in 2.79, which is a material setting, and that would save us some mesh-vs-material gymnastics that are really only here for the sake of 2.79?

[pjoe]

Managed to fix an issue where things broke if the mesh had no materials (i.e. only object instances had materials).

Here is a little test with 1000 monkeys:

1000_monkeys.zip

[emackey]

This PR is cooler than it looks. There's actually no way in master right now to re-use a mesh multiple times with different materials (aka "instancing"), and this enables that as a new export feature.

The automated tests pass and I did a little manual testing too. I'm going to merge this, and when I have time I will write a new automated test to look specifically for this feature.

Thanks @pjoe!

[julienduroure]

NOTE: from debugging I noticed that cache_wrapper does NOT work properly with bpy.types objects, which is the reason I had to change several places from passing around blender_mesh: bpy.types.Mesh to blender_mesh_name: str. This issue with cache_wrapper is likely to cause problems elsewhere :S

@pjoe This introduce a very bad regression : we can't export applying modifiers anymore ... This is already reported in several bug reports as #546 #536

I currently working on a better cache wrapper, for fixing this bug and some other cache issue with bpy.types

[pjoe]

@julienduroure Damn, sorry about that regression. Anyway obviously as you say best solution is to handle this generally in the cache_wrapper and revert back to passing actual Mesh objects instead of names.

[julienduroure]

NOTE: from debugging I noticed that cache_wrapper does NOT work properly with bpy.types objects, which is the reason I had to change several places from passing around blender_mesh: bpy.types.Mesh to blender_mesh_name: str. This issue with cache_wrapper is likely to cause problems elsewhere :S

Hello @pjoe
This is quite an old report, but I'm currently looking some test cases where cache_wrapper was not working properly with bpy.types
Do you remember what was your test case when you debugged ?

[pjoe]

@julienduroure Must admit this is a while back. IIRC the issue was later addressed in cache_wrapper. My main testing for this PR was instancing, but I think that was fully fixed.