WIP: Fix #116458: Added decay factor for flattening brushes. #118699

Draft
Raul Fernandez Hernandez wants to merge 87 commits from farsthary/blender:Fix-#116458-Sculpt-Clay-strip-sculpts-on-back-face-when-front-face-only-is-turned-on into blender-v4.1-release

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

Previously the falloff curve was not used for the flattening effect Clay, Clay Strips, Flatten, Fill & Scrape brushes have. The full strength was always used for flattening, resulting in geometry in the outer radius of the brush being pulled just as strong as in the center.

This PR introduce a decay slider to scale the height and depth of the falloff along the Z axis of the area plane. Basically like the "Plane Trim" setting on Flatten brushes, but relative to the brush radius and with a falloff curve instead of as a bool.
That way the ripping of unwanted geometry can be reduced for Clay, Fill and Scrape brushes in the direction that suits them.

Credits:
Julien Kaspar

Previously the falloff curve was not used for the flattening effect Clay, Clay Strips, Flatten, Fill & Scrape brushes have. The full strength was always used for flattening, resulting in geometry in the outer radius of the brush being pulled just as strong as in the center. This PR introduce a decay slider to scale the height and depth of the falloff along the Z axis of the area plane. Basically like the "Plane Trim" setting on Flatten brushes, but relative to the brush radius and with a falloff curve instead of as a bool. That way the ripping of unwanted geometry can be reduced for Clay, Fill and Scrape brushes in the direction that suits them. Credits: Julien Kaspar
Raul Fernandez Hernandez added 4 commits 2024-02-24 16:07:18 +01:00
Raul Fernandez Hernandez requested review from Julien Kaspar 2024-02-24 16:08:18 +01:00
Raul Fernandez Hernandez requested review from Sergey Sharybin 2024-02-24 16:09:28 +01:00
Iliya Katushenock changed title from WIP Fix #116458 Added decay factor for flattening brushes. to WIP: Fix #116458: Added decay factor for flattening brushes. 2024-02-24 16:16:26 +01:00
Sean Kim reviewed 2024-02-25 01:14:10 +01:00
Sean Kim left a comment
Contributor

You'll need to make changes in BKE_blender_version.h to bump the BLENDER_FILE_SUBVERSION value since this is adding a new field to the DNA.

Additionally, even though the plane_trim_decay value here is initialized to 0, I think it's probably best to still add a new block setting the value explicitly to versioning_400.cc for old files being loaded and versioning_defaults.cc for the startup.blend

You'll need to make changes in `BKE_blender_version.h` to bump the `BLENDER_FILE_SUBVERSION` value since this is adding a new field to the DNA. Additionally, even though the `plane_trim_decay` value here is initialized to 0, I think it's probably best to still add a new block setting the value explicitly to `versioning_400.cc` for old files being loaded and `versioning_defaults.cc` for the `startup.blend`
@ -44,6 +44,7 @@
/* How far above or below the plane that is found by averaging the faces. */ \
.plane_offset = 0.0f, \
.plane_trim = 0.5f, \
.plane_trim_decay = 0, \
Contributor

Nitpick: 0.0f instead of 0 to remain consistent with the rest of the file.

Nitpick: `0.0f` instead of `0` to remain consistent with the rest of the file.
farsthary marked this conversation as resolved
@ -3011,6 +3011,15 @@ static void rna_def_brush(BlenderRNA *brna)
"If a vertex is further away from offset plane than this, then it is not affected");
RNA_def_property_update(prop, 0, "rna_Brush_update");
prop = RNA_def_property(srna, "plane_trim_decay", PROP_FLOAT, PROP_DISTANCE);
Contributor

Based on your comment in the chat, I assume you wanted this to be a raw value instead of distance, the subtype here should be PROP_NONE then instead of PROP_DISTANCE

