Implement 2D instance shader parameters

[huwpascoe]

From KoBeWi to EIREXE to here. Issue #62943.

instvartest.zip

I wanted to fit instance_uniforms_ofs into one of the unused push constant slots, but I couldn't figure out how to get that working easily. So with the path of least resistance, that's another 16 bytes to the instance data. Not ideal, but it's working.

I intend to bring this long-standing PR to completion.

[KoBeWi]

Parameter list doesn't work properly. It's not updated right when modifying shaders and some operations may replace it with nulls:

godot.windows.editor.dev.x86_64_PfbFzWf1sD.mp4

Other than that they look functional, which is nice.

[huwpascoe]

Fixed the instances not being freed.

I spent 3 hours trying to figure out why the parameter list is broken and found no solution. Essentially the MaterialStorage fails to update correctly when the shader is changed. The uniforms default values for instance variables are initialized to nil. A workaround is to reload the scene/project.

So the parameter list failing to refresh seems to be an unrelated issue.

[Calinou]

Tested locally (rebased on top of master 99a7a9c), it works as expected in Forward+ and Mobile.

When running in Compatibility, you get a shader compilation error like in 3D (as expected):

SHADER ERROR: Uniform instances are not supported in gl_compatibility shaders.

Testing project: test_pr_97058.zip

PS: What is this checkbox for? It's present for both 3D and 3D per-instance uniforms. I can't uncheck it (nothing happens when I click it), regardless of whether the value is modified from its default or not.

[huwpascoe]

What is this checkbox for?

Don't know, I think it was like that in 4.2 as well.

[KoBeWi]

To update property list, you need to use notify_property_list_changed. Here's how it's done in 3D:

godot/scene/3d/visual_instance_3d.cpp

Lines 244 to 246 in 02b16d2

	if (material_override.is_valid()) {
	material_override->connect(CoreStringName(property_list_changed), callable_mp((Object *)this, &Object::notify_property_list_changed));
	}

However modifying shader does not add the CanvasItem to _instance_update_list, I think it also has to be done manually.
The parameters no longer become null, so the list not updating is only an inconvenience now.

Also if you want the instance parameters to have revert button, you need to implement _property_can_revert() (return true if instance uniform exists) and _property_get_revert() (return default value of uniform).

[huwpascoe]

Redid everything from the ground up, refactored all the common code into a class. Everything should work identically to the 3D instances now.

[clayjohn]

I haven't taken a look at the whole thing, but just a reminder that this needs to be removed before this PR is finished. We need to avoid bloating the instance struct at all costs

[huwpascoe]

Once #97340 is merged then I'll have the space for it I reckon.

[clayjohn]

Why is that? #97340 clears up a spot in the instance buffer, but uses that spot for its own needs

[huwpascoe]

...oh. I need to find some space then.

[stuartcarnie]

Here is the space that you can use once #97340 merges:

https://github.com/godotengine/godot/pull/97340/files#diff-a960cbf133befa41681f0d917a05f34b59265036361c79b2c32ace27e0f3a40fR385

[Chaosus]

Please squash the commits as it's required by our pipeline (see https://docs.godotengine.org/en/latest/contributing/workflow/pr_workflow.html).

[huwpascoe]

Please squash the commits

Do I need to do anything special to keep the other contributor's names or do I just squash?

[clayjohn]

Please squash the commits

Do I need to do anything special to keep the other contributor's names or do I just squash?

You will need to have the "Co-Authored by:" lines for both of them (one is already there).

I suggest waiting to squash until you are finished with the PR and it is ready to merge.

[bunnybreaker]

Any idea on if/when this will get added to the main branch?

[huwpascoe]

Any idea on if/when this will get added to the main branch?

Depends on #97340. Soon as it's merged, I'll submit the final revision and then it's in the hands of the maintainers.