Sculpt: Initialize hairs with custom radius #118339
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
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
Viewport & EEVEE
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
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
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
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
5 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#118339
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "Zyq-XDz/blender:custom-hair-radius"
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?
As #117101 (comment) mentioned, Curves Sculpting will initialize the radius of hair with zero, which is quite inconvenient. So I added the property
curve_radius
toBrushCurvesSculptSettings
. Users could manually modify the initial radius through a slider, as demonstrated below:Without any modification, the initial radius will be 0.01 instead of 0.
test scene: sculpt_add.blend
The alternative solution could simply initialize the radius property in
curves_sculpt_add
andcurves_sculpt_density
. However, I believe users need more options for the radius.Thanks for the PR! My only larger comment is that this should implement the
interpolate_radius
option as well, for consistency with the other settings here, and because it can be an optimization to avoid adding the attribute unnecessarily in some cases. That could probably be turned on by default too.@ -521,7 +521,6 @@ Brush *BKE_brush_add(Main *bmain, const char *name, const eObjectMode ob_mode)
if (ob_mode == OB_MODE_SCULPT_CURVES) {
BKE_brush_init_curves_sculpt_settings(brush);
}
Unrelated change
@ -285,6 +285,14 @@ struct DensityAddOperationExecutor {
selection.finish();
}
if (bke::GSpanAttributeWriter radius = attributes.lookup_for_write_span("radius")) {
This should probably always create the attribute if the
brush_settings_->curve_radius != 0.01f
. Otherwise the setting wouldn't have any effect.@ -286,2 +286,4 @@
}
if (bke::GSpanAttributeWriter radius = attributes.lookup_for_write_span("radius")) {
blender::GMutableSpan radius_span = radius.span.slice(
blender::
should be unnecessary hereAlso, the radius attribute is guaranteed to always be on the point domain, no need to check that here.
add_curves_on_mesh
`` 22e8eec69cHi @HooglyBoogly,
I have implemented
interpolate_radius
inadd_curves_on_mesh.cc
. All functionalities seem to work, but I am not sure I used the right method to initialize the radius.here is an example: sculpt_add.blend
WIP: Initialize hairs with custom radiusto Initialize hairs with custom radiusInitialize hairs with custom radiusto Sculpt: Initialize hairs with custom radiusIt looks like the radius interpolation is more complcated than I expected. The current PR fills the whole curve with the same value. Ideally it would interpolate along the length of the nearby curves-- I think the behavior is unexpected without that.
Let me know if you want to talk that through. Ideally we wouldn't need to sample the nearby curves more times for this, but the sampling factors/indices would be reused.
@ -200,6 +200,9 @@ class CurvesGeometry : public ::CurvesGeometry {
Span<float3> positions() const;
MutableSpan<float3> positions_for_write();
Span<float> radiuses() const;
We've avoided adding these so far, because there are some places that use the wrong defaults for when the attribute doesn't exist (basically it's just more complicated than it should be because of some questionable decisions from a while ago).
It should be fine to just use the attribute API to access the data instead.
Seems like the code should create the radius attribute if it doesn't exist. Otherwise the behavior is kind of unexpected. It's weird to set a radius that is then not used.
Will leave the rest of the review to Hans who might want to do some deduplication in this area later.
Instead, I guess we can skip the interpolation if the attribute doesn't exist. Since the default value will still be fine. If interpolation is turned off though, the attribute should be added, yes.
@ -236,13 +236,99 @@ static void interpolate_position_with_interpolation(CurvesGeometry &curves,
});
}
static void interpolate_radius_without_interpolation(CurvesGeometry &curves,
"do thing without doing thing" is a confusing name! How about "calc_radius_without_interpolation"?
@ -239,0 +267,4 @@
bke::SpanAttributeWriter radius_attr =
curves.attributes_for_write().lookup_for_write_span<float>("radius");
MutableSpan<float3> positions_cu = curves.positions_for_write();
MutableSpan<float> radiuses_cu = radius_attr.span;
Spelling:
radiuses
->radii
@ -239,0 +279,4 @@
const float root_cu = root_radiuses_cu[i];
if (neighbors.is_empty()) {
/* If there are no neighbors, just using uniform radius. */
If there are no neighbors, where is
root_radiuses_cu
coming from?@ -350,0 +439,4 @@
const bool has_radius_attr = !radius_attr.span.is_empty(); // check if the attribute exists
Array<float> root_radius_cu(added_curves_num);
if (inputs.interpolate_radius && has_radius_attr) {
interpolate_from_neighbors<float>(
It's not clear why this is necessary. The radius of the first point of a curve shouldn't be special, since we're interpolating the radii from all points at factors along neighboring curves. Am I missing something?
Now
interpolate_radius
will always be on which means the radius attribute will not be added if users are adding curves. But once users turn off the interpolation, the radius attribute will be added with default value.Only blender organization members with write access can start builds. See documentation for details.
Thanks! This works nicely in some simple tests. Just a few comments inline.
@ -239,0 +245,4 @@
bke::SpanAttributeWriter radius_attr =
curves.attributes_for_write().lookup_or_add_for_write_span<float>("radius",
bke::AttrDomain::Point);
threading::parallel_for(IndexRange(added_curves_num), 256, [&](const IndexRange range) {
This can be replaced with
radius_attr.span.slice(new_points_range).fill(radius);
@ -239,0 +287,4 @@
continue;
}
radii_cu.slice(points).fill(0);
0
->0.0f
@ -239,0 +292,4 @@
for (const NeighborCurve &neighbor : neighbors) {
const int neighbor_curve_i = neighbor.index;
const IndexRange neighbor_points = points_by_curve[neighbor_curve_i];
const float3 &neighbor_root_cu = positions_cu[neighbor_points[0]];
neighbor_root_cu
is unused@ -657,2 +657,2 @@
BRUSH_CURVES_SCULPT_FLAG_INTERPOLATE_SHAPE = (1 << 3),
BRUSH_CURVES_SCULPT_FLAG_INTERPOLATE_POINT_COUNT = (1 << 4),
BRUSH_CURVES_SCULPT_FLAG_INTERPOLATE_RADIUS = (1 << 3),
BRUSH_CURVES_SCULPT_FLAG_INTERPOLATE_SHAPE = (1 << 4),
Best not to change these values, since they're stored in files. You can just add the new one at the end
@ -239,0 +240,4 @@
const IndexRange new_points_range,
const float radius)
{
const OffsetIndices points_by_curve = curves.points_by_curve();
points_by_curve
is unused@ -239,0 +260,4 @@
curves.attributes_for_write().lookup_for_write_span<float>("radius");
if (!radius_attr) {
radius_attr.finish();
No need to call
finish()
in this case :)I'm bit late to party, but I can see the 'fallback_curve_radius' is by default set to 0.
This mean that first new strands will be added with radius attribute of 0? I would imagine, initializing radius to 1 would be way more convenient for users. @Zyq-XDz
I mean the main issue in #117101 - was, that if there was no strands and (or) no radius attribute, the new strand gets initialized with radius 1.
Thanks, committed the missing versioning here:
d6f76c0889
Hi @JoseConseco, thanks for your idea,
To prevent extra memory usage, if adius attribute does not exits at beginning, adding curves will not add this new attribute.
If the radius attribute exists (or
interpolate_radius
is off), the radius will be initialized with custom radius, and its default value is0.01m
not 0. The default value is 0.01 instead of 1 is because the unit of radius is meter. I believe it will be better to initialize with a plausible value. 😊