Based on your comment in the chat, I assume you wanted this to be a raw value instead of distance, the subtype here should be `PROP_NONE` then instead of `PROP_DISTANCE`
farsthary marked this conversation as resolved
@ -3014,0 +3017,4 @@
RNA_def_property_ui_text(
prop,
"Plane Trim Decay",
"If a vertex is away from offset plane, its deformation exponentially attenuates");
Contributor

This description is a bit confusing to me from a user perspective. I don't have a good suggestion for rewording it, but I think making it clear that this value is the strength (rate?) of the falloff would be clearer.

This description is a bit confusing to me from a user perspective. I don't have a good suggestion for rewording it, but I think making it clear that this value is the strength (rate?) of the falloff would be clearer.
farsthary marked this conversation as resolved
Author
Member

Thanks for the feedback, fixing asap

Thanks for the feedback, fixing asap
Raul Fernandez Hernandez added 1 commit 2024-02-25 18:47:33 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
b0a8b976b8
feedback
Raul Fernandez Hernandez changed title from WIP: Fix #116458: Added decay factor for flattening brushes. to Fix #116458: Added decay factor for flattening brushes. 2024-02-26 02:26:26 +01:00
Contributor

A few thoughts I had about this recently: Would it make sense to combine this with the normal falloff options that already exist for brushes? Either in reusing the CurveMapping concept and / or in being grouped in the same part of the Falloff panel.

A few thoughts I had about this recently: Would it make sense to combine this with the normal falloff options that already exist for brushes? Either in reusing the `CurveMapping` concept and / or in being grouped in the same part of the `Falloff` panel.

@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/PR118699) when ready.
Member

I think I'm confused what this PR really does.
Based on the description I thought it would be a slider that scales the height/depth of the spherical falloff shape. Instead it's a falloff which alters the behavior of the Plane Trim. Unfortunately I couldn't get a desirable behavior from the Decay setting.

Since it's hard to see and explain based on the Clay brushes, here's the Flatten brush, with a Constant Falloff and Anchored stroke. The Plane Trim is on 0.11 and Decay on 20.
It looks like it adds an additional falloff on top of the hard limit of the Plane Trim, resulting in a stepping effect. That second Decay falloff will always end on the brush radius, so it's not really improving the behavior.
image

Here's a mockup of what I had in mind (Using the Flatten brush with a Constant falloff as an example):
image

Basically by introducing a height and depth setting for the falloff, we could scale the influence in that particular direction. This is specifically useful for Clay, Scrape and Fill brushes because they have a prefered direction of influence.

I'd like to note that the direction of the depth should always be the opposite of the 'direction' setting of the brush (or typically holding Ctrl).

I'm not certain this approach would work but it's better to see after we tried.

I think I'm confused what this PR really does. Based on the description I thought it would be a slider that scales the height/depth of the spherical falloff shape. Instead it's a falloff which alters the behavior of the Plane Trim. Unfortunately I couldn't get a desirable behavior from the Decay setting. Since it's hard to see and explain based on the Clay brushes, here's the Flatten brush, with a Constant Falloff and Anchored stroke. The Plane Trim is on `0.11` and Decay on `20`. It looks like it adds an additional falloff on top of the hard limit of the Plane Trim, resulting in a stepping effect. That second Decay falloff will always end on the brush radius, so it's not really improving the behavior. ![image](/attachments/db4d720e-bddd-454a-a9fd-a86f70220d51) Here's a mockup of what I had in mind (Using the Flatten brush with a Constant falloff as an example): ![image](/attachments/3add07c1-3c8c-4c6f-9d34-8baea321d398) Basically by introducing a `height` and `depth` setting for the falloff, we could scale the influence in that particular direction. This is specifically useful for Clay, Scrape and Fill brushes because they have a prefered direction of influence. I'd like to note that the direction of the `depth` should always be the opposite of the 'direction' setting of the brush (or typically holding `Ctrl`). I'm not certain this approach would work but it's better to see after we tried.
175 KiB
112 KiB
Raul Fernandez Hernandez changed title from Fix #116458: Added decay factor for flattening brushes. to WIP: Fix #116458: Added decay factor for flattening brushes. 2024-02-26 15:45:16 +01:00
Author
Member

