Mesh: Replace auto smooth with node group #108014
No reviewers
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
11 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#108014
Loading…
Reference in New Issue
No description provided.
Delete Branch "HooglyBoogly/blender:refactor-mesh-corner-normals-lazy"
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?
Design task: #93551
This PR replaces the auto smooth option with a geometry nodes modifier
that sets the sharp edge attribute. This solves a fair number of long-standing
problems related to auto smooth, simplifies the process of normal computation,
and allows Blender to automatically choose between face, vertex, and face corner
normals based on the sharp edge and face attributes.
Versioning adds a geometry node group to objects with meshes that had auto-
smooth enabled. The modifier can be applied, which also improves performance.
Auto smooth is now unnecessary to get a combination of sharp and smooth
edges. In general workflows are changed a bit. Separate procedural and destructive
workflows are available. Custom normals can be used immediately without turning
on the removed auto smooth option.
Procedural
The node group asset "Smooth by Angle" is the main way to set sharp normals
based on the edge angle. It can be accessed directly in the add modifier menu.
Of course the modifier can be reordered, muted, or applied like any other, or
changed internally like any geometry nodes modifier.
Note: Unfortunately the node group asset can't be published by this PR, this makes testing more difficult, it is attached below
Destructive
Often the sharp edges don't need to be dynamic. This can give better performance
since edge angles don't need to be recalculated. In edit mode the two operators
"Select Sharp Edges" and "Mark Sharp" can be used. In other modes, the
"Shade Smooth by Angle" controls the edge sharpness directly.
Breaking API Changes
use_auto_smooth
is removed. Face corner normals are now used automaticallyif there are mixed smooth vs. not smooth tags. Meshes now always use custom
normals if they exist.
triangulated when all faces are shaded smooth.
auto_smooth_angle
is removed. Replaced by a modifier (or operator) controllingthe sharp edge attribute. This means the mesh itself (without an object) doesn't know
anything about automatically smoothing by angle anymore.
create_normals_split
,calc_normals_split
, andfree_normals_split
are removed,and are replaced by the simpler
Mesh.corner_normals
collection property. Since it givesaccess to the normals cache, it is automatically updated when relevant data changes.
Addons are updated here: blender/blender-addons#104609
Tests
geo_node_curves_test_deform_curves_on_surface
has slightly different results because face corner normals are used instead of interpolated vertex normals.bf_wavefront_obj_tests
has different export results for one file which mixed sharp and smooth faces without turning on auto smooth.cycles_mesh_cpu
has one object which is completely flat shaded. Previously every edge was split before rendering, now it looks triangulated.A good number of our shipped addons make use of all the APIs removed. They'll have to be updated too.
Right, I uploaded my changes to addons here: blender/blender-addons#104609
Is it planned to use autosmooth as a modifier?
WIP: Mesh: Replace auto smooth with node groupto Mesh: Replace auto smooth with node group@ -59,0 +170,4 @@
void do_versions_after_linking_400(FileData * /*fd*/, Main *bmain)
{
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 400, 14)) {
version_mesh_objects_replace_auto_smooth(*bmain);
Creating IDs in versioning code is forbidden. Until a better system (more generic) is implemented, a specific call needs to be added after all versioning is done, see e.g. the call to
BKE_lib_override_library_main_proxy_convert
insetup_app_data()
@ -2078,0 +2197,4 @@
BLI_addtail(&object->modifiers, md);
}
md->settings.properties = bke::idprop::create_group("Nodes Modifier Settings").release();
MOD_nodes_update_interface(object, md);
?Maybe. I'd rather not bring in another "MOD" include to the blenkernel though, theoretically the dependency is supposed to be in the other direction. I'm investigating just importing the asset here anyway, that might simplify things.
@blender-bot build
While I didn't do a detailed review, overall this seems reasonable, only noted minor things in-line.
@ -2819,3 +2819,2 @@
layout.operator("object.shade_smooth")
layout.operator("object.shade_smooth", text="Shade Auto Smooth").use_auto_smooth = True
layout.operator("object.shade_flat", text="Shade Flat")
layout.operator("object.shade_smooth_by_angle")
Shouldn't the
by_angle
operator only be shown for mesh objects? (the operator looks only to deal with meshes).@ -502,0 +470,4 @@
if (domain == ATTR_DOMAIN_FACE) {
mesh_final->face_normals();
}
else if (domain == ATTR_DOMAIN_FACE) {
Shouldn't this be
ATTR_DOMAIN_VERT
?@ -783,0 +849,4 @@
}
}
static void build_edge_to_loop_map(const OffsetIndices<int> faces,
suggestion: this reads like a generic map generation function, it could include that this is for calculating sharp edges.
@ -425,3 +441,1 @@
ME_AUTOSMOOTH = 1 << 5,
ME_FLAG_UNUSED_6 = 1 << 6, /* cleared */
ME_FLAG_UNUSED_7 = 1 << 7, /* cleared */
ME_AUTOSMOOTH = 1 << 5, /* deprecated */
Could call this:
ME_AUTOSMOOTH_LEGACY
Could also rename
Mesh::smoothresh
->Mesh::smoothresh_legacy
.Very nice simplification overall. Just have a few smaller comments.
It would be nice if the node group could already be updated so that it shows in the Add Modifier menu, which is kind of essential before this can be committed.
@ -499,3 +469,1 @@
* (i.e. even if auto-smooth is disabled). */
if (CustomData_has_layer(&mesh_final->loop_data, CD_NORMAL)) {
CustomData_free_layers(&mesh_final->loop_data, CD_NORMAL, mesh_final->totloop);
/* Eager normal calculation can potentially be faster than deferring the to drawing code. */
missing word after
the
@ -2075,0 +2110,4 @@
bNode *store_node = nodeAddNode(nullptr, node_tree, "GeometryNodeStoreNamedAttribute");
store_node->locx = 40.0f;
store_node->locy = 40.0f;
static_cast<NodeGeometryStoreNamedAttribute *>(store_node->storage)->data_type = CD_PROP_BOOL;
Don't use more casts than necessary.
@ -342,1 +417,4 @@
}
BM_mesh_elem_table_ensure(bm, BM_FACE);
const VArrayImpl_For_SharpFace sharp_faces({bm->ftable, bm->totface});
It seems like you could potentially use
VArray::ForDerivedSpan
here.@ -362,0 +366,4 @@
* face corner normals, since there is a 2-4x performance cost increase for each more complex
* domain.
*/
int normal_domain_all_info() const;
I wonder if we could call this
normals_domain
. I found "all info" more confusing than useful at first.@ -2249,3 +2214,2 @@
RNA_def_property_range(prop, -1.0f, 1.0f);
RNA_def_property_float_funcs(
prop, "rna_MeshLoop_normal_get", "rna_MeshLoop_normal_set", nullptr);
RNA_def_property_float_funcs(prop, "rna_MeshLoop_normal_get", nullptr, nullptr);
Think the description below has to be updated.
@ -337,1 +340,3 @@
void mesh_render_data_update_normals(MeshRenderData &mr, const eMRDataType data_flag)
static bool bm_edge_is_sharp(const BMEdge *const &edge)
{
return BM_elem_flag_test(edge, BM_ELEM_SMOOTH);
Looks like there is a "not" missing. Same below.
I have to resolve how "all flat" meshes are dealt with by Cycles, I've asked about that in blender.chat. But those changes should be local to Cycles, and other than that changes mentioned in comments are resolved.
This patch looks good to me. I find the name of the Smooth by Angle node group a bit confusing, because it's not actually smoothing things if everything was flat. I'm contemplating whether the node group should just be called Auto Smooth, because it's design is based on that feature. If we build a Smooth by Angle node from scratch, it would probably have slightly different behavior, or a more obvious name.
Hi @HooglyBoogly!
I tried out the geometry nodes setup
smooth by angle
but it smoothes all edges instead of taking angle into concideration. Perhaps there is code in this PR that I would need to test it? (See attached video)Blender Version
Broken: version: 4.0.0 Alpha,
branch: main,
commit date: 2023-09-10 22:17,
hash:
fca8df9415b1
Other thoughts:
The modifers
WeightedNormal
andNormalEdit
uses the propertyauto smooth
but I can see that you have changed he code in those modifiers. Just want to double check since I'm not familiar with c++.After using
auto smooth
in the object data as Hans suggested I got the geometry nodesmooth by angle
to work.Posting a conversation we had on blender.chat for visibility
Hans Goudey @HooglyBoogly00:12
One option that might be useful for the modifier is something like "Replace" which would make it ignore the existing sharpness and just set everything by angle. But maybe there's other ways to expose that, or something else is more useful
DanielBystedt00:22
That might be useful. I think I would prefer the name
ignore sharp edges though
.Since we currently do not have the option to store edge groups ( like vertex groups), it is sometimes useful to use sharp_edge and crease as a way of processing specific edges in geometry groups. In those cases it could be useful to ignore sharp edges in the node smooth by angle
Hans Goudey @HooglyBoogly00:23
Heh, right, that's painful to read every time someone says it. Though we sort of can do that now, with node group operators 🙂 (or half-way with the "Set Attribute" operator)
Ignoring sharp faces might be included in that too? Basically I'm thinking it might be nice if "Add Cylinder" "Smooth by Angle" worked to give the two rings of sharpness
DanielBystedt00:24
Sounds great to me 🙂
@blender-bot build
@blender-bot build windows
@blender-bot build windows
The test failures were the expected ones, and the Cycles principled test is also failing in main
Ignore Sharp Edges
is very long for the default interface.I propose
Keep Sharp Edges
withTrue
by default:I agree with this as well, seems more clean and I also think its slightly easier to understand what it does. Also it feels a bit weird that output attribrutes and internal dependencies are aviable since they are useles in the context of the modifer and takes a lot of space.
Also, it woulde be cool if theres where an option for overiden custom normals, currently I feel like this gives an unexpected results. Despit the smooth by angle being at the bottom of the modifer stack it still does not sharpen edges at 90 degress.
Is there going to be devtalk feedback topic on that? There are massive regressions both in performance and UX