WIP: Curve-to-Mesh node Even Thickness #108700

Draft
Bruno wants to merge 1 commits from BlenderBruno/blender:tmp-gn-curve-to-mesh-even-thickness-on-main-4.x into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
First-time contributor

Addresses #80979

This commit is the merge with main (4.0.0 at the date of the commit) to revive the patch that was archived as part of the Gitea migration.

See archived patch here: https://archive.blender.org/developer/D16829

It also includes renaming from Miter Joints terms to Even Thickness

Addresses https://projects.blender.org/blender/blender/issues/80979 This commit is the merge with `main` (4.0.0 at the date of the commit) to revive the patch that was archived as part of the Gitea migration. See archived patch here: https://archive.blender.org/developer/D16829 It also includes renaming from `Miter Joints` terms to `Even Thickness`
Bruno added 1 commit 2023-06-07 13:33:46 +02:00
Curve-to-Mesh node Even Thickness feature which addresses #80979
All checks were successful
buildbot/vexp-code-patch-coordinator Build done.
f8478f93c9
(prior to Gitea migration, this patch was https://archive.blender.org/developer/differential/0016/0016829/index.html)

This commit is the merge with `main` (4.0.0 at the date of the commit) to revive the patch that was archived as part of the Gitea migration.

It also includes renaming from `Miter Joints` terms to `Even Thickness`
Iliya Katushenock added this to the Nodes & Physics project 2023-06-07 13:34:03 +02:00
Iliya Katushenock added the
Interest
Geometry Nodes
Interest
Modeling
labels 2023-06-07 13:34:09 +02:00
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR108700) when ready.
Author
First-time contributor

Thanks for the build @HooglyBoogly.

Also, as a reminder, devtalk thread here: https://devtalk.blender.org/t/curve-to-mesh-node-even-thickness-feedback-thread/27271

Thanks for the build @HooglyBoogly. Also, as a reminder, devtalk thread here: https://devtalk.blender.org/t/curve-to-mesh-node-even-thickness-feedback-thread/27271
Member

Thanks for uploading the PR. For this to progress, I think the main change should be splitting out the tilt/rotation changes (which can be considered as a new normal calculation mode in the future), so this PR just changes adjusts the transformation matrices to account for the shear at each point.

Thanks for uploading the PR. For this to progress, I think the main change should be splitting out the tilt/rotation changes (which can be considered as a new normal calculation mode in the future), so this PR just changes adjusts the transformation matrices to account for the shear at each point.
Author
First-time contributor

Thx @HooglyBoogly. Seems like public feedback is also good so far, I'm glad it seems to be down to some implementation tweaks!

Do you already have some some thoughts on the approach for ...new normal calculation mode in the future ?

@mod_moder, feel free to pitch in spam me on the subject as well! :)

Anyway, I'll experiment further...

Thx @HooglyBoogly. Seems like public feedback is also good so far, I'm glad it seems to be down to some implementation tweaks! Do you already have some some thoughts on the approach for ***...new normal calculation mode in the future*** ? @mod_moder, feel free to ~~pitch in~~ spam me on the subject as well! :) Anyway, I'll experiment further...

The main thing I was thinking about is that the curve tilt can be a new kind of field. If we have a built-in tilt (regular Tilt Input node), then there can be other tilt fields as well. They are calculated relative to the built-in one and can optionally be used in various nodes. For example, a corner-specific that preserves slope at corners, like used in this node for save profil.

The reason why I didn't have time is that I planned to test this concept first and make these kind of nodes. But so far, even your node has not been seen, and it is unlikely that there will be time soon ...

The main thing I was thinking about is that the curve tilt can be a new kind of field. If we have a built-in tilt (regular Tilt Input node), then there can be other tilt fields as well. They are calculated relative to the built-in one and can optionally be used in various nodes. For example, a corner-specific that preserves slope at corners, like used in this node for save profil. The reason why I didn't have time is that I planned to test this concept first and make these kind of nodes. But so far, even your node has not been seen, and it is unlikely that there will be time soon ...
Member

