Allow more flexible adjustments of VisualInstance3D Lightmap Scale

[Calinou]

Any floating-point value greater than 0.01 can now be used. Prior to this change, only factors of 1×, 2× and 4× and 8× could be used.

I haven't spotted negative side effects from this change so far, but please test this in more complex scenes. cc @jcostello @WickedInsignia

• This closes Allow any floating-point value for Lightmap Scale in MeshInstance instead of just 1×/2×/4×/8× godot-proposals#3233. See also Improve Lightmapper Workflow godot-proposals#6544.

Testing project: test_lightmap_scale_float.zip

TODO

• Rename the property to something else and add a compatibility handler.

Preview

Lightmap Scale from left to right: 0.5×, 1×, 2×, 45×.

[jcostello]

I will test this. It would be nice to have the texel debug view merged to see it in action

[jcostello]

Some tests in a custom sponza scene.

Lightmap size at 1

Lightmap size at 1.75

It blows up at 2.25. Im pretty sure that the model was imported with a hight texel density and the issue is because the lightmap size (which is a already reported issue). It would be good to catch the error and display a message like Lightmap size too big. Trace down below.

Also it would be good to have a global lightmap size inside the lightmap to multiply the mesh value (I think there is a proposal)

To test this I have to update all meshes values but thankfully I was able to search by type and select all at once

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.1.dev.custom_build (a7da6e1785073f2c0629d81884224a41b6b27c11)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x7faae9c42520] (??:0)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x1a11ca) [0x7faae9da11ca] (??:0)
[3] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x4b14c59) [0x564d6cc7fc59] (??:0)
[4] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x4b20a97) [0x564d6cc8ba97] (??:0)
[5] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x133de50) [0x564d694a8e50] (??:0)
[6] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x1341e70) [0x564d694ace70] (??:0)
[7] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x3364dae) [0x564d6b4cfdae] (??:0)
[8] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x240bc67) [0x564d6a576c67] (??:0)
[9] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x240c7db) [0x564d6a5777db] (??:0)
[10] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0xc8728e) [0x564d68df228e] (??:0)
[11] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x4f82a60) [0x564d6d0eda60] (??:0)
[12] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x4c88526) [0x564d6cdf3526] (??:0)
[13] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x4f8cdbd) [0x564d6d0f7dbd] (??:0)
[14] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x2f17d45) [0x564d6b082d45] (??:0)
[15] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x2f18e11) [0x564d6b083e11] (??:0)
[16] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x2f19136) [0x564d6b084136] (??:0)
[17] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x2eaa96a) [0x564d6b01596a] (??:0)
[18] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x2eabaf1) [0x564d6b016af1] (??:0)
[19] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x2ec38c6) [0x564d6b02e8c6] (??:0)
[20] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x2ee9c4d) [0x564d6b054c4d] (??:0)
[21] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x2f01d16) [0x564d6b06cd16] (??:0)
[22] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0xb06859) [0x564d68c71859] (??:0)
[23] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x4c13be7) [0x564d6cd7ebe7] (??:0)
[24] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0x4c16632) [0x564d6cd81632] (??:0)
[25] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0xb07b9c) [0x564d68c72b9c] (??:0)
[26] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0xae30f0) [0x564d68c4e0f0] (??:0)
[27] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0xad43ee) [0x564d68c3f3ee] (??:0)
[28] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7faae9c29d90] (??:0)
[29] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7faae9c29e40] (??:0)
[30] /home/juan/dev/godot/bin/godot.linuxbsd.editor.x86_64(+0xae1005) [0x564d68c4c005] (??:0)
-- END OF BACKTRACE --
================================================================

[Calinou]

Also it would be good to have a global lightmap size inside the lightmap to multiply the mesh value (I think there is a proposal)

See godotengine/godot-proposals#3893 and its pull request #64908.

[jcostello]

Also it would be good to have a global lightmap size inside the lightmap to multiply the mesh value (I think there is a proposal)

See godotengine/godot-proposals#3893 and its pull request #64908.

Super nice. Where would be a good place to discuss the lightmap size hint property? Should't be that remove? We already have texel density import, lightmap size per mesh and global with #64908 PR

[Calinou]

Super nice. Where would be a good place to discuss the lightmap size hint property? Should't be that remove? We already have texel density import, lightmap size per mesh and global with #64908 PR

The lightmap size hint is used by the lightmapper to determine the base size of the object on the lightmap. We can't remove it, but we could make it read-only in the inspector as there's usually no point in manually editing it. This should be changed in a future PR though.

[jcostello]

Super nice. Where would be a good place to discuss the lightmap size hint property? Should't be that remove? We already have texel density import, lightmap size per mesh and global with #64908 PR

The lightmap size hint is used by the lightmapper to determine the base size of the object on the lightmap. We can't remove it, but we could make it read-only in the inspector as there's usually no point in manually editing it. This should be changed in a future PR though.

Should I made a proposal? I will suggest to hide it anyway since there is no useful information

[Calinou]

I will suggest to hide it anyway since there is no useful information

The lightmap size hint is useful information for troubleshooting why an object takes so much (or so little space) in the lightmap. It shouldn't be hidden 🙂

[jcostello]

@Calinou whats the current state of this and #64908?

[Calinou]

@Calinou whats the current state of this and #64908?

I'd need to add a compatibility handler (and rename the property so it's effective) for this PR to be in a mergeable state. It's too late to do this for 4.2 as feature freeze is imminent.

There were some concerns about the texel scale PR possibly causing increased lightmap bleeding (with scales below 1.0, i.e. less detailed than the default). This needs further investigation, which means it'll also have to be pushed after 4.2.

Edit (February 2024): Rebased and tested again, it works as expected. I haven't added compatibility handlers yet.

[passivestar]

2.0 on the train, 0.5 on the rocks and the floor, 0.2 on the background cliffs. It works!

Rename the property to something else (any suggestions?)

How about lightmap_texel_scale to match the name in #64908 ?

[Calinou]

Rebased and tested again, it works as expected.

I've pushed a second commit that makes the PR fully backwards-compatible. It automatically converts the old properties into the new ones and keeps the GDExtension API identical.

I'll squash commits into one if this is OK.

[jcostello]

Rebased and tested again, it works as expected.

I've pushed a second commit that makes the PR fully backwards-compatible. It automatically converts the old properties into the new ones and keeps the GDExtension API identical.

I'll squash commits into one if this is OK.

Works as expected. I tested on a personal project as well as the TPSDemo. This gives a lot of freedom to bake better and efficient lightmaps

[clayjohn]

Looks good to me with the new compatibility methods. I'd say go ahead and squash this so we can merge for the next dev release

[Repiteo]

Thanks!