Sculpt: Disable sculpt brushes on invisible objects #117746
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
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#117746
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "Sean-Kim/blender:112371-invisible-sculpt"
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 makes the following changes to sculpt mode:
Testing
.blend
fileLimitations
Addresses #112371
Only adding Julien for now, since I'm unsure of the performance impacts of this change in actual usage; if it seems satisfactory I'll look for further reviewers afterwards.
Hm, Juliens original proposal made sense, if object is hidden brushes shouldn't go over it, but accounting for ANY collection visibility is bit strange, because I might have object in separate collections for light linking, compositing, instances for geonodes and etc, which are hidden in the viewport, and I'll have to unhide them to sculpt on object.
Collections are very widely used in all parts of Blender besides just scene organization, and I think we should account for that. Some collections are made to be always hidden and it shouldn't get in the way.
@nickberckley I think I may have misunderstood a comment in the original issue then; I took
To mean that if an object was in any hidden collection at all that it should be filtered out, though I can see why that would be undesirable behavior from a workflow perspective. I'll wait for further clarifying comments and adjust the PR later when I wake up.
@Sean-Kim I would rephrase it then as: "If the object or all collections that the object is linked to are hidden".
Any case where the object is no longer visible in the viewport and therefore should not be editable.
Is it possible to have the brush cursor no longer read the hidden geometry? So the cursor circles would always be view aligned and the target vertex would not be highlighted?
There is generic code for checking effective object visibility in a viewport, it should not be needed to invent anything new for sculpt mode.
For other modes there are iterators like
BKE_view_layer_bases_in_mode_iterator_next
that loop over all visible object in the mode. Since sculpt mode does not yet support multi object editing, probably it should callBKE_base_is_visible
same as that iterator does.Sculpt: Disable sculpting on invisible objectsto WIP: Sculpt: Disable sculpting on invisible objectsMarking this as WIP for now since there's a fair amount of feedback to address.
@JulienKaspar - Makes sense, I'll keep that in mind for how the feature is intended to work. I'll investigate the cursor circle and target vertex as well
@brecht - Thanks for the tip about the methods, completely missed them when searching for something appropriate to use. I saw the comment in the existing code about
#BKE_object_is_visible_in_viewport
not working for sculpt and assumed that was the canonical method to use in this situation.WIP: Sculpt: Disable sculpting on invisible objectsto Sculpt: Disable sculpting on invisible objectsOk, addressed changes so far and updated the description with a new GIF, I think the PR is now in a state where the functionality can be tested @JulienKaspar
Works like intended now. So it's all good on the user side.
@brecht do you mind if I add you as a reviewer since you already took a look at the PR?
@ -1413,0 +1422,4 @@
ViewLayer *view_layer = CTX_data_view_layer(C);
BKE_view_layer_synced_ensure(scene, view_layer);
const Base *base = BKE_view_layer_base_find(view_layer, ob);
Use
CTX_data_active_base(C)
which does the same thing.@ -5636,0 +5639,4 @@
ViewLayer *view_layer = CTX_data_view_layer(C);
BKE_view_layer_synced_ensure(scene, view_layer);
const Base *base = BKE_view_layer_base_find(view_layer, ob);
Use
CTX_data_active_base(C)
which does the same thing.@ -1413,0 +1415,4 @@
/* Ideally, this code would go inside SCULPT_cursor_geometry_info_update when we perform a
* raycast check, however the method is used by many different editors across the entirety of
* sculpt. To mitigate potential unwanted changes of hiding the cursor, the logic is extracted
* here. */
I think this should just be in
SCULPT_cursor_geometry_info_update
, not sure why it shouldn't be.With this only the cursor drawing is changed, but the drawing and actual behavior should be consistent.
Admittedly, I cut the investigation of moving this into
SCULPT_cursor_geometry_info_update
rather short after seeing it referenced from theSCULPT_OT_sample_detail_size
andSCULPT_OT_cloth_filter
operators among others.I was planning on adding the check here but the subsequent modification of some
SculptSession
variables (active_vertex
andactive_face_index
/active_grid_index
) made me hesitant to introduce the changes there out of fear of breaking other operators that may have been dependent on them.I'll do some further investigation later today to see if it actually has any impact or if this change can be moved there, because I do agree that it's currently in a hacky spot.
Made the change, manually tested the various operators that call into the method and didn't find any crashes for them when operating on invisible geometry, though they do still perform the corresponding action.
Sculpt: Disable sculpting on invisible objectsto Sculpt: Disable sculpt brushes on invisible objectsEdited the title of the PR slightly to reflect some findings after testing the most recent change, I'll put this update in #112371, but the summary of it is that the following operators currently still operate on hidden geometry. I think it makes more sense to address those in other PRs instead of potentially cluttering this up with a lot more changes:
SCULPT_OT_expand
SCULPT_OT_trim_box_gesture
SCULPT_OT_trim_lasso_gesture
SCULPT_OT_sample_detail_size
MESH_OT_face_set_extract
SCULPT_OT_mesh_filter
SCULPT_OT_cloth_filter
SCULPT_OT_color_filter
@ -4932,1 +4933,3 @@
if (!srd.hit) {
const View3D *v3d = CTX_wm_view3d(C);
const Base *base = CTX_data_active_base(C);
if (!srd.hit || !BKE_base_is_visible(v3d, base)) {
I think this check should be done earlier, at
if (!ss->pbvh || !vc.rv3d)
?No point doing a raycast on an invisible object.
Oh true, will move this there.