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

Shader menu keeps closing with every change. #88564

Open
viksl opened this issue Feb 19, 2024 · 5 comments
Open

Shader menu keeps closing with every change. #88564

viksl opened this issue Feb 19, 2024 · 5 comments

Comments

@viksl
Copy link
Contributor

viksl commented Feb 19, 2024

Tested versions

4.2.1

System information

Windows 11 - Vulkan - Nvidia RTX 4070 - intel i5 13600KF

Issue description

Shader menu keeps closing on any change, check the video to see.
I'd expect the menu would stay open.

I did not label this just for visual shader since I don't know if there's some way to add stuff to this menu from gdshader too (?) so keeping it shader related in general.

Link: https://youtu.be/AlxfhCcsRAs (only first 10 seconds of the video are relevant, I then added gdshader just to show there's nothing else but I don't know if there can be only for clarity)

Steps to reproduce

Add a visual shader to a shader material and try changing some of its properties or use the MRP where it's all set up already. Follow the video below for better understanding.

Link: https://youtu.be/AlxfhCcsRAs (only first 10 seconds of the video are relevant, I then added gdshader just to show there's nothing else but I don't know if there can be only for clarity)

Minimal reproduction project (MRP)

ShaderMenuClosesMRP.zip

@jsjtxietian
Copy link
Contributor

Same as #81481

@viksl
Copy link
Contributor Author

viksl commented Feb 20, 2024

Changing Node3D properties such as rotation edit mode also causes the materials to fold but it does not fold Mesh resource of MeshInstance3D but it does collapse the Material (this time StandardMaterial3D). So it's not just VisualShader related.

If you have both material override and material overlay occupied then you can't have both open at the same time if you set a visual shader there.

This line in particular starts the collapse:

_clear(!object);

Inside that method it's this particular part it seems (commenting it out doesn't cause the collapse):
https://github.com/godotengine/godot/blob/fb10e67fefb85af3b8f5abd30db561588d8456c6/editor/editor_inspector.cpp#L3412C1-L3414C3

This comment makes me wonder:

	// Only hide plugins if we are not editing any object.
	// This should be handled outside of the update_tree call anyway (see EditorInspector::edit), but might as well keep it safe.
	_clear(!object);

Maybe the clear here could go away as the comment suggest and be only at place where it's supposed to be?

EDIT:
Heh I just noticed if the clear isn't called there it keeps adding new sections with the material obviously not wanted :D. But if the resource container is always deleted and created anew than the folding makes sense perhaps?

@KoBeWi
Copy link
Member

KoBeWi commented Feb 20, 2024

The inspector update is caused by this line:

const_cast<VisualShader *>(this)->set_code(final_code);

Visual shader will update the shader code with every change, which causes the shader to do emit_changed(), which makes ShaderMaterial reload its properties:
void ShaderMaterial::_shader_changed() {
notify_property_list_changed(); //update all properties
}

@viksl
Copy link
Contributor Author

viksl commented Feb 21, 2024

I understand that this is called and I'm out of my depth here but I don't think this is the main problem here.
For example, if you open the menu as in the video but instead of changing properties on VisualShader you scroll down and change rotation_edit_mode which is part of Transform section and not a shader then the same behaviour as in the report occurs but the code path doesn't trigger the lines above.
Apart from that it also closes ShaderMaterial3D in Mesh resource and maybe some other stuff elsewhere so I guess there might be other places triggered in case someone checks it out. I'll be looking but again I'm not very experienced with this so I don't have high expectations for myself here ;0.

EDIT: This is beyond so hopefully someone else picks it up.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 21, 2024

rotation_edit_mode also updates the inspector, but that is expected, as some properties change their display. So I guess a more complete solution would be not folding the resources when inspector updates.

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

4 participants