Do you already have some some thoughts on the approach for ...new normal calculation mode in the future ?

Sure, currently we have "Z-Up" and "Minimum Twist" (see rna_enum_curve_normal_modes). The changes here could be another mode. Or they could be done as part of the curves custom normals design: #100859. Most important is just to not do it specifically for this node though-- the curve to mesh node should just use the existing normals, consistent with elsewhere, the sample curve node, etc.

> Do you already have some some thoughts on the approach for ***...new normal calculation mode in the future*** ? Sure, currently we have "Z-Up" and "Minimum Twist" (see `rna_enum_curve_normal_modes`). The changes here could be another mode. Or they could be done as part of the curves custom normals design: #100859. Most important is just to not do it specifically for this node though-- the curve to mesh node should just use the existing normals, consistent with elsewhere, the sample curve node, etc.
Author
First-time contributor

Hey Iliya, thx for the reply.

The main thing I was thinking about is that the curve tilt can be a new kind of field. If we have a built-in tilt (regular Tilt Input node), then there can be other tilt fields as well. They are calculated relative to the built-in one and can optionally be used in various nodes. For example, a corner-specific that preserves slope at corners, like used in this node for save profil.

What you describe about the tilt seems to be some sort of a custom normal since the base tilt is by design determined by the normal (later adjusted by the user provided Tilt value). Did I get it wrong? If so, what's the difference with what Hans is describing?

Hey Iliya, thx for the reply. > The main thing I was thinking about is that the curve tilt can be a new kind of field. If we have a built-in tilt (regular Tilt Input node), then there can be other tilt fields as well. They are calculated relative to the built-in one and can optionally be used in various nodes. For example, a corner-specific that preserves slope at corners, like used in this node for save profil. What you describe about the tilt seems to be some sort of a custom normal since the base tilt is by design determined by the normal (later adjusted by the user provided Tilt value). Did I get it wrong? If so, what's the difference with what Hans is describing?

Back then, I just didn't read much into the discussion.

Back then, I just didn't read much into the discussion.
Author
First-time contributor

Do you already have some some thoughts on the approach for ...new normal calculation mode in the future ?

Sure, currently we have "Z-Up" and "Minimum Twist" (see rna_enum_curve_normal_modes). The changes here could be another mode. Or they could be done as part of the curves custom normals design: #100859. Most important is just to not do it specifically for this node though-- the curve to mesh node should just use the existing normals, consistent with elsewhere, the sample curve node, etc.

Understood, thx...

> > Do you already have some some thoughts on the approach for ***...new normal calculation mode in the future*** ? > > Sure, currently we have "Z-Up" and "Minimum Twist" (see `rna_enum_curve_normal_modes`). The changes here could be another mode. Or they could be done as part of the curves custom normals design: #100859. Most important is just to not do it specifically for this node though-- the curve to mesh node should just use the existing normals, consistent with elsewhere, the sample curve node, etc. Understood, thx...
Author
First-time contributor

@HooglyBoogly,

Now I remember why I didn't pursuie the Minimum Twist/Z-Up approach!

The Even Thickness method (refering to the one of this PR only) does not use the existing normals and it requires more than the tangent to calculate the final transformation matrix. So it is not really possible to just have a additional "Normal Mode" without adding extra data.

Even Thickness is not really a normal mode but rather a custom curve profile tranformation method so I am not sure exposing any of the underlying data to the user, like normal, tangent and tilt which are more fundamental, makes sense.

Why not offer a custom profile transformation methods through an additional node similar to the Set Curve Normal node? Something like Set Curve Profile Orientation node. The default mode being Use Normals:

Untitled.png

Or keep it inside the Curve to Mesh node as it may only make sense there:

Untitled 2.png

