HungryProton / scatter

Godot engine addon to randomly fill an area with props or other scenes
MIT License
1.95k stars 87 forks source link

Ignored parameter "mesh_instance/visibility_range_begin" on asset #201

Open why-try313 opened 3 weeks ago

why-try313 commented 3 weeks ago

The problem:

As said on the title, ScatterShape ignores the parameters [mesh_instance].visibility_range_begin, [mesh_instance].visibility_range_end and margins set on the model , which makes the assets with low poly versions into one merged mesh.

Note that I did search for this setting on the menu, it's either not present or hidden under some menu


A visual representation

In this example, there's 2 models, a LOD set on a .gltf file and distances set via the integrated godot importer, and a second model without low levels meshes. When the LOD object is set to ScatterShape, all meshes are displayed scatter

Solutions

I dond't know if the code implies a remesh of the model or a small bug but if it's not a bug, a simple "Ignore remesh" boolean button could be added to the ScatterShape menu, set false by default, to either use the addon method or the godot method

HungryProton commented 3 weeks ago

Try to set the visibility properties on the ScatterItem node (not the main scatter one, or the shapes, the item node). It's under the visibility sub menu, in the inspector. You can disable the auto LOD generation in the Level of Detail submenu. If you still have LOD issues, then it's a bug I still need to fix.

why-try313 commented 3 weeks ago

First of, thanks for the fast reply ! To answer, been there, done that:

I tweaked some of my models to use script on import (and without the script it's the same result, so it's not the script) and I know that the importer script returns a scene, maybe using the result of the imported scene instead of the model itself could be a quick way to fix this?

If it can help, a sample of a script used on the importer:

@tool # Needed so it runs in editor.
extends EditorScenePostImport
func _post_import(scene): # called by the importer
    do_something(scene)
    return scene # the function expects to return the modified scene
why-try313 commented 3 weeks ago

@HungryProton my bad! The functionality is integrated, if [ProtonScatter]render_mode is set to Create copies the addon works as expected.

image

I guess the problem was with the cached obects (?). After digging into the code get_merged_meshes_from did mush all the meshes into one since it had less than 8 surface # Less than 8 surfaces, merge in a single MeshInstance but setting it to Create copies still used the cached mushed version, clearing all out and setting the render_mode before setting the object uses the default godot loader.

I guess the bug is more about cache than logic, i'd suggest to test and close if the problem is not confirmed, might be a specific case unrelated to the addon

HungryProton commented 3 weeks ago

It's a bit complex:

By default, scatter uses instancing to speed up the rendering.

Instancing uses a MultiMeshInstance3D node under the hood, however it handles materials differently:

So when using instancing with a mesh with surface material overrides, Scatter has to create a new mesh and set the materials to the mesh surface instead (I can't reuse the original mesh here or it might mess up the materials for another scene using the same mesh).

Recreating that mesh makes it lose its custom LOD, and that's something I need to investigate.

why-try313 commented 3 weeks ago

I'd say it works "as intended" The logic goes:

    [Scatter] _update_multimeshes():
        [Util] get_or_create_multimesh(item)
            [Utils] get_or_create_multimesh(item)
                mesh = ImporterMesh.new()
                -> mesh.add_surface(PRIMITIVE_TRIANGLES, item[suface].mesh)
                # Here, item still has the visibility paramaters, but the
                # importer loses it when surfaces are ingested which is normal since
                # the vertex are passed to `Mesh.PRIMITIVE_TRIANGLES`
            returns a Mutlmesh with lost visibility settings

Using ImporterMeshInstance3D instead of ImporterMesh could be an option since ImporterMeshInstance3D has the visibility options, but I would imply having separated meshes anyway to have that visibility option set per mesh, which doesn't work for optimization

why-try313 commented 3 weeks ago

Now that I think about it, grouping items by visbility option just as the script merges by materials could be an option if visibility options are set

    var meshes_per_range = {}
    for mesh in surfaces: # Note that this loop already exists in scatter_util.gd:322
        var range_name = "_"+string(mesh.visibility_range_begin)
        if !meshes_per_range.has(range_name):
            meshes_per_range[ range_name ] = []
        meshes_per_range[ range_name ].push_back(mesh)

    var ranges_keys = meshes_per_range.keys()
    var ranges_len  = len(ranges_keys)
    if ranges_len > 1:
        for index in range(ranges_len):
            var new_mesh = ImporterMeshInstance3D.new()
            var current_range = int(ranges_keys[ index ])
            # mush merge here
            for mesh in meshes_per_range[ ranges_keys[ index ] ]:
                mesh.add_surface(PRIMITIVE_TRIANGLES, mesh)
                mesh.visibility_range_begin = current_range
                if index < ranges_len - 1: # if has next range, set as end of current range
                    mesh.visibility_range_end = int(ranges_keys[ index + 1 ])

Something like that? It processes the meshes if the visibility options are set and avoids useless processing if it only has one range (visibility_range_begin = 0)