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

Add CollisionShape3D custom debug colors #90644

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

BattyBovine
Copy link
Contributor

This allows changing the display colour of a collision shape preview on a per-shape basis. It also adds the ability to display a solid coloured preview of a collision shape.

Closes godotengine/godot-proposals#906

This is a feature that I had been working on independently before realising that #78273 already exists. My implementation is currently more complete, in that all collision shapes are supported, including height maps and convex/concave collision shapes. I was hesitant to make a duplicate pull request for this feature, but I was told I could do so in the comments of the other pull request.

Screenshot - 4_8_2024 , 7_44_08 PM

Screenshot - 4_13_2024 , 4_42_33 PM

@Zireael07
Copy link
Contributor

Honestly I NEED one of those to finally make it in!

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 292e50e), the base feature mostly works so far. This is encouraging 🙂

I noticed some issues though:

Testing project: test_pr_90644.zip

  • At runtime, instead of following the properties set for each shape in the editor, the collision shape color is defined by the last collision shape that is parented to a PhysicsBody3D in the scene tree:
Editor Runtime
Screenshot_20240611_180922 Screenshot_20240611_180914
  • The default shape opacity is lower, which makes their outlines less visible than in master;
master This PR
Screenshot_20240611_181128 Screenshot_20240611_181219

I suggest leaving the default color's opacity at 255 to avoid this. That said, even with opacity set to 255, collision shape lines become slightly less opaque than in master:

Screenshot_20240611_181328

  • Trimesh collision shape drawing seems to break under certain conditions, as seen in the above screenshot.

@BattyBovine
Copy link
Contributor Author

* The default shape opacity is lower, which makes their outlines less visible than in `master`;

This is an issue I noticed as well, and I did my best to compensate for it. I'll see about fixing it further, but as far as I can tell, this is an issue brought about by using vertex colours as albedo. No matter what I tried, the opacity was always significantly lower, no matter whether FLAG_SRGB_VERTEX_COLOR was set, or how I compensated for it myself, or whether opacity was set to the maximum. This might not be a problem I can fully solve with this PR.

I will look into this though, and the other issues are probably easily resolved, too.

@BattyBovine
Copy link
Contributor Author

Sorry for the immediate double-post, but I think I figured all of these issues out:

At runtime, instead of following the properties set for each shape in the editor, the collision shape color is defined by the last collision shape that is parented to a PhysicsBody3D in the scene tree:

This is actually not what was happening. It just appeared that way due to a different issue. The actual problem was that the custom colours were being set properly on the CollisionShape3D, but the Shape3D resource attached to these nodes were not unique, so they were secretly all sharing the same colour regardless of the CollisionShape3D property. I couldn't think of a way to make the CollisionShape3D nodes all update to reflect this without massively overcomplicating things, so I made the decision to make the debug colour properties part of the Shape3D resource instead. I didn't do this initially since I wanted to keep things consistent with the CollisionShape2D node, but this seems like a reasonable solution.

The default shape opacity is lower, which makes their outlines less visible than in master;

The issue here seems to just be a mistake I made and never noticed. Debug lines were having their opacity multiplied by 0.25 inside the Shape3D resource, for reasons I can't remember in the slightest. I simply removed that, and now everything seems much better.

Trimesh collision shape drawing seems to break under certain conditions, as seen in the above screenshot.

I have not been able to reproduce this since making the changes described above, so I think whatever was happening there was related to either the lower-than-intended opacity, or is fixed due to the Shape3D resource handling its own colour settings.

compare

@Mickeon
Copy link
Contributor

Mickeon commented Jul 5, 2024

This failed the Linux / Editor w/ Mono check and I am not sure why? Could it be worth to rebase and try another push?

@Calinou
Copy link
Member

Calinou commented Jul 5, 2024

I recall C# failing on CI a few weeks ago, so a rebase should fix the issue.

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Documentation is completely fine now. Something similar to this feature has been heavily requested, too.

The PR and commit title should probably be changed, as this PR modifies Shape3D.

#78273 also exists, but it's not as maintained, at all. I am to assume this PR would essentially make it redundant, but there are benefits in tying the debug color to CollisionShape3D, so it's worth keeping in mind.

@BattyBovine
Copy link
Contributor Author

BattyBovine commented Jul 6, 2024

It would make more sense to keep the colour options in CollisionShape3D, so I'd like to make that happen again. Moving it to Shape3D is an easy and clean fix for unique resources causing the colour settings to desync between nodes, but it's not an ideal fix. I'll take a look at the other PR later to see how it handles colour changes with non-unique Shape3D resources on CollisionShape3D nodes. In the meantime, I've edited the commit message and the title of the pull request to reflect how it works at the moment.

@BattyBovine BattyBovine changed the title Add CollisionShape3D custom debug colours Add Shape3D custom debug colours Jul 6, 2024
@BattyBovine BattyBovine changed the title Add Shape3D custom debug colours Add CollisionShape3D custom debug colours Aug 23, 2024
@BattyBovine
Copy link
Contributor Author

In light of the recent release of Godot 4.3, I have started working on this again. I finally figured out a method for updating all CollisionShape3D properties properly when one Shape3D resource is shared among many nodes. I was hoping that the other pull request for this feature would help, but it seems that had the same issue with not properly handling non-unique resources. As such, I came up with my own solution. It's not the most elegant, and I hope to improve it over time, but it does ensure that changes are synced correctly and it seems to work just fine.

@fire
Copy link
Member

fire commented Oct 10, 2024

Need to wait for the tests to pass before merge queue adding.

