Add layer, shadow and visibility range options to the Scene importer

[EMBYRDEV]

Sister PR to #77533.

Adds additional importy options for MeshInstance3Ds.

• Render Layers
• VisibilityRange
• Shadow Casting

Has been discussed with @Calinou and is a desired feature.
EDIT(lyuma): Linking to proposal: godotengine/godot-proposals#7173
EDIT(lyuma): Please write a specific proposal to cover this PR and link it here.
EDIT(EMBYRDEV): I am specifically trying to address the proposal that lyuma initially linked.

[fire]

Do you want me to backfill a godot improvement proposal or are you ok with backfilling it?

[EMBYRDEV]

Do you want me to backfill a godot improvement proposal or are you ok with backfilling it?

If you feel it's nessisary, feel free. Considering existing discussions that have taken place I feel like this is a fairly small feature that is a gap in existing functionalisty and is an obvious addition.

The proposals never seem to land on a solid consensus anyway but I do welcome discussion.

[fire]

Well if they don't get consensus maybe we shouldn't implement them?

Honestly I expect consensus though..

[Saul2022]

Honestly I expect consensus though..

For me it look like a good improvement, my possible concern could be the visibility range option as i am not sure that setting a range of visibility in an object at importer is not accurate for a scene as you typically make the scene first and then use the visibility range to optimize the performance know what the appropiate end range for that mesh.

[EMBYRDEV]

Honestly I expect consensus though..

For me it look like a good improvement, my possible concern could be the visibility range option as i am not sure that setting a range of visibility in an object at importer is not accurate for a scene as you typically make the scene first and then use the visibility range to optimize the performance know what the appropiate end range for that mesh.

However if you're importing small meshes like cups or a stack of papers, you know they will fade out at an approx distance no matter what scene they are in so why duplicate the effort every time?

[Saul2022]

However if you're importing small meshes like cups or a stack of papers, you know they will fade out at an approx distance no matter what scene they are in so why duplicate the effort every time?

That true, tho.

[EMBYRDEV]

Is there anything else needing done before looking at getting this merged?

[Calinou]

Tested locally (rebased on top of master 851bc64), it works as expected.

Testing project: test_pr_78803.zip

[clayjohn]

The feature seems fine to me, but this needs comment from @YuriSizov and @fire who were opposed to merging this when we discussed it earlier.

[YuriSizov]

I am not opposed to having this included. I am concerned if this won't complicate things in the importer UI and I have been uncertain if it is required by many users to justify such a complication if we assume it to be true.

But ultimately, this was never blocked by me, I was merely offering my counsel.

Edit: And my other point was that this may be desired but could be exposed in a less intimidating way perhaps. But discussing that requires more feedback from other potential users.

[lyuma]

I am concerned if this won't complicate things in the importer UI and I have been uncertain if it is required by many users to justify such a complication if we assume it to be true.

Yeah, at some point, we're going to have to decide how we want the workflow: If we're allowing you to effectively edit the imported scene, in which case, why stop with layers and culling? What about blend shapes? Overlay materials? Transform edits?

I think things like this may be better served by a workflow of inheriting the scene to make more customizations. And probably we should have a proper feature proposal (godot-proposals) so we can understand more about the specific workflow being addressed by these types of additional properties.

I do want to see some of these properties being preserved from ImporterMeshInstance3D to MeshInstance3D, in case a custom importer needs to modify them (such as layers, shadows, culling, as well as perhaps meta properties and blend shape values). In my VRM importer, I've already run into issues with layers and meta properties being lost, and I had to write an addon script as workaround:

Here's how an addon could already implement this type of property in 4.0. You would write an EditorScenePostImportPlugin to present the options UI, and use the replacing_by to migrate the properties into MeshInstance3D. This code is from my godot-vrm addon to show how to use replacing_by:

@tool
extends ImporterMeshInstance3D

@export var layers: int
@export var first_person_flag: String


func _on_replacing_by(p_node: Node):
	if not (p_node is MeshInstance3D):
		push_error("ImporterMeshInstance3D was not replaced with MeshInstance3D")
	var mi: MeshInstance3D = p_node as MeshInstance3D
	mi.layers = layers
	mi.set_meta("vrm_first_person_flag", first_person_flag)


func _init():
	self.replacing_by.connect(_on_replacing_by)

[EMBYRDEV]

What's the consensus on this? I personally think this makes sense, fits with the other existing options and is something that needs to be exposed to users. Would love for this to make it into 4.2.

[EMBYRDEV]

We're approaching beta stage, is there anything holding this back from being merged?

[QbieShay]

To me this looks like an issue of not having a clear, holistic design of how the asset workflow should be.
From what i gather, and I may be wrong, Godot 4's direction was to add more information and options into the import dialog, and for me this work is aligned with the direction that Godot4 took on asset import.

If the options are too many, this can be solved by folding the UI parts by default, I think.

[YuriSizov]

We're approaching beta stage, is there anything holding this back from being merged?

