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

gaudecker · 2022-03-18T07:59:20Z

Add unique face materials and UV transforms for select CSG nodes.

This is in response to godotengine/godot-proposals#937 and godotengine/godot-proposals#2879

This is backwards compatible, since the old material property is used as a fallback when any of the new materials are null.

Simple demo project:

csg-faces-demo-3.zip

modules/csg/csg_shape.cpp

Calinou · 2022-03-18T16:12:10Z

This is a good idea (one that I've been thinking about for a while). However, I'm not sure if this will be very usable in practice since you have to apply the material on faces one-by-one (without being able to drag and drop materials onto faces). This is especially the case if you've rotated the CSG node beforehand, which means that the top face may not be at the top anymore (for instance).

SaracenOne is working on implementing material drag-and-drop support in master. The implementation works on CSG nodes, but it doesn't take per-face materials into account yet.

The changes mentioned above would definitely break compatibility with existing projects, so this needs to be considered before 4.0 is released.

Should the new faces be called override_{face}, since they override the default material?

If the are labeled as override materials, should they be collapsed under a group in the inspector (like surface_material_override/* in MeshInstance3D)?

I'd say yes and yes 🙂

Also, CSGCylinder and CSGPolygon should probably have an override property for the "sides". This is useful when you only want to override the sides material, not the top and bottom. This would also be more consistent with CSGBox having 6 material properties (rather than just 5).

gaudecker · 2022-03-20T15:11:55Z

This is a good idea (one that I've been thinking about for a while). However, I'm not sure if this will be very usable in practice since you have to apply the material on faces one-by-one (without being able to drag and drop materials onto faces). This is especially the case if you've rotated the CSG node beforehand, which means that the top face may not be at the top anymore (for instance).

I couldn't think of a cleaner way to expose these at the moment. I thought of just exposing them as indices like in MeshInstance3D, but that would make setting specific faces even more of a gamble. With proper face names there's at least some point of reference even if the object is rotated.

After #56597 is merged, we can do some face normal comparisons to determine what face is under the mouse pointer and set the correct material.

Should the new faces be called override_{face}, since they override the default material?

If the are labeled as override materials, should they be collapsed under a group in the inspector (like surface_material_override/* in MeshInstance3D)?

I'd say yes and yes slightly_smiling_face

Also, CSGCylinder and CSGPolygon should probably have an override property for the "sides". This is useful when you only want to override the sides material, not the top and bottom. This would also be more consistent with CSGBox having 6 material properties (rather than just 5).

This is now done 👍

gaudecker · 2022-03-30T07:50:50Z

After some thinking, I think that this approach to defining face materials is not good in the long run. It is too limited and offers no way for future extension.

Instead, I think the correct way would be to expose the surfaces as an arbitrarily long array of surface overrides.

In the future these surface overrides can be extended to allow specifying uv transformations as well.

I am going to rewrite this PR soon, if anyone has opinions on this I would like to hear them.

Calinou · 2022-03-30T17:43:41Z

Instead, I think the correct way would be to expose the surfaces as an arbitrarily long array of surface overrides.

Isn't this done for CSGMesh already?

On the other hand, I agree that the approach in this PR will show significant limitations if you use it for CSGPolygon (where you may want to apply a specific texture on one part of the sides only).

gaudecker · 2022-03-30T18:00:55Z

Isn't this done for CSGMesh already?

CSGMesh yes, but not others I think.

gaudecker · 2022-04-08T12:44:29Z

I accidentally added UV editing while I was refactoring 😅

Kooha-04-08-2022-15-40-23.mp4

I'll do some minor clean up, then this should be ready to go!

gaudecker · 2022-04-10T11:50:47Z

I think this should now be ready for review.

One thing that bothers be is that there's awful lot of repetition here, would it be good to move these override getters/setters under CSGShape3D and expose them to the inspector and GDscript from the child classes? Or just expose them as properties of CSGShape?

Calinou · 2022-11-07T12:32:12Z

Does this PR only duplicate surfaces when needed? For performance reasons, we should avoid duplicating surfaces when they use the same material (or no material at all).

gaudecker · 2022-11-07T16:52:39Z

Does this PR only duplicate surfaces when needed? For performance reasons, we should avoid duplicating surfaces when they use the same material (or no material at all).

I'm not sure what you mean.

The default material is used for surfaces that are not overridden. Or do you mean that when same material is used twice (or more) on different surfaces, it'd point to the same material and they'd be rendered in the same pass?

Calinou · 2022-11-07T17:59:34Z

The default material is used for surfaces that are not overridden. Or do you mean that when same material is used twice (or more) on different surfaces, it'd point to the same material and they'd be rendered in the same pass?

Yes 🙂

This should also apply to the fallback material (i.e. when a face doesn't have any material set).

That said, the number of surfaces is different from the number of materials. If you want rendering to be able to use instancing (dynamic batching), it needs to avoid using multiple surfaces, not just multiple materials. Otherwise, each surface will be in its own draw call.

gaudecker · 2022-11-08T05:48:33Z

Hmm.. I have no idea how to do this. Can anyone help?

Also, I would like if someone could review the code. 🙏

FyiurAmron · 2023-12-29T15:09:29Z

@gaudecker @Calinou any update on this? Is it abandoned or can we assume it will be shipped in some (hopefully not that far) future? TBH this is a great and powerful improvement and Godot could really use it.

gaudecker · 2023-12-30T07:55:25Z

You can consider this abandoned.

I assume this never moved forward for the same reason as #59386, there should've have been a proposal first.

I personally don't have time or energy for that at this point.

AThousandShips · 2023-12-30T09:14:10Z

Shall I go ahead and mark this as salvageable? And someone can pick it up if you end up not coming back to it 🙂

AThousandShips

Leaving these for a future salvager of this PR

AThousandShips · 2023-12-29T17:51:20Z

modules/csg/csg_shape.cpp

+	face_uv_transforms.set(p_face, transform);
+	_make_dirty();
+}
+Vector2 CSGBox3D::get_face_uv_offset(int p_face) const {


Suggested change

Vector2 CSGBox3D::get_face_uv_offset(int p_face) const {

Vector2 CSGBox3D::get_face_uv_offset(int p_face) const {

Separate methods

AThousandShips · 2023-12-29T17:51:29Z

modules/csg/csg_shape.cpp

+	face_uv_transforms.set(p_face, transform);
+	_make_dirty();
+}
+float CSGBox3D::get_face_uv_rotation(int p_face) const {


Suggested change

float CSGBox3D::get_face_uv_rotation(int p_face) const {

float CSGBox3D::get_face_uv_rotation(int p_face) const {

AThousandShips · 2023-12-29T17:51:33Z

modules/csg/csg_shape.cpp

+	face_uv_transforms.set(p_face, transform);
+	_make_dirty();
+}
+float CSGBox3D::get_face_uv_skew(int p_face) const {


Suggested change

float CSGBox3D::get_face_uv_skew(int p_face) const {

float CSGBox3D::get_face_uv_skew(int p_face) const {

AThousandShips · 2023-12-29T17:52:00Z

modules/csg/csg_shape.cpp

+	face_uv_transforms.set(p_face, transform);
+	_make_dirty();
+}
+Vector2 CSGCylinder3D::get_face_uv_offset(int p_face) const {


Suggested change

Vector2 CSGCylinder3D::get_face_uv_offset(int p_face) const {

Vector2 CSGCylinder3D::get_face_uv_offset(int p_face) const {

AThousandShips · 2023-12-29T17:52:07Z

modules/csg/csg_shape.cpp

+	face_uv_transforms.set(p_face, transform);
+	_make_dirty();
+}
+float CSGCylinder3D::get_face_uv_rotation(int p_face) const {


Suggested change

float CSGCylinder3D::get_face_uv_rotation(int p_face) const {

float CSGCylinder3D::get_face_uv_rotation(int p_face) const {

AThousandShips · 2023-12-29T17:52:40Z

modules/csg/csg_shape.cpp

+	face_uv_transforms.set(p_face, transform);
+	_make_dirty();
+}
+float CSGPolygon3D::get_face_uv_rotation(int p_face) const {


Suggested change

float CSGPolygon3D::get_face_uv_rotation(int p_face) const {

float CSGPolygon3D::get_face_uv_rotation(int p_face) const {

AThousandShips · 2023-12-29T17:52:47Z

modules/csg/csg_shape.cpp

+	face_uv_transforms.set(p_face, transform);
+	_make_dirty();
+}
+float CSGPolygon3D::get_face_uv_skew(int p_face) const {


Suggested change

float CSGPolygon3D::get_face_uv_skew(int p_face) const {

float CSGPolygon3D::get_face_uv_skew(int p_face) const {

AThousandShips · 2023-12-30T16:38:00Z

modules/csg/csg_shape.cpp

+		p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_lesser,or_greater,radians"));
+		p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians"));


Suggested change

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_lesser,or_greater,radians"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_less,or_greater,radians_as_degrees"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians_as_degrees"));

New syntax

AThousandShips · 2023-12-30T16:38:20Z

modules/csg/csg_shape.cpp

+		p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_lesser,or_greater,radians"));
+		p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians"));


Suggested change

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_lesser,or_greater,radians"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_less,or_greater,radians_as_degrees"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians_as_degrees"));

AThousandShips · 2023-12-30T16:40:19Z

modules/csg/csg_shape.cpp

+		p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_lesser,or_greater,radians"));
+		p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians"));


Suggested change

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_lesser,or_greater,radians"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_less,or_greater,radians_as_degrees"));

p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians_as_degrees"));

AThousandShips · 2024-02-06T10:53:28Z

@kryztoval Please don't bump without contributing significant new information. Use the 👍 reaction button on the first post instead.

However this has been abandoned by the original contributor so someone else will have to pick it up and make it ready

evertiro · 2024-04-10T08:01:34Z

Leaving a note on this as my map designer has requested this functionality and if I get tired of hearing him complain about not having it, I'll likely pick this up and get it refactored for the latest codebase.

gaudecker commented Mar 18, 2022

View reviewed changes

modules/csg/csg_shape.cpp Outdated Show resolved Hide resolved

Calinou added enhancement topic:core topic:3d labels Mar 18, 2022

Calinou added this to the 4.0 milestone Mar 18, 2022

Calinou mentioned this pull request Mar 18, 2022

Enable use_collision on CSGShape3D by default #59288

Open

gaudecker force-pushed the csg-face-materials branch from f334651 to f47abf1 Compare March 20, 2022 14:47

gaudecker marked this pull request as ready for review March 20, 2022 15:12

gaudecker requested a review from a team as a code owner March 20, 2022 15:12

gaudecker force-pushed the csg-face-materials branch 5 times, most recently from d479d59 to 5d1f51d Compare March 22, 2022 11:40

This comment was marked as outdated.

Sign in to view

Calinou mentioned this pull request Mar 29, 2022

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

Merged

gaudecker force-pushed the csg-face-materials branch from 5d1f51d to d3567cb Compare April 8, 2022 13:49

gaudecker changed the title ~~Allow unique face materials for CSGBox, CSGCylinder, and CSGPolygon~~ Allow overriding face materials & UVs for CSGBox, CSGCylinder, and CSGPolygon Apr 8, 2022

gaudecker force-pushed the csg-face-materials branch 2 times, most recently from 837d35c to 740f506 Compare April 10, 2022 11:44

gaudecker force-pushed the csg-face-materials branch 2 times, most recently from 3a9e713 to 08868b6 Compare April 11, 2022 06:42

Override face materials & UV for CSGBox, CSGCylinder, and CSGPolygon

7164629

gaudecker force-pushed the csg-face-materials branch from 08868b6 to 7164629 Compare April 11, 2022 07:06

YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023

YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023

AThousandShips modified the milestones: 4.2, 4.3 Oct 26, 2023

Calinou added the salvageable label Jan 1, 2024

AThousandShips reviewed Jan 1, 2024

View reviewed changes

AThousandShips modified the milestones: 4.3, 4.x Feb 6, 2024

This comment was marked as duplicate.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

gaudecker commented Mar 18, 2022 •

edited

Loading

Calinou commented Mar 18, 2022 •

edited

Loading

gaudecker commented Mar 20, 2022

This comment was marked as outdated.

gaudecker commented Mar 30, 2022 •

edited

Loading

Calinou commented Mar 30, 2022 •

edited

Loading

gaudecker commented Mar 30, 2022

gaudecker commented Apr 8, 2022

gaudecker commented Apr 10, 2022 •

edited

Loading

Calinou commented Nov 7, 2022 •

edited

Loading

gaudecker commented Nov 7, 2022

Calinou commented Nov 7, 2022 •

edited

Loading

gaudecker commented Nov 8, 2022

FyiurAmron commented Dec 29, 2023

gaudecker commented Dec 30, 2023

AThousandShips commented Dec 30, 2023

AThousandShips left a comment

AThousandShips Dec 29, 2023

AThousandShips Dec 29, 2023

AThousandShips Dec 29, 2023

AThousandShips Dec 29, 2023

AThousandShips Dec 29, 2023

AThousandShips Dec 29, 2023

AThousandShips Dec 29, 2023

AThousandShips Dec 30, 2023

AThousandShips Dec 30, 2023

AThousandShips Dec 30, 2023

AThousandShips commented Feb 6, 2024 •

edited

Loading

evertiro commented Apr 10, 2024

This comment was marked as duplicate.

	Vector2 CSGBox3D::get_face_uv_offset(int p_face) const {

	Vector2 CSGBox3D::get_face_uv_offset(int p_face) const {

	float CSGBox3D::get_face_uv_rotation(int p_face) const {

	float CSGBox3D::get_face_uv_rotation(int p_face) const {

	float CSGBox3D::get_face_uv_skew(int p_face) const {

	float CSGBox3D::get_face_uv_skew(int p_face) const {

	Vector2 CSGCylinder3D::get_face_uv_offset(int p_face) const {

	Vector2 CSGCylinder3D::get_face_uv_offset(int p_face) const {

	float CSGCylinder3D::get_face_uv_rotation(int p_face) const {

	float CSGCylinder3D::get_face_uv_rotation(int p_face) const {

	float CSGPolygon3D::get_face_uv_rotation(int p_face) const {

	float CSGPolygon3D::get_face_uv_rotation(int p_face) const {

	float CSGPolygon3D::get_face_uv_skew(int p_face) const {

	float CSGPolygon3D::get_face_uv_skew(int p_face) const {

		p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_rotation", PROPERTY_HINT_RANGE, "-360,360,0.1,or_lesser,or_greater,radians"));
		p_list->push_back(PropertyInfo(Variant::FLOAT, "face_overrides/face_" + itos(i) + "/uv_skew", PROPERTY_HINT_RANGE, "-89.9,89.9,0.1,radians"));

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

Are you sure you want to change the base?

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

Conversation

gaudecker commented Mar 18, 2022 • edited Loading

Simple demo project:

Calinou commented Mar 18, 2022 • edited Loading

gaudecker commented Mar 20, 2022

This comment was marked as outdated.

gaudecker commented Mar 30, 2022 • edited Loading

Calinou commented Mar 30, 2022 • edited Loading

gaudecker commented Mar 30, 2022

gaudecker commented Apr 8, 2022

gaudecker commented Apr 10, 2022 • edited Loading

Calinou commented Nov 7, 2022 • edited Loading

gaudecker commented Nov 7, 2022

Calinou commented Nov 7, 2022 • edited Loading

gaudecker commented Nov 8, 2022

FyiurAmron commented Dec 29, 2023

gaudecker commented Dec 30, 2023

AThousandShips commented Dec 30, 2023

AThousandShips left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AThousandShips commented Feb 6, 2024 • edited Loading

evertiro commented Apr 10, 2024

This comment was marked as duplicate.

gaudecker commented Mar 18, 2022 •

edited

Loading

Calinou commented Mar 18, 2022 •

edited

Loading

gaudecker commented Mar 30, 2022 •

edited

Loading

Calinou commented Mar 30, 2022 •

edited

Loading

gaudecker commented Apr 10, 2022 •

edited

Loading

Calinou commented Nov 7, 2022 •

edited

Loading

Calinou commented Nov 7, 2022 •

edited

Loading

AThousandShips commented Feb 6, 2024 •

edited

Loading