-
Notifications
You must be signed in to change notification settings - Fork 54
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
[FEATURE ADD] Add a bezier extrude #32
Comments
Bezier extrusion is a very nice feature! Currently, it is very important to keep the API in its current state since we are working on a different CSG renderer that might reuse the current API. What changes would you introduce? Would the API changes involve other classes, in addition to the Extrude class? |
I can make it clean room, but it would be convenient if i can add my helper functions to CSG such as: https://github.com/NeuronRobotics/JCSG/blob/master/src/main/java/eu/mihosoft/vrl/v3d/CSG.java#L1592 and https://github.com/NeuronRobotics/JCSG/blob/master/src/main/java/eu/mihosoft/vrl/v3d/CSG.java#L288 I generally have not changed any of your API's only extended them with cleaner syntax for user level code by adding pass through helper functions that are syntactically simpler and consistent. I have been doing a lot of development on this API and always am careful to make it so it merges with your upstream. I would have to make a 3rd fork, and in there hand tease out changes, then merge them into my system and yours in turn. I think thats likely worth the effort if we can agree on feature add paths and a development pipeline. |
So heres an idea, any chance you can check your projects that use JCSG to see if they work with my changes by simply changing the build.gradle to import:
and check if it breaks your builds. If it works, then i have not changed any API that you are using. |
I wrote a Bezier extursion, that lets you extrude along a bezier curve for making complex geometry, would you be interested in me pulling it out to make a clean PR for your upstream system?
It is a lot of work to pull it out to make the PR work, so ill only do it if you are interested in merging that feature (without having to pull all the rest of stuff iv been working on)
Here is the function i would be adding (along with its helper functions)
https://github.com/NeuronRobotics/JCSG/blob/master/src/main/java/eu/mihosoft/vrl/v3d/Extrude.java#L365
The text was updated successfully, but these errors were encountered: