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

CSGPolygon add interpolation #53195

Closed
wants to merge 0 commits into from

Conversation

stebulba
Copy link
Contributor

@stebulba stebulba commented Sep 28, 2021

This new feature come from my proposal:
Add Scaling/Modify a CSG Polygon in PathFollow when PathNode is drawing the mesh
godotengine/godot-proposals#1062
This pull request supersede #41499 where is now updated with the last PR.
3.x version is here --> #53462
This new pull request can interpolate in all CSGPolygon mode.

The interpolation work using a Curve who interpolate between two polygon array. If both polygons are equal, a single scaling mode is active.

interpolate_exemple_angle_symplify
If the polygon2 is empty, when you active the interpolation bool, you make a copy of the first polygon to the second. Is a way to reset or help you for editing the polygons.
No interpolation are happening if the curve is null and if the size of polygon2 is different of the polygon.

The interpolation make 3 styles of modification, like a scaling, moving or expanding.
Those video show how that work for depth mode, spin mode and path mode. All videos is in 5 fps.

Mode Depth :
Doesn't use the avantage of path_interval. The interpolation is straight up from point to point of the curve.

mode_depth_scaling-moving-expand_5fps.mp4

Mode Path :
In a expand mode, to modify when you modifie x axis only in polygon2, you can for exemple make larger a racing/road in some point of your race.

mode_path_scaling-moving-expand_5fps.mp4

Mode Spin :
In mode spin I add a new property " spin_fix_neg ". When interpolation is enable, that will reverse/remove faces if the curve is negative. I don't like to add this additional property, but it's a constraint in mode spin if the user want set a negative position in the interpolate_curve.

mode_spin_scaling-moving-expand-spin_fix_neg_5fps.mp4

I am waiting the 3.x merge of #52509 before to pull request the version 3.x.
I need a revision for the documentation. English is not my first language.

@akien-mga
Copy link
Member

CC @BastiaanOlij @jitspoe

@BastiaanOlij
Copy link
Contributor

I need to find time to give this a run for its money and see if I agree with the way it's implemented but at face value this is really cool and powerful enhancement. I've put this on my todo list to give it a proper once over.

@jitspoe
Copy link
Contributor

jitspoe commented Oct 7, 2021

Of course I would rename the simplifiaction angle to "path_simplify_angle" and now it's being used for more than just that. Wonder if it should be just "simplify_angle"?

I feel like "interpolation" is maybe not the best way to describe this. Blender calls it a "profile" when you use a curve to define the bevel shape. Wonder what the shape is called on things like crown moulding.

@stebulba
Copy link
Contributor Author

stebulba commented Oct 7, 2021

Of course I would rename the simplifiaction angle to "path_simplify_angle" and now it's being used for more than just that. Wonder if it should be just "simplify_angle"?

The path simplify_angle still only use in mode path. spin_sides is the ways for mode spin and the mode depth is a simplest extrusion.

I feel like "interpolation" is maybe not the best way to describe this. Blender calls it a "profile" when you use a curve to define the bevel shape. Wonder what the shape is called on things like crown moulding.

Interesting. Like "interpolation" to "bevel profile "? And "interpolate_curve" to "profile_curve" ?

@jitspoe
Copy link
Contributor

jitspoe commented Oct 7, 2021

Interesting. Like "interpolation" to "crow_moulding" ? And "interpolate_curve" to "moulding_curve" ?

I think maybe enable_profile_curve and profile_curve would probably be the clearest. Crown moulding is the decorative strip put between a wall and ceiling, which sometimes has shapes similar to what you've demonstrated. I was just curious if there was a term for that. I think it might also be "profile".

@stebulba
Copy link
Contributor Author

stebulba commented Oct 7, 2021

enable_profile_curve

maybe "profile_polygons" mean you profile the interpolation between both with the next variable "profile_curve".
enable_profile_curve look too long. I prefer have a discussion on that, because the variables name is the most importante impossible to change without break compat. My suggestion :
clear that the 3 variables work together.

  • profile (_interpolation)
  • profile_polygon2
  • profile_curve
    or
  • profile_polygons
  • polygon2
  • profile_curve

My choice will be ( profile_interpolation, polygon2, profile_curve)
I will wait BastiaanOli' opinion before pushing modification.

@stebulba
Copy link
Contributor Author

stebulba commented May 7, 2022

Note to me:
Conflict invert_faces to flip_faces. #60838
To fix in same time interpolation to do_interpolate and fix soustraction -1 on spin.

@akien-mga
Copy link
Member

This was auto-closed by GitHub, probably because you pushed a state with no mergeable change (e.g. master branch) to your fork's csg_interpolation branch. AFAIK it can't be reopened when that happens so you'll have to open a new PR.

@stebulba
Copy link
Contributor Author

That will be easier, because I am stock with bugs.
icudt71l.dat still always recognized as modified.
Git include random other file too.
I use Git from ubuntu 16.04 lts. Maybe this version of Git is broke.

@Calinou
Copy link
Member

Calinou commented May 25, 2022

I use Git from ubuntu 16.04 lts. Maybe this version of Git is broke.

This is likely what's causing the "file is always recognized as modified" issue.

Ubuntu 16.04 is end-of-life, and Ubuntu 18.04 will be soon, so I recommend upgrading to Ubuntu 20.04 or later. In your case, you'll probably need to reinstall to upgrade, so you might as well install Ubuntu 22.04 and get 5 years of support this way.

@stebulba
Copy link
Contributor Author

stebulba commented May 25, 2022

Ubuntu 16.04 is end-of-life, and Ubuntu 18.04 will be soon, so I recommend upgrading to Ubuntu 20.04 or later

Sure, I am thinking about since long time. It's a big job, lot databases to migrate etc. I waited to buy a new computer, but the pression will force me to migrate before that.

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.

None yet

5 participants