@Mickeon Mickeon modified the milestones: 4.x, 4.4 Oct 11, 2024
@akien-mga akien-mga changed the title Add CollisionShape3D custom debug colours Add CollisionShape3D custom debug colors Nov 12, 2024
This allows changing the display colour of a CollisionShape3D node on a per-shape basis.
It also adds the ability to display a solid coloured preview of a CollisionShape3D.

Closes godotengine/godot-proposals#906
@akien-mga
Copy link
Member

Rebased to solve merge conflicts.

@Repiteo Repiteo merged commit b41f02c into godotengine:master Nov 26, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 26, 2024

Thanks! Congratulations on your first merged contribution! 🎉

@TokisanGames
Copy link
Contributor

Filled collision shapes are great. This implementation has a fundamental problem in that the meshes don't get occluded. The material should be fixed so it doesn't render on top of everything, or at least have an option that might always be on.

Consider this real world scenario. I want to have a collision shape make a death zone in the riverbed. Is the collision shape in the right spot? It's impossible to tell from nearly any angle.

{8DCF0FAC-BE73-4F50-BE67-3D26733EB4F6}

My own filled collisionshape script creates a meshinstance with a material that shows me exactly where it is. I can fill out the river in 1/10th the time or less with this.

{9FAB6D62-628A-41C5-AEB5-8A7EBA6376AF}

@Mickeon
Copy link
Contributor

Mickeon commented Dec 3, 2024

Could you share your own script? It may expose the reason for the rendering to behave as is in the current PR.

@TokisanGames
Copy link
Contributor

TokisanGames commented Dec 3, 2024

Sure. It's probably using Alpha instead of TRANSPARENCY_ALPHA_DEPTH_PRE_PASS. The script only works for boxmesh. I was planning on eventually expanding it to other shapes and make a plugin when I discovered this PR.

@tool
class_name FilledCollisionShape3D
extends CollisionShape3D


@export var visible_in_game: bool = false
@export var update: bool = false : set = update_mesh
var mesh_inst: MeshInstance3D
var _timer: Timer


func update_mesh(value:bool = false) -> void:
	mesh_inst.mesh.size = shape.size


func _ready() -> void:
	if not ( visible_in_game or Engine.is_editor_hint() ):
		return

	if not shape:
		shape = BoxShape3D.new()
		shape.size = Vector3(10, 10, 10)
	
	# Setup visualization
	mesh_inst = MeshInstance3D.new()
	mesh_inst.mesh = BoxMesh.new()
	mesh_inst.mesh.size = shape.size
	mesh_inst.cast_shadow = GeometryInstance3D.SHADOW_CASTING_SETTING_OFF
	add_child(mesh_inst)
	var mat := StandardMaterial3D.new()
	mesh_inst.set_surface_override_material(0, mat)
	mat.transparency = BaseMaterial3D.TRANSPARENCY_ALPHA_DEPTH_PRE_PASS
	mat.cull_mode = BaseMaterial3D.CULL_DISABLED
	mat.albedo_color = Color(.15, .15, .15, .65)

	# Update size in editor
	if Engine.is_editor_hint():
		_timer = Timer.new()
		add_child(_timer)
		Util.reconnect(_timer.timeout, update_mesh)
		_timer.start(1.)

	
func my_set_shape(new_shape: Shape3D) -> void:
	shape = new_shape


func _set(property: StringName, value: Variant) -> bool:
	match property:
		"shape":
			my_set_shape(value)
			return true
		_:
			return false

@BattyBovine
Copy link
Contributor Author

If the only thing that needs to change here is to use TRANSPARENCY_ALPHA_DEPTH_PRE_PASS, then that's a very easy fix. As long as it doesn't cause any other issues somehow, I can make another pull request for that. I'll look into this soon.

@smix8
Copy link
Contributor

smix8 commented Dec 4, 2024

Other debug has an "x-ray" property for the debug that controls if part of the mesh (e.g. edges only) should have depth cull or not. Might want to go into that direction with settings instead of hard-coding only one material property. With transparency no matter your setting choice you will always create cases where the debug does not render correct depending on what else is going on in that project. So imo it is good to give options for users to fix that for their project.

@Calinou
Copy link
Member

Calinou commented Dec 8, 2024

We should implement glPolygonOffset() emulation in materials so that gizmo materials can make use of it. This will allow them to always be drawn in front of overlapping meshes without Z-fighting, but while still being able to make use of the usual depth sorting. This will also help implement godotengine/godot-proposals#1000.

This likely involves exposing a constant depth offset property in BaseMaterial3D.

Engines like DarkPlaces use this to prevent Z-fighting when drawing mesh-based decals and shape debug drawing:

image

Normal

xonotic20241208181351-00

r_showcollisionbrushes_polygonfactor -1 (default)

xonotic20241208181402-00

r_showcollisionbrushes_polygonfactor 0

Just to show what it looks like without any polygon factor or offset.

xonotic20241208181416-00

r_showcollisionbrushes_polygonfactor -10

Just to show what it looks like with a very high polygon factor (more than needed).

xonotic20241208181426-00

r_showcollisionbrushes_polygonfactor 10

Just to show what it looks like with the polygon factor set the wrong way around (it needs to be negative for this use case).

xonotic20241208181437-00

r_showcollisionbrushes_polygonfactor 0; r_showcollisionbrushes_polygonoffset -2

Using polygon offset instead of polygon factor. This is the solution that can be implemented at a shader level in Godot without having to modify mesh data on the CPU. A value of -1 works OK in most situations but leads to occasional Z-fighting, while -2 avoids it entirely.

xonotic20241208181500-00

@Calinou
Copy link
Member

Calinou commented Dec 9, 2024

#100211 implements something akin to the aforementioned r_showcollisionbrushes_polygonoffset. Please test 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More configurable/custom colors for visible collision shapes debug option (already implemented in 2D)