The former being a separate node from Curve to Mesh would allow for some precalculation which could help with performance. For example, calculate and store the final profile transformation matrix for each main curve point, matrix which would be used directly by the Curve-to-Mesh node.

This approach would also beneficiate from a separate Custom Normal (#100859) as the orientation method could access the custom normal data at the time of computation.

What am I missing?

@HooglyBoogly, Now I remember why I didn't pursuie the `Minimum Twist`/`Z-Up` approach! The Even Thickness method (refering to the one of this PR only) does not use the existing normals and it requires more than the tangent to calculate the final transformation matrix. So it is not really possible to just have a additional "Normal Mode" without adding extra data. Even Thickness is not really a normal mode but rather a custom curve profile tranformation method so I am not sure exposing any of the underlying data to the user, like normal, tangent and tilt which are more fundamental, makes sense. Why not offer a custom profile transformation methods through an additional node similar to the `Set Curve Normal` node? Something like `Set Curve Profile Orientation` node. The default mode being `Use Normals`: ![Untitled.png](/attachments/f2975dde-1516-4bba-a4d3-fae2cb354522) Or keep it inside the `Curve to Mesh` node as it may only make sense there: ![Untitled 2.png](/attachments/abce0181-2369-471f-abd7-256d422eefab) The former being a separate node from `Curve to Mesh` would allow for some precalculation which could help with performance. For example, calculate and store the final profile transformation matrix for each main curve point, matrix which would be used directly by the `Curve-to-Mesh` node. This approach would also beneficiate from a separate `Custom Normal` (#100859) as the orientation method could access the custom normal data at the time of computation. What am I missing?
Member

The behavior might not be the same with these things split apart. I know this would probably be less useful without the orientation changes you've added. But it should be possible to at least adjust the thickness along the axis per-evaluated point without adding a new concept, and I think it's better to start there.

The behavior might not be the same with these things split apart. I know this would probably be less useful without the orientation changes you've added. But it should be possible to at least adjust the thickness along the axis per-evaluated point without adding a new concept, and I think it's better to start there.
Author
First-time contributor

The behavior might not be the same with these things split apart.

Why not?

I know this would probably be less useful without the orientation changes you've added.

I think it will indeed leave out a good portion of the reported issues specially for archviz for which predictable profile size and orientation is important.

But it should be possible to at least adjust the thickness along the axis per-evaluated point without adding a new concept, and I think it's better to start there.

OK then, I'll drop it! I thought I had build a strong case 🤣 but enough beating around the bush on this approach!

The new approach being significantly different, should this PR be closed and a new one started or should I continue here?

> The behavior might not be the same with these things split apart. Why not? >I know this would probably be less useful without the orientation changes you've added. I think it will indeed leave out a good portion of the reported issues specially for archviz for which predictable profile size and orientation is important. >But it should be possible to at least adjust the thickness along the axis per-evaluated point without adding a new concept, and I think it's better to start there. OK then, I'll drop it! I thought I had build a strong case 🤣 but enough beating around the bush on this approach! The new approach being significantly different, should this PR be closed and a new one started or should I continue here?
First-time contributor

where can I download this build? links are not working.
and when it will be in the main branch?

where can I **download this build?** links are not working. and when it will be in the main branch?
All checks were successful
buildbot/vexp-code-patch-coordinator Build done.
This pull request has changes conflicting with the target branch.
  • source/blender/blenkernel/intern/curve_to_mesh_convert.cc
  • source/blender/nodes/geometry/nodes/node_geo_curve_to_mesh.cc

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u tmp-gn-curve-to-mesh-even-thickness-on-main-4.x:BlenderBruno-tmp-gn-curve-to-mesh-even-thickness-on-main-4.x
git checkout BlenderBruno-tmp-gn-curve-to-mesh-even-thickness-on-main-4.x
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
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
Asset Browser Project
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 Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#108700
No description provided.