Add drag-and-drop support for materials in 3D Instances #56597

SaracenOne · 2022-01-07T16:13:44Z

PR for adding drag and drop support for materials into 3D editor viewport. Uses the material override slot by default.

Things changed since draft:

Replace raw pointer hackery with proper references
Fix preview material getting stuck on mesh instances when mouse exits the viewport
Adds feature requested in Add support for dragging and dropping materials and textures onto MeshInstances and CSG shapes godot-proposals#394 to make a new material if a texture is dragged onto a mesh (consistent with the behaviour for dragging a texture in a material slot in the inspector).

I would still like to add some extra features in the future such as being able to drag onto individual surfaces and/or a modifer key to choose the individual surface once the mouse is released (similar to what happens when you drag a material onto a mesh instance in the scene tree), but I think this is in a good enough state for review now.

editor/plugins/node_3d_editor_plugin.cpp

SaracenOne · 2022-01-07T19:54:14Z

Updated to support assignment to CSG objects. Requires this PR to work properly though #56602

editor/plugins/node_3d_editor_plugin.h

SaracenOne · 2022-02-11T04:37:04Z

Okay, this has now been updated to a state that I think this can come out of draft and consider it ready for review. The biggest issue with code quality should be resolved now, a minor bug has been addressed, and new feature of being able to drag textures directly into your 3D scene has been added.

SaracenOne · 2022-02-11T05:02:09Z

Hold on, I missed something...

SaracenOne · 2022-02-11T05:11:00Z

Okay, should be sorted now

editor/plugins/node_3d_editor_plugin.cpp

fire

Wasn't able to give a deep technical review but my brief test shows the functionality does what I think it does.

2022-02-14.06-33-31.mp4

akien-mga · 2022-02-14T14:47:24Z

editor/plugins/node_3d_editor_plugin.cpp

+							break;
+						}
+
+						editor->set_preview_material(mat);


This is can_drop_data, AFAIK it's only supported to check if the selected files contain anything that can be used as a result of drag and drop.

This is actually correct, because it allows previewing the material before dropping it:

But AFAIK this method is called with every mouse move, so material swapping should only be done when needed to avoid performance problems.

akien-mga · 2022-02-14T14:48:13Z

editor/plugins/node_3d_editor_plugin.cpp

