Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow lightmap size hints in import dialogue (or make default size dependent on model size) #11383

Open
michaelharmonart opened this issue Dec 19, 2024 · 6 comments

Comments

@michaelharmonart
Copy link

Describe the project you are working on

I'm creating an interactive map and timeline which will be built as a standalone VR app. Due to the performance requirements, baked lighting with shadowmasks is the current choice that will allow for the best visual quality and performance tradeoff for the map itself.

Describe the problem or limitation you are having in your project

When importing high poly meshes (such as chunks of a landscape that need to have baked lighting) the default godot auto UV is far too slow and poor quality to be usable, meaning I UV in an external software and import into godot, but disable the lightmap UV generation. The issue is that the default size of these objects on the lightmap is far too small to be useful, meaning I have to set a mesh save path and edit each mesh to change the lightmap size hint property.

Upon changes to the glb scene these changes are cleared and I have to redo the lightmap size hints for each mesh AGAIN.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

The easiest solution I see is simply allowing the lightmap size to be adjusted per mesh in the import dialogue (perhaps right under the dropdown for enabling or disabling lightmap UV generation for the mesh)

Even better would be to have the ability for Godot to automatically assign a better default lightmap size depending on the size of the mesh.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

For the first solution the implementation is as simple as adding an input for the lightmap size hint to the import settings of the mesh
Image

For the second option (having size aware default lightmap size per mesh) I think the best way would be to base it on the surface area of the mesh, or if that calculation is too slow, even just based on the AABB would be better than what it is now.

I assume the code to calculate that already exists since the lightmap UV generation is size aware.
Image

Though the texel size option only appears when the import is set to generate lightmap UVs for everything. We would want to use this value to calculate the lightmap size hint of the meshes even if you don't want auto lightmap UVs, so maybe we should add an option for "Static Lightmaps (Manual UV2)" that defaults to not generating lightmap UVs, but simply setting a lightmap size hint for each mesh based on the texel size chosen.

If this enhancement will not be used often, can it be worked around with a few lines of script?

I think it will be used extremely often. Many models already are built with non-overlapping artist authored UVs that are perfectly suitable for lightmapping, having them able to be used easily by default should be extremely helpful.

Is there a reason why this should be core and not an add-on in the asset library?

I will admit it might be possible to write an import script that will do a basic version of this, but I think it being in the core engine makes sense. It should be a fairly simple implementation (relatively speaking)

@Calinou
Copy link
Member

Calinou commented Dec 21, 2024

Shouldn't the lightmap size hint be defined using glTF metadata instead? If you're using externally authored UV2, this would make more sense to me.

That said, I'd expect Lightmap Texel Size to be effective when using externally authored UV2, with lower values resulting in a higher lightmap size hint value. This is probably easier to tweak than tweaking the lightmap size hint directly, so you don't end up with a wrong aspect ratio.

We would want to use this value to calculate the lightmap size hint of the meshes even if you don't want auto lightmap UVs, so maybe we should add an option for "Static Lightmaps (Manual UV2)" that defaults to not generating lightmap UVs, but simply setting a lightmap size hint for each mesh based on the texel size chosen.

Using the Static bake mode should work if your mesh has external UV2. Static Lightmaps just ensures UV2 is generated if it's not present.

@michaelharmonart
Copy link
Author

michaelharmonart commented Dec 21, 2024

honestly I didn't know gltf even contained such metadata.

If that's an option I'd be more than happy to use that. What search term would I use to find how to use that metadata? (I'm exporting glb files from blender)

Also didn't realize that that static lightmaps mode only generated uv2 for meshes that it wasn't present on. In that case the current system should be perfectly adequate without any need for lightmap hints assuming the lightmap texel size will effectively scale the pre authored uv2s

@Calinou
Copy link
Member

Calinou commented Dec 21, 2024

honestly I didn't know gltf even contained such metadata.

glTF allows for arbitrary metadata to be inserted. I don't think there's a standard field for that (like a lot of things in game development in general...). Godot would then have to be modified to read this metadata, or you can read it from an import script.

In that case the current system should be perfectly adequate without any need for lightmap hints assuming the lightmap texel size will effectively scale the pre authored uv2s

I'd try it and see what happens. If not, it sounds like a bug to me.

@michaelharmonart
Copy link
Author

michaelharmonart commented Dec 23, 2024

@Calinou
after some testing it looks like It's still doing some pretty heavy calculations on meshes that already have UV2s when set to use the "Static Lightmaps" import mode
(looking at the generated lightmaps it is clear that it is generating UV2s rather than simply calculating a scale for the already present UV2s)

So looks like my initial thoughts were correct. The current behavior is that all uv2s are discarded and regenerated if the import is set to "Static Lightmap"

The behaviour you described

That said, I'd expect Lightmap Texel Size to be effective when using externally authored UV2, with lower values resulting in a higher lightmap size hint value. This is probably easier to tweak than tweaking the lightmap size hint directly, so you don't end up with a wrong aspect ratio.

Sounds exactly like what I'd want the Godot default behavior to be.

In that case the current system should be perfectly adequate without any need for lightmap hints assuming the lightmap texel size will effectively scale the pre authored uv2s

I'd try it and see what happens. If not, it sounds like a bug to me.

So does that mean this should this be reported as a bug?

@Calinou
Copy link
Member

Calinou commented Jan 2, 2025

So does that mean this should this be reported as a bug?

As I understand it, lightmapping on meshes with external UV2 should already work if the scene is imported with the Static bake mode (not Static Lightmaps). The issue is that you can't change the lightmap texel size because it's hidden in the Import dialog, but I believe it could be made to work by making it visible.

After doing this, we could rename Static Lightmaps in the UI to Static Lightmaps (Generate UV2) to make its purpose more clear.

@michaelharmonart
Copy link
Author

That sounds like a pretty elegant way to do it!

If it really is as simple as making the Texel density slider visible and clarifying Static Lightmaps (Generate UV2) that would be ideal!

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

No branches or pull requests

2 participants