This begin here: Closing an extruded shape generated over a circular path - #49 by lowclouds, after @aritzmetal noticed bogosities with his closed path extrusion. I thought a generic fix to Path3D would fix it, but that’s maybe more problematic, I’m not sure.
Anyway, I worked through a fix that could be used in ExtrudeShape* in shapebuilder.ts - I don’t have time to actually implement the patch, but here’s a playground with a working algorithm:
1 Like
i dont know what you are all doing in your example but at a quick glance it doesnt seem right ,
here is a simple image to show how normals should be , open path on left , close on right
1 Like
yes but the normals should not be averaged when the path is open , as he did in his example. Look at my image again , this is from blender itself and you will see the open path start and finish vertex normals are not averaged. here is a quick demo using a simple path tool i made many years back
Ok, some confusion based on my lack of clarity, I think. In my playground, the tangents are red, binormals green and normals are blue; I don’t change the normals at all. Then, the two figures on the left are unchanged from current Path3D behavior, while the two figures on the right apply the averaging code for the tangents on the endpoints and then adjust the binormals . This would only be done if the user specified ClosedPath = true for an extrusion. I think this gives the expected behavior even if the underlying path is not closed.
It’s interesting that Blender has this built into the path component, which was my original thought, in which this would be an enhancement to both Path3D and the extrusion code to use it. In this case, you’d pass the ClosedPath flag to Path3D and let it close the path and compute the new tangents and binormals
yes the closed path bi-normals need fixing in babylon , but looking at the PG, it really its the tangent that is incorrect and thus the calculation for the binormal is incorrect. ( sorry i called the binormals , the normals )
but for the open path , it does not need adjustments as you made them ( top right ) because the tangents would be a vector in the direction of that edge , making the binormal orthagonal to the edge at those points, which is correct and what i displayed above
So only the path closing needs fixing
Here’s the scenario I’m addressing: user passes in a path and a shape to ExtrudeShape* and specifies closePath: true. In this case, the extrusion will be closed and you will get the artifacts that @aritzmetal pointed out. It makes sense to me to have this option also close the open path if it is open and patch up the tangents and binormals. Note, too, that there is currently NO way to create a correct closed path to use in an extrusion.
the tangent and binormal must always be perpendicular to each other, so if you change one, you must change the other. The algorithm I proposed, changes the tangents based on what Path3D does for mid-path points, then adjusts the binormals by the angle change of the adjusted tangent to the original tangent. I’m thinking again that the best place to put this code is in Path3d in math_path.ts, but that code is even more obscure than shapebuilder.ts
I guess, with the modification I propose, you could no longer get the current behavior, because even with a closed path, the binormals would prevent the underlying ribbon paths from meeting if you chose closePath=false. Conversely, by supplying an open path while choosing closePath=true, you would avoid inserting an extra unnecessary point in the path coincident with the start point. This extra point would cause degenerate faces in the mesh to be created.
In order to avoid any backwards incompatibility, you could add a path3d option to ExtrudeShape* that would allow you to pass in a precomputed path3d, which it would use instead of the passed in array of points. Then you could modify Path3D to generate closed paths with nice tangents and binormals.
Hi,
I was following this topic from a distance.
May be I can allow myself to add just my two-cents here, saying that if I were you, I’d keep it just at the level of ‘ideation’ for now and not put too much efforts in it before getting feedback from the team (and in case, guidelines and a plan). In essence, I don’t think half a solution makes for a solution and as said, the entire thing is currently already a black hole. Adding just another level of complexity with exceptions is (in my opinion) not very ‘babylonish’.
Don’t misunderstand me: I totally do appreciate your efforts (a lot 
), and then, don’t ask me, just to make this clear, I would be incapable of doing just a fraction of what you do 
, …but I wouldn’t want you to spend time on something that might just never make it through the validation phase.
As I said, just my sunday morning thought looking at the situation at this very moment.
Take it or leave it, and meanwhile, enjoy your weekend 
1 Like
cc @Evgeni_Popov if he can have a look ?
Thanks @mawa, I have no plans to actually do the work - luckily, I don’t require this for my extrusions 
I really do think that the best place to make this change would be in Path3D as that would not cause any breakage (even if I think the current behavior is wrong). I am also reluctant to touch that piece of code, both because it touches a lot of other code and because it appears to be highly tuned.
(On a side note, there is a bug in my playground code for which I have a fix if anyone cares.)
2 Likes
I think it’s something that might take some time, as I’m not familiar with extrude/path3D code, so we should give some priority if it’s something I should look at at some point. Or ask for help 
1 Like
Yup same I will be happy to review a PR with @Evgeni_Popov if someone wants to create it.
2 Likes
Hi again @Evgeni_Popov, do you have a GH issue to subscribe to?
There’s no issue created for that, it’s not a priority in our current plan, but if anyone’s willing to contribute, we’re all for it!
closing the path is one thing , but all of this was for the usage case of extrusions and to assume that will just work properly once the path is closed is probably wishful thinking. I definitely know if the extrusions are doing any UV unwrapping this will need changes because while you do have shared vertices in the mesh , you cant have them shared in the UV island on the seam.
I’ve been slowly working myself up to this, but I know nothing about UV unwrapping. There are two ways you could implement the closing of a Path3D: merging the first and last points, or you could always ensure that they are separate with an identical set of normals. Or, I suppose, you could leave that to the user. Would those two solutions behave differently for your use case?
@ lowclouds Hi again, those two solutions seem good to me! Thanks 
(Sorry for the delay, we paused this part and now Im coming back to it)
1 Like