+	if (editor->get_preview_material().is_valid()) {
+		ObjectID new_object_id = _select_ray(p_point);
+
+		// Get the corresponding GeometryInstance for new_object_id, and if it is not valid, reset the new_object_id
+		if (new_object_id.is_valid()) {
+			GeometryInstance3D *geometry_instance = Object::cast_to<GeometryInstance3D>(ObjectDB::get_instance(new_object_id));
+			if (!geometry_instance) {
+				new_object_id = ObjectID();
+			}
+		}


Same thing, this is in the wrong callback. It should only be set when the drop action is actually confirmed (drop_data_fw I believe).

reduz · 2022-02-15T08:13:41Z

My main issue with this is that it uses material override. I feel this should be setting the proper surface and not the override (or at least we should make it controllable somehow by hitting a key while dragging over the viewport).

reduz · 2022-02-15T08:15:31Z

Maybe show some subtext while dragging at the bottom of the viewport like "hit space to switch between surface and override", or some option in the view menu setting? like DND Material -> Set Surface, Set Override.

akien-mga · 2022-03-29T12:54:13Z

As discussed on PR this requires some more work, maybe other contributors familiar with it can help:

As @reduz mentioned this should likely apply to surfaces by default, and with a modifier it can apply as override. Maybe @JFonS can help
The drag and drop API is not properly used, should be refactored (maybe @KoBeWi or @groud can help?)
To show a text label while dragging to display whether it's affecting surface (and which one) or override, you can look at the CanvasItemEditorPlugin that has similar stuff for dragging PNGs

SaracenOne · 2022-04-23T16:28:09Z

@KoBeWi Okay, should be resolved now

reduz · 2022-04-23T21:55:31Z

scene/resources/mesh.h

@@ -42,6 +42,7 @@ class Mesh : public Resource {
 	GDCLASS(Mesh, Resource);

 	mutable Ref<TriangleMesh> triangle_mesh; //cached
+	mutable Vector<Ref<TriangleMesh>> surface_triangle_meshes; //cached


I think rather than doing this, it should be simpler to modify TriangleMesh to contain an index of the surface ID as an optional second argument Vector<int32_t> to triangle_mesh->create()

This will also ensure that ray collision is correct.

SaracenOne · 2022-04-24T07:22:17Z

Since I didn't write the per-surface assignment part of this, do you have any thoughts @gaudecker? The technique seems like it would be a little more efficent, but I feel there was likely reluctance to alter too many internal data structures for one feature; supplying an array of custom face IDs may not be too bad though, since it could possibly be used for other things outside of the scope of this too.

@reduz One alternative approach I might want to mention though is instead of simply creating the TriangleMesh with the surface IDs directly, instead, we modify the intersection functions so the return the specific face ID (and maybe even the exact triangle intersection point too) which could then be cross-referenced with an external ID buffer which would give the surface IDX. The reason doing it this way might be better is it may make the TriangleMesh more generically useful when it comes to mapping intersections tests with other forms of regular mesh data, like UVs for example.

It could be just me though, I tend to get a bit nervous about modifying APIs for one specific purpose unless I can claim there's other stuff changing it might be useful for.

SaracenOne · 2022-04-24T07:35:59Z

Actually, thinking about it, the internal TriangleMesh data would still likely need to be modified to factor the mapping between raw triangle data and face indicies, but my question was how generic this metadata should be

gaudecker · 2022-04-24T07:50:40Z

@SaracenOne In the context of #59266, I think these meta-indices could probably work both for mesh surfaces and CSG faces. 🤔

There are probably other use-cases as well.

reduz · 2022-04-27T08:54:03Z

I would just harcode it into TriangleMesh because this is only really used by the editor, to be honest. If other uses can be added, I think they should be added at the time they are needed.

Helps unblock godotengine#56597

reduz · 2022-06-22T19:50:53Z

I opened #62321 to help unblock this PR. Once merged, it should make implementation of this easier as you can simply query the surface index of the ray collision.

fire · 2022-07-19T18:32:56Z

I will be salvaging this.

Add mesh surface picking for material drag & drop, show drag info label

fire · 2022-07-19T20:14:05Z

Having an infinite loop in TriangleMesh::intersect_ray. Investigating. This is not the pr branch, but trying to use reduz's new api.

reduz · 2022-07-27T15:10:48Z

@fire Did you have any luck so far?

fire · 2022-07-27T17:13:04Z

Not so much. I got stuck and I moved onto assisting tokage on animations and lyuma on ModificationStack conversion to nodes..

reduz · 2022-07-28T07:21:25Z

Ok, if for the PR it works well, your freeze is mostly unrelated because all my code did was add an extra return information.

akien-mga · 2022-07-28T07:22:41Z

Thanks!

TokisanGames · 2022-07-31T16:10:42Z

I think this should not put the material on the GeometryInstance3D/material_override. It should be on the actual mesh material slot (Mesh/surface_X/material or CSG/material), with an option to go on the MeshInstance3D/surface_material_override/X. Or the reverse, on the surface override with an option for the mesh.

Reasons:

For attaching it to the mesh (Mesh/surface_X/material): The most common use case is setting up master assets, which are saved in scenes to be reused. When meshes are imported, materials are attached to the mesh. We don't import materials into override slots, so dropping materials on meshes should also default to this most common use case. That is a specific material for a specific mesh, tied to the actual mesh as a master material. Not just instances of the mesh.
For Mesh Instance override as a secondary option (MeshInstance3D/surface_material_override/X): In some less common cases, I might have generic objects like rocks that can use multiple rock materials. In this case I'd want a blank or default rock material on the mesh, and a quick drag and drop to the mesh instance surface material override for a custom, per instance override of say a mossy or dirt covered variant.
Against using GeometryInstance3D/material_override: besides the fact that it's way down in the inspector, the biggest reason against this is material_override only supports one slot. surface_material_override and mesh/surfaces support all material slots. If I have an object w/ 4 material slots, there's no reason I would want the same material in all four slots. If I wanted full coverage of one material, I wouldn't have taken the time to split the uvs into 4 groupings. I would have made a mesh with only one slot.

e.g. 4 material slots

SaracenOne force-pushed the material_drag_and_drop branch from 4775b1f to 980801a Compare January 7, 2022 16:33

Chaosus added enhancement topic:3d topic:editor labels Jan 7, 2022

Chaosus added this to the 4.0 milestone Jan 7, 2022

Calinou added the usability label Jan 7, 2022

fire reviewed Jan 7, 2022

View reviewed changes

editor/plugins/node_3d_editor_plugin.cpp Outdated Show resolved Hide resolved

SaracenOne force-pushed the material_drag_and_drop branch from 980801a to 9cdf295 Compare January 7, 2022 19:49

fire reviewed Feb 4, 2022

View reviewed changes

editor/plugins/node_3d_editor_plugin.h Outdated Show resolved Hide resolved

SaracenOne force-pushed the material_drag_and_drop branch from 9cdf295 to e93163e Compare February 11, 2022 04:22

SaracenOne marked this pull request as ready for review February 11, 2022 04:32

SaracenOne requested a review from a team as a code owner February 11, 2022 04:32

SaracenOne force-pushed the material_drag_and_drop branch from e93163e to 6d84182 Compare February 11, 2022 04:33

SaracenOne force-pushed the material_drag_and_drop branch from 6d84182 to 51f9daa Compare February 11, 2022 04:56

SaracenOne force-pushed the material_drag_and_drop branch from 51f9daa to 519ee22 Compare February 11, 2022 05:10

aaronfranke reviewed Feb 11, 2022

View reviewed changes

editor/plugins/node_3d_editor_plugin.cpp Outdated Show resolved Hide resolved

editor/plugins/node_3d_editor_plugin.cpp Show resolved Hide resolved

SaracenOne force-pushed the material_drag_and_drop branch 2 times, most recently from b78e670 to 9fe6e8e Compare February 12, 2022 03:42

fire approved these changes Feb 14, 2022

View reviewed changes

akien-mga reviewed Feb 14, 2022

View reviewed changes

akien-mga added the needs work label Feb 25, 2022

Calinou mentioned this pull request Mar 18, 2022

Allow overriding face materials & UVs for CSGBox, CSGCylinder, and CSGPolygon #59266

Open

SaracenOne force-pushed the material_drag_and_drop branch from 1d83275 to dade434 Compare April 23, 2022 15:46

reduz requested changes Apr 23, 2022

View reviewed changes

lyuma force-pushed the material_drag_and_drop branch from dade434 to 720deca Compare May 3, 2022 23:08

Calinou mentioned this pull request May 9, 2022

Have raycast in GDScript be able to intersect with non-collision mesh triangles and return the UV coordinate of the texture, as well as the standard info already present godotengine/godot-proposals#2863

Open

reduz added a commit to reduz/godot that referenced this pull request Jun 22, 2022

Add surface indices to TriangleMesh

3d1ab57

Helps unblock godotengine#56597

reduz mentioned this pull request Jun 22, 2022

Add surface indices to TriangleMesh #62321

Merged

Add drag-and-drop support for materials in 3D

86aa2a8

Add mesh surface picking for material drag & drop, show drag info label

fire force-pushed the material_drag_and_drop branch from 720deca to 86aa2a8 Compare July 19, 2022 19:57

reduz approved these changes Jul 28, 2022

View reviewed changes

akien-mga removed the needs work label Jul 28, 2022

akien-mga merged commit 422725c into godotengine:master Jul 28, 2022

akien-mga mentioned this pull request Jul 28, 2022

Add support for dragging and dropping materials and textures onto MeshInstances and CSG shapes godotengine/godot-proposals#394

Closed

fire deleted the material_drag_and_drop branch July 28, 2022 11:16

Calinou mentioned this pull request Sep 20, 2023

the "drag and drop a material on a mesh instance" feature behaves improperly in the case where the dragged material was the one set already on the hovered mesh instance #74077

Closed

themancalledjakob mentioned this pull request Mar 31, 2024

the "drag and drop a material on a mesh instance" does not work for a ShaderMaterial #90075

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add drag-and-drop support for materials in 3D Instances #56597

Add drag-and-drop support for materials in 3D Instances #56597

SaracenOne commented Jan 7, 2022 •

edited

Loading

SaracenOne commented Jan 7, 2022

SaracenOne commented Feb 11, 2022

SaracenOne commented Feb 11, 2022

SaracenOne commented Feb 11, 2022

fire left a comment

akien-mga Feb 14, 2022

KoBeWi Mar 29, 2022 •

edited

Loading

akien-mga Feb 14, 2022

reduz commented Feb 15, 2022

reduz commented Feb 15, 2022

akien-mga commented Mar 29, 2022

SaracenOne commented Apr 23, 2022

reduz Apr 23, 2022

reduz Apr 23, 2022

SaracenOne commented Apr 24, 2022 •

edited

Loading

SaracenOne commented Apr 24, 2022

gaudecker commented Apr 24, 2022 •

edited

Loading

reduz commented Apr 27, 2022

reduz commented Jun 22, 2022

fire commented Jul 19, 2022

fire commented Jul 19, 2022

reduz commented Jul 27, 2022

fire commented Jul 27, 2022

reduz commented Jul 28, 2022

akien-mga commented Jul 28, 2022

TokisanGames commented Jul 31, 2022 •

edited

Loading

Add drag-and-drop support for materials in 3D Instances #56597

Add drag-and-drop support for materials in 3D Instances #56597

Conversation

SaracenOne commented Jan 7, 2022 • edited Loading

SaracenOne commented Jan 7, 2022

SaracenOne commented Feb 11, 2022

SaracenOne commented Feb 11, 2022

SaracenOne commented Feb 11, 2022

fire left a comment

Choose a reason for hiding this comment

akien-mga Feb 14, 2022

Choose a reason for hiding this comment

KoBeWi Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

akien-mga Feb 14, 2022

Choose a reason for hiding this comment

reduz commented Feb 15, 2022

reduz commented Feb 15, 2022

akien-mga commented Mar 29, 2022

SaracenOne commented Apr 23, 2022

reduz Apr 23, 2022

Choose a reason for hiding this comment

reduz Apr 23, 2022

Choose a reason for hiding this comment

SaracenOne commented Apr 24, 2022 • edited Loading

SaracenOne commented Apr 24, 2022

gaudecker commented Apr 24, 2022 • edited Loading

reduz commented Apr 27, 2022

reduz commented Jun 22, 2022

fire commented Jul 19, 2022

fire commented Jul 19, 2022

reduz commented Jul 27, 2022

fire commented Jul 27, 2022

reduz commented Jul 28, 2022

akien-mga commented Jul 28, 2022

TokisanGames commented Jul 31, 2022 • edited Loading

SaracenOne commented Jan 7, 2022 •

edited

Loading

KoBeWi Mar 29, 2022 •

edited

Loading

SaracenOne commented Apr 24, 2022 •

edited

Loading

gaudecker commented Apr 24, 2022 •

edited

Loading

TokisanGames commented Jul 31, 2022 •

edited

Loading