[3.x] CSGPolygon fixes and features: Angle simplification, UV tiling distance, interval type. #52509
Conversation
|
Here's the pull request for master: #52512 |
jitspoe
force-pushed
the
3.x.csg_fixes_and_simplification
branch
from
1f184c3 to
066e3be
Compare
|
Reading the original PR that changed the behavior it has no mention that the way UVs are calculated in this way, shame nobody caught it because I know a number of people who rely on the old behavior. Having the UV increase over the length of a path according to distance is paramount to anyone who uses CSG path extrusions to create roads, paths, etc. and want to use tiling textures that consistently tile over distance. So I'm all for this change. |
modules/csg/csg_shape.cpp
Outdated
| switch (path_rotation) { | ||
| case PATH_ROTATION_POLYGON: |
There was a problem hiding this comment.
There are style issues with the lack of indentation for switch cases:
https://github.com/godotengine/godot/pull/52509/checks?check_run_id=3556178843
There was a problem hiding this comment.
Yeah, I fixed everything up for the master version: #52512 Figured it'd be best to get that approved first then come back to this one. Not sure if that should be cherry-picked back into 3.x and this PR should be closed or if it's better to be independent, since the class names and stuff changed, likely making a merge problematic.
There was a problem hiding this comment.
Yeah finalizing the master PR before updating this one sounds good.
| @@ -50,6 +56,9 @@ | |||
| <member name="spin_sides" type="int" setter="set_spin_sides" getter="get_spin_sides"> | |||
| When [member mode] is [constant MODE_SPIN], the number of extrusions made. | |||
| </member> | |||
| <member name="uv_distance" type="float" setter="set_uv_distance" getter="get_uv_distance" default="1.0"> | |||
| When [member mode] is [constant MODE_PATH], this is the distance along the path, in meters, the texture coordinates will tile. When set to 0, texture coordinates will match geometry exactly with no tiling. | |||
There was a problem hiding this comment.
| When [member mode] is [constant MODE_PATH], this is the distance along the path, in meters, the texture coordinates will tile. When set to 0, texture coordinates will match geometry exactly with no tiling. | |
| When [member mode] is [constant MODE_PATH], this is the distance along the path, in meters, the texture coordinates will tile. When set to 0, texture coordinates will match geometry exactly with no tiling. |
|
Ok, I need to find some time to do a proper code review, I know @akien-mga already did some, but just played around with these fixes and I'm duly impressed. The There is some strangeness in the calculated Us for the width of my track so it took a bit of trial and error to get the right values and I need to dive into that a bit more to discover the why and if they make sense. Otherwise though the |
Ah, right. The UV only uses 50% of the texture so the other half can be used for the caps. I was contemplating adding an option for that, but I didn't want to make the CSG stuff TOO complicated. If you're going to review, check the master one, as I've done some cleanup and variable renames: #52512 |
Ah yes that makes sense I guess. So with 4 sides to the polygon being extruded adding up to have of the width of the texture my road section U width is 0.125. Hence * 8 brings it back to 1.0. |
|
Please edit the PR title to make it clear it's for 3.x (I was confused why there are two identical PRs sitting open in my notifications) |
akien-mga
changed the title
|
I'm happy merging the |
Bump :) |
Sorry, out of town for a few days. Will try to address this next week. |
akien-mga
force-pushed
the
3.x.csg_fixes_and_simplification
branch
from
066e3be to
cbf855d
Compare
|
I went ahead and amended your PR to do the same changes (renames, reordering) as in #52512. |
…distance option, and interval type, which allows distance-based intervals (old) and subdivision-based intervals (new to 3.4).
akien-mga
force-pushed
the
3.x.csg_fixes_and_simplification
branch
from
cbf855d to
d7af7a9
Compare
|
Thanks! |
[3.x] Quick fix on path_simplify_angle introduce here godotengine#52509 (comment) after merging godotengine#52509
[3.x] Quick fix on path_simplify_angle introduce here godotengine#52509 (comment) after merging godotengine#52509
[3.x] Quick fix on path_simplify_angle introduce here godotengine#52509 (comment) after merging godotengine#52509
This is to address new issues introduced in 3.4: #52179
Namely the removal of distance-based intervals and the way UV tiles. I needed these for creating cables in my game.
I also added an option for simplifying geometry (In my game, it reduced the polycount of a cable from around 2900 triangles to 600).
I'm submitting this directly to 3.x because of changes to the class names, removal of pool vector, etc. mean this can't be merged cleanly from master, and it might be nice to get these fixed before 3.4 rolls out. I'll do a separate pull request for master.