WIP: Sculpt: Add increase / decrease visibility operator #120282
No reviewers
Daniel Bystedt
Hans Goudey
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
7 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#120282
Loading…
Reference in New Issue
No description provided.
Delete Branch "Sean-Kim/blender:hide-show-grow-shrink"
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?
This PR adds a new operator:
PAINT_OT_visibility_editto support iteratively expanding or shrinking the visibility of a mesh, similar to the Grow / Shrink Mask actions and the Grow / Shrink Face Set options. This operator is exposed via two new entries in the Sculpt toolbar entry as Show More and Show Less and have also been assigned to Page Up and Page Down in the default Blender keybinds for Sculpt Mode.Revision History
Technical Details
Each of the PBVH types is solved slightly differently, though the general principle for each is as follows:
Limitations
paint_hide.ccfile. This new operator is limited to Sculpt mode only.I'm still in the process of tidying the naming up for these functions, but otherwise I think this PR is in a valid state to be reviewed.
Based off of this RCS request.
I would add the property in "Options" (header toolbar, Tool in Properties) that defines the "Step" of increase/decrease. Similar to how rotation increment was added for snapping recently in object mode.
@blender-bot package
Package build started. Download here when ready.
pretty cool. but why not call it "grow visibility" / "shrink visibility" to keep it consistent with the grow/shrink of mask and face sets?
i believe thats how this feature is normally called in other sculpting apps as well
That’s valid feedback - I found “Grow Visibility” & “Shrink Visibility” to be more awkward sounding personally, but conforming to existing nomenclature is probably worth doing here.
Looks like buildbot failed. Is it possible to get out a new build or is a rebase required @HooglyBoogly @Sean-Kim?
Once there is a functioning build I can test the feature.
The error the windows build gave is an internal compiler error, I think kicking off a new attempt should fix it but I’ll rebase soon just to eliminate that as a potential error.
@blender-bot package
Package build started. Download here when ready.
Seems pretty reasonable, I'm sure this will be helpful for people. I don't fully understand the "duplicates" part of the multires neighbors system, I have to look at that to be more confident here..
@ -719,0 +730,4 @@const Span<int2> edges = mesh.edges();bke::MutableAttributeAccessor attributes = mesh.attributes_for_write();if (!attributes.contains(".hide_vert")) {// If the entire mesh is visible, we can neither grow nor shrink the boundary.Comment style
@ -719,0 +741,4 @@bool any_changed = false;const bool desired_state = !(mode == EditMode::Grow);threading::parallel_for(edges.index_range(), 1024, [&](const IndexRange range) {I think ideally this would be implemented by iterating over faces and face corners instead of edges. Reasoning is a long term goal of mine to reduce use of edges much as possible with the ultimate goal of making them unnecessary to store.
For Blender's
Mesh, the only fundamental reason we need to store an array of edges and corner edge references is because we have loose edges. Otherwise edges could just be defined implicitly by the rest of the topology.Since edges have very large memory requirements and aren't used much elsewhere in this code, not using them here would help to keep the arrays out of CPU caches as well.
Ah, I think I see how to do this from the documentation - Using
mesh::face_corner_prev/mesh::face_corner_next, right?Right, that should work!
@ -719,0 +743,4 @@const bool desired_state = !(mode == EditMode::Grow);threading::parallel_for(edges.index_range(), 1024, [&](const IndexRange range) {for (const int i : range) {const blender::int2 edge = edges[i];blender::should be unnecessary since this is in that namespace already@ -719,0 +805,4 @@BKE_subdiv_ccg_neighbor_coords_get(subdiv_ccg, coord, true, neighbors);bool should_scan_dup = true;int dup_idx_start = neighbors.coords.size() - neighbors.num_duplicates;for (int i = 0; i < dup_idx_start; i++) {for (const int i : neighbors.coords.index_range().drop_front(neighbors.num_duplicates))?@HooglyBoogly - To give a bit more of my reasoning around the duplicates bit of the multires code, it was based on my assumption that to retrieve all neighbors / adjacent elements for a particular grid coordinate to propagate the hidden attribute, you need to request duplicates from the
BKE_subdiv_ccg_neighbor_coords_getfunction in cases where the vertex being inspected is on a coarse edge or vertex.There's a bug somewhere in the current implementation when there's more than one PBVH node, I'm not yet sure exactly what's causing it.Edit: Found the bug.
When I tested this build, I noticed that it is only possible to grow/shrink visiblity when the mesh is not using the multires modifier.
My suggestion would be to add functionality so the operator works with the multires modifier.
Another thing that could add usability is a integer multiplier in the operator for multiplying the amount of steps of grow/shrink visibility
while multires support would be great, why not make it a separate improvement/PR?
this is already very useful as it is. i dont see the need to block this just because of that.
in the meantime, the operators could be greyed out when using multires.
i agree on the multiplier
There is a performance downside to this sort of "Repeat" slider in the redo panel. Because the entire operation is redone from scratch, there is a lot of extra work happening. For example, when you change the repetitions from 100 to 101, it doesn't just run the operation 1 more time, it runs 101 times from scratch.
This may not be a bottleneck for relatively smaller meshes, but when there are millions and millions of faces involved I would imagine it would have a noticeable effect.
Aside from changing how the redo panel works in general, the alternative could be to make this a modal operator that depends on the mouse cursor like the existing sculpt filters.
As i can see, this is the reason why geodesic selection operator peform computation of distance field at first, and after this just perform selection on this field each time you change selection range.
Regarding the multires functionality, I thought I had it working in the build before requesting review initially, but a change I did in the meantime must have broken it. I'll take a look at it again.
As for the "step" size, currently this operator mimics the Grow / Shrink Mask and adds an extra step for every 50k verts. I recently looked at the other similar operators (Mask & Face Set) to see if there was a common way of exposing this larger step size, but didn't find anything cohesive other than the modal controls. I'll spend a bit more time thinking about this and exposing a few different options in the PR.
Sculpt: Add increase / decrease visibility operatorto WIP: Sculpt: Add increase / decrease visibility operatorCheckout
From your project repository, check out a new branch and test the changes.