Account for curvature in curve to mesh node #80979
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
19 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#80979
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
System Information
Operating system: Darwin-18.5.0-x86_64-i386-64bit 64 Bits
Graphics card: NVIDIA GeForce GT 750M OpenGL Engine NVIDIA Corporation 4.1 NVIDIA-12.0.23 355.11.10.50.10.103
Blender Version
Broken: version: 2.90.0, branch: master, commit date: 2020-08-31 11:26, hash:
0330d1af29Worked: (newest version of Blender that worked as expected)
Short description of error
I used a curve as a bevel profile on another one with 90 degree angles but the result is not 90 degrees on the result
Exact steps for others to reproduce the error
please see attached file
badBevel.blend
Added subscriber: @Funnybob
#103419 was marked as duplicate of this issue
#83207 was marked as duplicate of this issue
#81071 was marked as duplicate of this issue
Added subscriber: @HooglyBoogly
Thanks for the report. For anyone else, here is the top view of the curve:
It's just using the same profile object at each segment, not taking into account the angle of the curve at each point to change the radius.
I don't believe this is a bug, but I would propose this be made a TODO for curves, since it's a nice feature and it doesn't seem like it would be too difficult.
Changed status from 'Needs Triage' to: 'Confirmed'
Added subscriber: @ideasman42
While we could have an option for this to work differently (how it works for 2D curves), this isn't a bug.
Setting as good-first-task.
Hint, see
shell_v3v3_normalized_to_distand related functions.Added subscribers: @vocalize, @lichtwerk
Added subscriber: @dilithjay
Hi. This is my first time contributing to any open source project ever so any help I can get is highly appreciated.
I believe I understand the math needed to make the changes. Unfortunately, I'm having a really hard time identifying which part of the code I should be paying attention to. I took a look at the
shell_v3v3_normalized_to_distfunction as well but I was unable move forward from there. After hours of looking around in the source code and using printf, I'm led to believe that curve_bevel.c (under source/blender/blenkernel/intern/) has something to do with it but I'm not entirely sure. Any help is much appreciated.Also, please do let me know if there's anything else I need to include in a comment like this. Thank you.
@dilithjay
curve_bevel.cis used to build the coordinates for a single section of the curve's geometry.BKE_curve_bevelList_makesamples the curve locations, anddo_makeDispListCurveTypesuses that information to build the 3D mesh.I recommend building a debug build or Blender and stepping through it in a debugger with breakpoints rather than adding print statements everywhere. If you have more specific questions, feel free to ask on https://blender.chat/channel/blender-coders.
I would also recommend looking at uses of
shell_v3v3_normalized_to_distaround the code-base rather than its implementation.@HooglyBoogly Oh okay. That makes sense. This is actually very helpful. Thanks a lot. I'll look into it and get back if I have more questions.
I was unable to fix this issue. Here's what I tried and what I learned, in case it'll help someone else.
Pretty much everything I tried were changes to the file
source/blender/blenkernel/intern/displist.cand within it, almost all of what I'll be writing about is under the functionrotateBevelPiece().rotateBevelPiece()is used to rotate and scale the bevel. I believe thinking about just the bevel pieces at the corners (bends) of the path is sufficient.By default, here's how the pieces are created, according to my understanding:
The curve that is being used as the bevel object is saved inside
dlb->verts, wheredlbis one of the arguments inrotateBevelPiece(). It contains a vector pointing to each point on the curve, relative to the center of the shape. Each of these vectors are rotated using quaternions (mul_qt_v3()) such that it forms the bevel piece in the proper orientation. Next, the x and y values of each of the resulting vectors are multplied byfacand added todata, wheredatacontains the coordinates of the vertices that form the actual curve to which we add the bevel to. Thus, the bevel pieces are formed at each bend and the rest is filled using interpolation.Simply changing the argument
facresults in the entire bevel piece being scaled in all directions equally. This isn't what we want because the scaling should happen only in one certain direction.This direction vector can be identified as the line created by the intersection between the face of the bevel piece and the plane created by the two vectors on either side of the bevel piece (
(bevp -1)->vectobevp->vecandbevp->vecto(bevp + 1)->vec).The factor by which the bevel piece should be scaled in the given direction is
1/cos(theta/2), where theta is the angle between the two vectors on either side of the bevel piece. This also means that the case with theta = 180 has to be dealt with.I wanted to see how a change in the code affects the model so first, I tried changing
facin just one direction for thedatapoints, say the x direction.This resulted in the shape being scaled in the global x direction by a factor of 2. It is impossible (I think) to get the shape we want in this way because the axis that we want to scale in is different.
Next, I tried multiplying
vecbyfacbeforemul_qt_v3.This is when I realized that the rotation of the bevel piece changes in an unpredictable (at least to me) manner.
As seen in the picture, the bevel at the top end is slightly rotated even though the entire curve is made of 90 degree angles, which made no sense to me. I checked it again with the original code and that was still the case.
There were other things that I attempted but I doubt knowing them would be of any use. Hopefully, this'll help someone to figure out what's going on and how to deal with it.
The manual way to do this is to treat all segments separately, extend the geometry extruded on each segments and cut at the intersections. Maybe you could try this approach. I don't know if that helps.
@Funnybob Hi. Thanks for the tip. It gave me an idea. I'll give it a try.
Added subscriber: @Spacerfrance
Hi, am not a developer, but wanted to know if this is fixed or a workaround available? Thanks - Peter
Nope, not yet. This task will be closed when this is fixed.
Added subscriber: @patjan
A solution is proposed in D9678
Added subscriber: @sayak_adak
Hello everyone, I am totally new to open source contribution, I have been using blender for sometime now and have a basic idea about python and C++, can anybody kindly tell me which skills to acquire and from where so I can start contributing?
Added subscriber: @rjg
@sayak_adak Take a look at our [wiki ]]. For further questions I would suggest you join [ https:*blender.chat/home | blender.chat and contact me there.
I, too, have a proposed solution at D9684: Fix #80979: Fixed the bevel distortion issue at bends. Would appreciate if someone can take a look.
Added subscribers: @Joe_W, @mano-wii
You know, I logged the same bug for Maya in 2000 and again in 2017 (but in Maya the bug is only when you extrude in the Z axis) The big is still there. Now you know why I ditched Maya for Blender. Here, we already have two solutions. You guys rock!
I have updated my fix D9678 which should no more produce distortions in the bevel. I have tested it with many files and could not identify any issue. It even fixes the issues with the distortion in the attached file (I am not sure in which issue I found this file) Curve Geometry Bug.blend
It would be nice if someone could test it and provide any feedback.
The blend file I have upload is take from #83207.
If anyone is interested in looking into this, the place to look is
fill_mesh_positionsincurve_to_mesh_convert.ccBevel on curves created distorted geometryto Account for curvature in curve to mesh nodeAdded subscriber: @SHREY-AGGARWAL
Added subscriber: @genesis2303
Added subscriber: @Syscrusher
Added subscriber: @BlenderBruno
Alright I have attempt N+1 uploaded in D16076. TLDR the approach is to scale the profile in the direction of the curvature, so that the newly formed mesh line would stay parallel to the original curve segment.
Before:
After: 
Before (Y axis view):
After: 
D16076 currently applies the stretching to all curve_to_mesh calls, but I wonder if it's useful to add a toggle to opt in / out of this behavior, and have it default to opt in.
For reference Houdini appears to provide a "stretch around turns" toggle (that defaults to on), and a "max stretch" parameter.
Hi Shan Huang, thx for continuing to work on this. Part of the UI toggle approach has been discussed in D9678 to use "
Use Consistent Thickness" as description term but it might be good to run it again through @HooglyBoogly and @mano-wii?Personally I have been tinkering with it as well (you beat me to it 😉 ) to get familiarize with the code and for the purpose of experimenting with different approaches I have simply created a "Corner Method" attribute to the Curve to Mesh node which allows comparing between them. The final UI should still probably be a check box. This is now trivial to setup within
node_geo_curve_to_mesh.ccand pass a parameter tocurve_to_mesh_swep(), down tofill_mesh_positions()Here is my test blend file, let me know if you can use it (though it is using my "Corner Method" node param).
curve to mesh profile testing.blend
*the screenshot shows the original method, your previous version and an experimental projection method
I will continue to test your versions and will review the code (I haven't tested the diff you just uploaded in D16076) also, let me know if I can help you otherwise.
@Shan-Huang and @BlenderBruno, thanks for taking a look at this.
There's definitely room for experimentation here, thanks for the testing.
@Shan-Huang Could you remove the screenshot from Houdini in your comment? There are copyright issues with referring to proprietary software directly here (more info here: https://devtalk.blender.org/t/copyright-guidelines-for-devtalk/17331)
Was there an issue with Patjan's fix? Can't we just use it?
Hi Robert, Patjan's fix was done with the legacy Curve bevel implementation. As per Hans Goudey, the task of fixing this issue is now focusing on the new Geometry Node "Curve To Mesh" which has a newer, better curve underlying implementation which will eventually replace the old one.
Hi All, Feel free to check out and review my patch D16685 that implements
Consistent ThicknessforGN/Curve to Mesh. I'll adding some more info and test reports as we go.@BlenderBruno I understand about the geo node but if I'm modeling a building and I need to extrude a profile on top of it, I'm certainly not going to use geo nodes for that. It would be super inefficient. That's why I think Patjan's solution should ALSO be applied. Extrude is something we do all the time in architecture. I don't want Blender to become like Houdini where you need a dozen nodes to do something you would normally do in a few clicks. Geo nodes are not a solution for everything and it will never replace traditional hard surface modeling.
@Funnybob, I get it. I will let the module devs comment but I guess the idea is not to force the User (you!) to use GN for handling curves. It means just to eventually replace the internal legacy Curves implementation with the new one used in GN (new hair system etc..) while keeping the same UI. It happens that we are at the transition point now so from a development perspective, better focus improvements on the new Curves. If not, we would have to maintain 2 curves code base: not practical.
Oh and just so you know that's my first contribution attempt to Blender and I wouldn't have attempted to tackle this with the old curve code. I have scratched my head quite a bit trying to make sense of it and in comparison the new one is kid's play, kudos to Hans and team!
Sorry about that! Removed. Thanks for sharing the info on copyright. Is it OK in general to reference Houdini's behavior in words though?
Thanks so much for the help on testing! I'd say that the twisting around corners is somewhat working as intended. The current curve to mesh implementation (without any special treatment for turns) naturally introduces a small amount of twist around corners. I tried running curve_to_mesh node on the maze you provided. Please see the below screenshot. The cross section of the first turn is already not perfectly aligned to the 45 degree tangent plane:
I believe the twist is introduced in:
bke::curves::poly::calculate_normals_minimum, when the curve normals are computed withNORMAL_MODE_MINIMUM_TWISTmode.Nice approach (and super nice math diagram too!). I feel like our approaches are conceptually similar. Both are trying to keep the newly formed edges parallel to the base curve edge. I think ignoring twist might be too significant of a drawback for a workable solution though. Nullifying extrusion after a zero radius can be limiting too. For robustness I think it's worth finding an approach where the cross sections can be computed independently for each cross section (which is what D16076 tries to do :)), instead of relying on results from previous cross sections.
Thx @Shan-Huang , I agree that the tilt and radius limitations are a problem. Got a few ideas to work them out.
Now that I have managed to somewhat refresh memory with linear algebra and after reviewing your approach, I understand that while performing the scaling correction, it still relies on the default
point_matrixbased on the evaluated normal which changes whether usingMinimum TwistorZ-Upmethods, thus twisting the profile accordingly at the corners for 3D curves. That may be a perfectly acceptable result...Thanks to both of you for looking into this.
I think the rotation of the profile at each evaluated point should be consistent with the normal. There's some discussion of that in #100859, but I think it's a more general problem and projection might not be the right solution. My opinion isn't that strong though.
It's nice for performance as well, since it allows easier parallelism within a curve.
The issue I see with D16076 is the number of matrix inversions and Euler conversions. Don't have performance numbers to back myself up, but I would imagine that has a noticeable performance cost.
Thanks @HooglyBoogly for taking time to check it out.
Isn't the expectation here and in most related issues of obtaining a straight "90deg" look? Here are a few examples:
{F14004136, layout=center, width=50%}
{F14004137, layout=center, width=50%}
{F14004140, layout=center, width=50%}
If I am not mistaken, this is not achievable if the profiles are dependent on the Normals because they do not always line up with the curve segments "straightness". In other words, today, a normal does not necessarily fall in the plane formed by 2 curve segments (even ones at 90deg). As pointed out by @Shan-Huang, I believe this is caused by the current
minimum twistevaluation, right? Here is the example of #81071, where the four bottom normals of this cubic curve (as reported by GN) are pointing toward the center:{F14004148, layout=center, width=50%}
I am not saying that the projection approach is the right one but may be the requirements need to be refined and formalized?
Also, that sounds like the perfect type of feature to have some associated test blend file a with a set of curves, profiles and expected result meshes. To be added to the test suite, CI etc... Is there a formal way of doing this?
Hi All, just published D16829 to address this (improved from D16685).
Reviews/Comments appreciated!
... and Happy Holidays!
So... this is all very nice and appreciated but it fixes a geo node problem, not the original one, a modeling problem. Who makes the call about also fixing it in the modeling? And we had a working solution too. :-)
Added subscriber: @sozap
@Funnybob , as @BlenderBruno explained it's not only about geo nodes : these changes will benefit also the "regular" 3D curves objects once the new system replace completely the legacy one.
For users the tools won't changes, we'll just benefits the same improvements being geo nodes or regular modeling. And just like now we can't really tell what system is being used under the hood.
From an user POV it might sound silly to not use the currently available patch ( for the legacy system ) since it's available.
But I also totally understand the developers logic : since it's going EOL investing time on that is very questionable, and there are no guarantee that just applying the patch won't create some new bugs that will need to be fixed,
slowing down the switch to the new implementation even more.
Furthermore, given this is not a bug and just how blender works since forever it makes sense to keep that improvement for later. I agree that this fixes a longstanding issue and it's how it should be by default, and I do too have been annoyed many times with that...
There are some geo nodes setups available that fixes that issues in the meantime. Really not ideal, but if you want a quick solution I'll go that route, and at some point this workaround won't be needed anymore !
Understood. Thanks :-)
Added subscriber: @Dominique-Herrmann
Hello,
I have previously modeled in 3Ds Max. And Spline Modelling is pretty common there.
Starting with box modelling and then for some selected edge lines you convert them to a Spline (in blender Curve),
And use the spline object with the bevel option, e.g. as cornice or cable.
I've really wondered why that does not work well in blender, as the profile / cross section is uneven.
As you can see here, it's because this task is over 2 years old and not an easy solution.
But I hope its added soon, because Spline/Curve Modelling is pretty useful.
Btw, where is the Geometry Node solution downloadable?
You're definitely right that the requirements aren't totally clear. I'm struggling a bit with lack of experience in this area but also a desire for consistency between normals and the curve to mesh node, short term usability and longer term consistency and performance. That all adds up to making a decision a bit hard as you could imagine!
Totally agreed! Geometry nodes tests are quite easy to add, there's some information here: https://wiki.blender.org/wiki/Tools/Tests/GeometryNodesTests One limitation right now is that only mesh objects are supported, but that can be worked around with the object info node.
@Dominique-Herrmann : https://blenderartists.org/t/z-curve-sweeper-for-blender-3-0-curve-to-mesh-with-superpowers/1365277
I think I've seen other setup, this one is very complete with a lot of features !
Added subscribers: @JacquesLucke, @dfelinto, @pablovazquez
Thanks @HooglyBoogly
Understood!
Now I don't want to drag the entire Blender team around this but could someone else help with making the decision? @dfelinto, @JacquesLucke, @pablovazquez? Or could it be brought up at the next GN meeting?
Meanwhile, let me call for comments and requirement details from previous bug reporters.
Thanks, I will take a look.
Added subscriber: @MaciejMorgas
Added list of related public reports about curve extrusion
Uneven Thicknesshere: P3442