Add a debug draw mode for displaying UV2 (lightmap) texel density

[techiepriyansh]

Adds a debug draw mode for displaying UV2 texel density. It displays the UV2 texel density of all MeshInstance3Ds which have their GI_MODE set to GI_MODE_STATIC.

Partially addresses godotengine/godot-proposals#3213.

Todo

• Port to mobile backend.
• Figure out a better way to set DEBUG_UV2_TEXEL_DENSITY inside the scene shader instead of using a render mode define.
• Use the fancy checkerboarding shader.

[jcostello]

So far is looking good

[fire]

As the maintainer of the import pipeline giving more ways to determine this crucial detail to art budget and quality is important. 👍

[JFonS]

Changes look good so far.

I wonder if it wouldn't be easier to put the checkerboard code in the Godot shader code (i.e. here), rather than in the internal scene shader.

The lightmap size would be an instance uniform then.

[clayjohn]

Looks amazing so far! I think this was definitely the general approach to take.

With a little bit of polish you should be able to finish this up pretty soon!

[clayjohn]

Suggested change

	if (inst->shader_parameters_offset == -1) {
	if (inst->shader_parameters_offset == -1 && !inst->data->instance_allocated_shader_parameters && ) {

In my test project I get a spammed error because global_variables_instance_allocate() return -1 if the instance has already been allocated. Adding the check for instance_allocated_shader_parameters ensures that this doesn't happen.

[techiepriyansh]

I've added appropriate checks to mitigate the spammed error.

[clayjohn]

Did you test just adding && !inst->data->instance_allocated_shader_parameters? From my test it was able to remove the spammed error and it is much simpler code

[clayjohn]

The CI build is failing because of missing documentation changes. When you add new features that are exposed, the docs need to be updated as well. You can update the docs by running the doctool command and then filling in the changed sections in the .XML documentation.

For example, here is how you run ``doctool` on 64 bit linux

./bin/godot.linuxbsd.tools.64 --doctool

For more information, we have a whole page on this in the docs: https://docs.godotengine.org/en/latest/community/contributing/updating_the_class_reference.html

[clayjohn]

I am still getting the instance buffer error when there are duplicate mesh resources in the scene:

servers/rendering/renderer_rd/storage_rd/material_storage.cpp:2173 - Condition "global_shader_uniforms.instance_buffer_pos.has(p_instance)" is true. Returning: -1

Otherwise it looks great! The new shader looks amazing.

I think to resolve the issue we need to add a function to material_storage to check if the global shader uniforms instance buffer contains the instance. Then we can make that check directly instead of relying on global_shader_uniforms_instance_allocate to return -1.

[clayjohn]

Looks great! I like the new approach. It is unfortunate to add more to the material storage API just for a debug feature. But it makes your actual debug code much more elegant.

[clayjohn]

I've left one final style nitpick on the code comments (sorry). I am happy with the code as it is and have tested it extensively and it seems to work great! Good work, this required a lot more intrusive changes than any of us originally envisioned, but the results a pretty good.

Before merging @reduz and @JFonS should take a look as well.

[akien-mga]

@JFonS reduz was asking if this is intended to be merged before or after further changes you might have pending for UV2 layers?

[jcostello]

@akien-mga @clayjohn should this be taking into consideration? #64908

[Calinou]

@akien-mga @clayjohn should this be taking into consideration? #64908

You can have multiple LightmapGI nodes in a scene, so I don't think you can have a 1:1 mapping between meshes and the LightmapGI they're using. Some nodes may not be included in any LightmapGI yet either.

[atirut-w]

You can have multiple LightmapGI nodes in a scene

Possibly off-topic, but why would someone do that? Tricks using layers?

[JFonS]

The PR looks good to me.

I still haven't looked at moving UV2 to a custom layer, but it should be pretty straightforward. I would merge this PR, and then I will see if anything needs to be changed later on.

[akien-mga]

Needs rebase, then should be good to merge if @clayjohn approves it too.

[techiepriyansh]

Updated with master.

[clayjohn]

I'm happy with the changes. But it requires @reduz to take a quick look as well because of the additions to the MaterialStorage API.

[jcostello]

This PR is getting older. What can be done to merge it ASAP?

[Calinou]

@techiepriyansh Could you look into rebasing this pull request if you have time? If you don't have time, let us know and we can mark this PR as salvageable 🙂

[techiepriyansh]

Hi, as much as I'd like to get this PR merged myself, I'm afraid that I don't really have time right now. Please go ahead marking this PR as salvageable. When I get some time in the future, and it's still not taken up by anyone else, I'll do it.