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

Fix gizmo on top material option having no effect #90183

Merged

Conversation

stevenjt
Copy link
Contributor

@stevenjt stevenjt commented Apr 3, 2024

This fixes #44077

I found that the get_material function in EditorNode3DGizmoPlugin is changing the gizmo materials directly to add and remove the disable depth test flag to enable/disable them for the 'x-ray' visibility state before returning them. This is causing the original state of the disable depth test flag to be lost so materials that were created with the on_top parameter are never returned with the disable depth test flag enabled when in the regular 'visible' visibility state.

To fix this I have updated get_material to change the disable depth test flag on duplicate versions of the gizmo
materials when the gizmo is set to use the 'x-ray' / ON_TOP visibility state if the flag is not already set or otherwise return the stored materials without change.

(Since StandardMaterial3D is RefCounted these duplicates should be freed automatically when later unreferenced when the gizmo is redrawn.)

@stevenjt stevenjt requested a review from a team as a code owner April 3, 2024 20:01
Update get_material function in EditorNode3DGizmoPlugin so that it
enables the disable depth test flag on duplicate versions of the gizmo
materials if the flag is not already set and the gizmo is set to use the
'x-ray' visibility state.
@stevenjt stevenjt force-pushed the fix-gizmo-on-top-material-option branch from 601652e to 9b1a1d2 Compare April 3, 2024 20:32
@Calinou Calinou added this to the 4.3 milestone Apr 3, 2024
@Calinou Calinou added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Apr 3, 2024
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased on top of master b2f425f), it works as expected on #44077 (comment). Code looks good to me.

Toggling x-ray visibility on default gizmos still works.

@akien-mga akien-mga merged commit 5eadb88 into godotengine:master Apr 12, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@stevenjt stevenjt deleted the fix-gizmo-on-top-material-option branch April 12, 2024 20:18
@donn-xx
Copy link

donn-xx commented Sep 1, 2024

Trying this in my plugin: create_material("small_cursor_shape", Color.RED, false, true)
Not sure what it's meant to do, but my expectation is to see the red (sphere) through the geometry in the way. However it's hidden.

Zylann's workaround still better:

func create_on_top_material(name: String, color := Color.WHITE,
		bmode := StandardMaterial3D.BLEND_MODE_ADD):
	var material := StandardMaterial3D.new()
	material.set_blend_mode(bmode)
	material.set_shading_mode(StandardMaterial3D.SHADING_MODE_PER_VERTEX)
	material.set_flag(StandardMaterial3D.FLAG_DISABLE_DEPTH_TEST, true)
	material.set_albedo(color)
	material.render_priority = 100
	add_material(name, material)

ETA:
The func create_icon_material("test", iconref, true) is doing the same thing, the icon not visible through the other things in the scene.
ETA:
In the case of the icon_material, there is some more weirdness. My texture is from an svg. Sometimes the billboard drawn by add_unscaled_billboard does show through, other times not. Also, when the gizmo is selected, the texture gets an ugly background (it was alpha before).

Selected Gizmo

Selected gizmo. The background has gone awry.

Unselected Gizmo
Unselected gizmo.

Godot v4.4.dev (61598c5) - Ubuntu 22.04.4 LTS 22.04 - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 (nvidia; 535.183.01) - Intel(R) Core(TM) i5-9400F CPU @ 2.90GHz (6 Threads)

@stevenjt
Copy link
Contributor Author

stevenjt commented Sep 1, 2024

@donn-xx in your create_material example does it work when the node using the gizmo is selected? Gizmos have different material instances created for selected and unselected states. The on top effect is only applied to the selected material instance:

if (p_on_top && selected) {

@donn-xx
Copy link

donn-xx commented Sep 1, 2024

he selected material instanc
Selected and unselected. The billboard simply changes (in some way) how it appears.

How do we setup the selected and unselected materials?

Also, imo, I need the "icon material" (at least) to be visible always, so that it's visible in the scene (in the editor). It would be great to be able to select the node by clicking on the billboard/icon too. (Otherwise, why bother with it all? May as well select the node in the tree.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:editor topic:3d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The "on_top" parameter of EditorSpatialGizmoPlugin.create_material has no effect
4 participants