thanks for the feedback, In the current PR basically the farthest the vertex is from the sculpt plane (in the sculpt plane normal direction), the weakest it gets pulled. The falloff is exponential, as the other types of curves are "too soft" for preventing outer vertices from getting pulled.
Low decay values 0-10 will give a smooth falloff, while bigger values will act as a harder falloff trim

thanks for the feedback, In the current PR basically the farthest the vertex is from the sculpt plane (in the sculpt plane normal direction), the weakest it gets pulled. The falloff is exponential, as the other types of curves are "too soft" for preventing outer vertices from getting pulled. Low decay values 0-10 will give a smooth falloff, while bigger values will act as a harder falloff trim
Author
Member

The clay strips brush don't have a falloff in Z direction, it uses a cube tests instead of a sphere of influence. I think the decay should be independent of a preexisting falloff shape or curve.

The clay strips brush don't have a falloff in Z direction, it uses a cube tests instead of a sphere of influence. I think the decay should be independent of a preexisting falloff shape or curve.

@farsthary I am a bit confused. Are there changes planned here to accomodate feedback from Julien, or do you believe this PR is ready for review and good to have, even though it might be solving slightly different problem?

@farsthary I am a bit confused. Are there changes planned here to accomodate feedback from Julien, or do you believe this PR is ready for review and good to have, even though it might be solving slightly different problem?
Member

In the current PR basically the farthest the vertex is from the sculpt plane (in the sculpt plane normal direction), the weakest it gets pulled. The falloff is exponential, as the other types of curves are "too soft" for preventing outer vertices from getting pulled.
Low decay values 0-10 will give a smooth falloff, while bigger values will act as a harder falloff trim

Thanks for the explanation!
I think the current settings are not communicating this well.
We should also not tie this to the Plane Trim setting at all. That setting is doing something different and won't combine well with the concept of Decay.

The settings should be descriptive. A "Decay" slider that goes from 0 to 50 is very obscure from the user perspective.
From what you explained I'd expect sliders to control the depth and falloff curve of the "Decay" (Which as I understand refers specifically to the strength of the flattening effect).
I'd even call it "Flatten Depth" and "Flatten Hardness" to be more descriptive and consistent with existing UI terminology.

IMO we should ideally show 0-1 factor sliders.
The depth should also be relative to the brush radius to make it easy to understand. I don't know why a lot of these settings are currently showing meter units ... that could be something to tackle in another PR.

> In the current PR basically the farthest the vertex is from the sculpt plane (in the sculpt plane normal direction), the weakest it gets pulled. The falloff is exponential, as the other types of curves are "too soft" for preventing outer vertices from getting pulled. Low decay values 0-10 will give a smooth falloff, while bigger values will act as a harder falloff trim Thanks for the explanation! I think the current settings are not communicating this well. We should also not tie this to the Plane Trim setting at all. That setting is doing something different and won't combine well with the concept of Decay. The settings should be descriptive. A "Decay" slider that goes from 0 to 50 is very obscure from the user perspective. From what you explained I'd expect sliders to control the **depth** and **falloff curve** of the "Decay" (Which as I understand refers specifically to the strength of the flattening effect). I'd even call it "Flatten Depth" and "Flatten Hardness" to be more descriptive and consistent with existing UI terminology. IMO we should ideally show 0-1 factor sliders. The depth should also be relative to the brush radius to make it easy to understand. I don't know why a lot of these settings are currently showing meter units ... that could be something to tackle in another PR.
Author
Member

@Sergey Yes, I'm planning to include @JulienKaspar feedback in this PR, currently is in WIP since I will be iterating back and forth with him until achieved the expected behavior.

