Curves: cage overlay for sculpt mode #104467
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
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
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#104467
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "JacquesLucke/blender:sculpt-edit-overlay"
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 adds a new overlay for curves sculpt mode that displays the curves that the user currently edits. Those may be different from the evaluated/original curves when procedural deformations or child curves are used.
The overlay can clash with the evaluated curves when they are exactly on top of each other. There is not much we can do about that currently. The user will have to decide whether the overlay should be shown or not on a case-by-case basis.
This changes some naming from
data
andcolor
to the more specificselection
. This makes it easier to shareGPUVertBuf
between different shaders.Thanks for the adjustments! I think it's looking pretty good now.
I do see an issue how the cage overlay is colliding with the selection overlay.
In this screenshot they are both on full opacity:
Not sure how easily fixable this is, it's not high priority for me.
It bothers me a little that the default of the selection opacity is 0.75 and the cage opacity 0.7
I think, going 0.75 for both would be better.
MutableMatView
WIP: Curves: cage overlay for sculpt modeto Curves: cage overlay for sculpt modeBesides the patch description, which still needs to be finished ("...Once that is done, I'll do another pass on the code."), I'm missing some of the fundamentals of why the implementation works this way. I wouldn't have expected it to be necessary to create a new curves ID just to draw the cage, I expected all the information to be available on the original curves and the edit hints / deformation created by geometry nodes. That problem seems similar to #104705 BTW.
@ -343,0 +365,4 @@
/* Create cage curves geometry for drawing. */
if (generate_cage) {
const blender::bke::CurvesGeometry &curves_orig = curves_id_orig.geometry.wrap();
Curves *curves_id_cage = blender::bke::curves_new_nomain(curves_id_orig.geometry.point_num,
Why does this have to create a new
Curves
at all? Wouldn't it be enough to use the original curves combined with the deformed positions? At the very least this should be strongly motivated by a comment here.@ -361,0 +379,4 @@
GPU_vertbuf_data_alloc(&point_color_buf, curves.points_num());
blender::uchar4 *data = static_cast<blender::uchar4 *>(GPU_vertbuf_get_data(&point_color_buf));
const blender::VArraySpan<float> selection = curves.attributes().lookup_or_default<float>(
I don't think
VArraySpan
should be used for selections. They are often stored as a single value when everything is selected.Ideally this whole "cage color" upload would be skipped in that case. It's too bad that ends up being a bit complicated sometimes though. But at least the data extraction should support that case.
@ -172,2 +172,4 @@
struct Mesh *editmesh_eval_cage;
/** Evaluated curve cage in edit and sculpt mode. */
struct Curves *editcurves_eval_cage;
Better to combine this with
editmesh_eval_cage
which can be made anID *
.@ -4717,0 +4717,4 @@
prop = RNA_def_property(srna, "sculpt_curves_cage", PROP_BOOLEAN, PROP_NONE);
RNA_def_property_boolean_sdna(prop, NULL, "overlay.flag", V3D_OVERLAY_SCULPT_CURVES_CAGE);
RNA_def_property_ui_text(
prop, "Sculpt Curves Cage", "Show points that are currently being edited");
Show points
->Show curves
(not just showing the points but the segments between them)Maybe add
Show original curves
to make the difference clearer too?This seems to work well in my tests and the code is much more like I had hoped now, nice job.
@ -8,3 +8,3 @@
gl_Position = point_world_to_ndc(world_pos);
finalColor = mix(colorWire, colorVertexSelect, color);
finalColor = mix(colorWire, colorVertexSelect, selection);
It might be nicer if these
color
->selection
renames were committed separately (includingedit_points_data
->edit_points_selection
, etc)? That might make the commit clearer.@ -293,1 +292,3 @@
DEG_id_tag_update(&ob->id, ID_RECALC_COPY_ON_WRITE);
/* Necessary to change the object mode on the evaluated object and to evaluate the cage that is
* edited. */
DEG_id_tag_update(&ob->id, ID_RECALC_COPY_ON_WRITE | ID_RECALC_GEOMETRY);
Not a big deal if you don't know, but isn't
ID_RECALC_COPY_ON_WRITE
redundant afterID_RECALC_GEOMETRY
is added?Seems to work without it now. I'm pretty sure that it was needed at some point. Not sure what changed exactly.
Viclio referenced this pull request2023-03-05 17:48:55 +01:00