Lack of consensus is holding it back. Stakeholders and other maintainers expressed their concerns, and as of now these concerns haven't been addressed (e.g. Lyuma's latest comment just above yours). Maybe these concerns should be ignored, but nobody is taking this responsibility upon themselves. So we're in a stalemate, and usually when we're in a stalemate we prefer not to merge things and preserve the status quo. So for now this is in limbo.

[Calinou]

I think things like this may be better served by a workflow of inheriting the scene to make more customizations.

Inherited scenes were the preferred workflow in Godot 3.x for this kind of post-import scene customization, but it always felt clunky and had a lot of bugs (such as not reloading when they should). There was that moment when you needed to create an inherited scene and it felt frustrating whenever I reached that point (also because it duplicates some data, making the project files larger on VCS).

I consider the Godot 4.x approach of the advanced import settings dialog far more robust and easier to use. This is why I support this PR, as it reduces the need for import scripts and inherited scenes.

Regarding why this needs to be set in the game engine at import: There is no standard way to set those in glTF, and the values you use for HLOD distance cutoffs (for instance) will vary depending on how the model is used. Not all projects will use the same asset with the same purpose. For instance, two different projects may choose to use the same asset with a completely different camera perspective, requiring different HLOD distance cutoffs for each.

[TokageItLab]

As Lyuma said, there are other values that should be set for MeshInstance, and I am concerned that including them all in the import options could complicate the UI.

I think it might be better to allow users to add presets such as "Item", "Obstacle", " Character" and etc. for that purpose in the EditorSetting or anywhare, but in any case I think a more detailed proposal is needed.

[QbieShay]

I think if we want to support presets in the future, we can just have presets and a custom option, and auto-mark everything before the version that supports it as "custom". There's way to keep it backwards compatible.

And i second what calinou said about the inherit scene workflow, it breaks at scale.

[lyuma]

I'm seeing a lot of people supporting this type of change, so on that basis I think it would be good to make sure this or something like it lands in 4.2 -- we haven't forgotten.

The real problem I'm having is my own fault. I mistakenly edited the original message to link to a proposal from @fire, but the more I think about it, the proposal I linked to is more general and covers the concept of improving the scene inheritance workflow, which I'm in favor of, but it is larger in scope than this PR and does not discuss the specific needs of all the people coming out of the woodwork to comment in favor of this PR.

What would be really helpful, would be if anyone interested in this PR (especially the OP @EMBYRDEV ) could make a specific proposal for this PR. Specifically, why these properties. Are these three properties (layers, shadows, visibility range) sufficient for everyone in this thread? Do you have examples that show why these properties in particular are important (what is the application being addressed here)? Are there any other properties that need to be exposed?

My (and Calinou's) assumption here is that these are required for HLOD. However, the OP and others did not mention HLOD. If it is HLOD support we are looking for, it would be helpful to know that so we can think about the problem from that perspective and see if any other things should be addressed.

If you can answer these questions, it would help explain why this should be a feature in and of itself rather than a more general improvement to the inherited scenes workflow.

Also, a side note, but we are likely going to see physics extensions for glTF which do this on physics objects, for example. Since physics objects also have layers, do we also need to add those?

Anyway this is the reason that I've been struggling to review this PR. I'm missing some information on what specific problem we're solving here, as well as what remains unsolved and what users actually need.

Also, feel free to reach out to me or chat with me in RocketChat in the #asset-pipeline channel.

[EMBYRDEV]

Since physics objects also have layers, do we also need to add those?

#77533 implemented these.

I feel like the support of this PR is pretty unanimous and the use cases fairly obvious? HLOD is one of many potential reasons to implement these properties and I implemented them before I ran into a specific use case due to the ergonomic issues I was having importing assets into this engine vs others such as unreal.

The goal was to reduce the need for these inherited .tscn files, not to address one particular use case. So I believe that Fire's proposal is actually what this is helping to address.

With that in mind I'd love to hear if people have suggestions for any other properties they find themselves needing to change for a vast majority of meshes that they import?

[QbieShay]

I havent used 4.x pipeline extensively yet but shadow casting and layers are definitely very useful for games that use special render layers for making a distinction between different objects, for example have characters also render in a separate viewport used for effects, or on the contrary force vfx meshes to never cast a shadow.

[lyuma]

Thanks QB for explaining some of the VFX explanations for these import settings. That combined with the HLOD seems more than enough justification for adding these specific settings.

The PR itself is perfect from a technical level: I like that the property names and defaults match throughout.

I do wish we could add extra category headers somehow for the visibility ranges such as HLOD, but doing so would make the property names not match up (I really wish the inspector could be made decoupled from the property names, but that's not something we can change really right now).

We're going to make sure this lands in 4.2. I appreciate your patience as I tried to figure out the extent to which we need these properties exposed

[lyuma]

(Sorry for duplicate comment. Had a network glitch.)

[akien-mga]

Thanks for the contribution, and for your patience with the review process :)