@Sergey Yes, I'm planning to include @JulienKaspar feedback in this PR, currently is in WIP since I will be iterating back and forth with him until achieved the expected behavior.
Raul Fernandez Hernandez added 1 commit 2024-03-02 00:36:13 +01:00
Raul Fernandez Hernandez added 1 commit 2024-03-03 22:51:08 +01:00
Raul Fernandez Hernandez added 1 commit 2024-03-03 23:26:17 +01:00
Raul Fernandez Hernandez added 1 commit 2024-03-04 07:06:05 +01:00
Raul Fernandez Hernandez added 1 commit 2024-03-04 16:35:18 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
2109ac974b
tweak
Author
Member

Cleaned up and incorporated feedback.
Flatten brush in anchored mode is improved a lot!
@JulienKaspar hope this update aligns better with your vision 👍

Cleaned up and incorporated feedback. Flatten brush in anchored mode is improved a lot! @JulienKaspar hope this update aligns better with your vision 👍
Raul Fernandez Hernandez changed title from WIP: Fix #116458: Added decay factor for flattening brushes. to Fix #116458: Added decay factor for flattening brushes. 2024-03-04 16:39:56 +01: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/PR118699) when ready.
Raul Fernandez Hernandez added the
Module
Sculpt, Paint & Texture
label 2024-03-06 01:13:34 +01:00
Member

I tested this for a bit while sculpting objects with varying complexity. The current approach successfully mitigates the effect of sculpting nearby backsides and geometry further behind.

But sadly it also causes some nearby surfaces to also be excluded, if they are too deep behind the sculpt plane or when the sculpt plane orientation causes them to be missed. This happens frequently, especially if the start of the stroke is not deep enough.
I tried configuring the Depth slider to something that works but it is very context dependent on each geometry that is being sculpted.

So I don't think the approach that I suggested will work. I can't think of a better simple solution right now, sorry.
Maybe excluding geometry based on a geodesic falloff is the best approach after all.

I tested this for a bit while sculpting objects with varying complexity. The current approach successfully mitigates the effect of sculpting nearby backsides and geometry further behind. But sadly it also causes some nearby surfaces to also be excluded, if they are too deep behind the sculpt plane or when the sculpt plane orientation causes them to be missed. This happens frequently, especially if the start of the stroke is not deep enough. I tried configuring the Depth slider to something that works but it is very context dependent on each geometry that is being sculpted. So I don't think the approach that I suggested will work. I can't think of a better simple solution right now, sorry. Maybe excluding geometry based on a geodesic falloff is the best approach after all.
Author
Member

Thanks for the testing and the feedback! I will add back the WIP tag and we can revisit again this PR once I implement geodesic distance brush action.

Thanks for the testing and the feedback! I will add back the WIP tag and we can revisit again this PR once I implement geodesic distance brush action.
Raul Fernandez Hernandez changed title from Fix #116458: Added decay factor for flattening brushes. to WIP: Fix #116458: Added decay factor for flattening brushes. 2024-03-08 21:14:48 +01:00
Author
Member

After getting more familiarized with the code, we actually don't need mathematically accurate (and slower) geodesic distance brush influence for sculpting, we can achieve the same results using a fast topological set for the brush's influence.
I'll make the time to revisit and try implementing it soon.

After getting more familiarized with the code, we actually don't need mathematically accurate (and slower) geodesic distance brush influence for sculpting, we can achieve the same results using a fast topological set for the brush's influence. I'll make the time to revisit and try implementing it soon.
Some checks failed
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u Fix-#116458-Sculpt-Clay-strip-sculpts-on-back-face-when-front-face-only-is-turned-on:farsthary-Fix-#116458-Sculpt-Clay-strip-sculpts-on-back-face-when-front-face-only-is-turned-on
git checkout farsthary-Fix-#116458-Sculpt-Clay-strip-sculpts-on-back-face-when-front-face-only-is-turned-on
Sign in to join this conversation.
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
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#118699
No description provided.