Curves: cage overlay for sculpt mode #104467

Merged
Jacques Lucke merged 44 commits from JacquesLucke/blender:sculpt-edit-overlay into main 2023-02-14 18:10:24 +01:00
Member

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.

image

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 and color to the more specific selection. This makes it easier to share GPUVertBuf between different shaders.

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. ![image](/attachments/d62f6f48-28eb-49e4-b514-bd78cf7d0ac2) 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` and `color` to the more specific `selection`. This makes it easier to share `GPUVertBuf` between different shaders.
Jacques Lucke added 23 commits 2023-02-08 14:32:48 +01:00
Jacques Lucke requested review from Simon Thommes 2023-02-08 14:35:05 +01:00
Jacques Lucke requested review from Hans Goudey 2023-02-08 14:35:11 +01:00
Jacques Lucke added 1 commit 2023-02-08 14:39:03 +01:00
Hans Goudey added this to the Nodes & Physics project 2023-02-08 14:46:23 +01:00
Hans Goudey added the
Interest
Geometry Nodes
label 2023-02-08 14:46:29 +01:00
Simon Thommes requested changes 2023-02-09 14:58:05 +01:00
Simon Thommes left a comment
Member

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:
image
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.

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: ![image](/attachments/49c3328f-4010-4bd5-9093-ff0b664d3493) 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.
Jacques Lucke added 23 commits 2023-02-09 14:59:20 +01:00
buildbot/vdev-code-daily-coordinator Build done. Details
a1282ab015
Fix Cycles debug build error after host falback changes
Introduced in dcfb6df9ce6.

Co-authored-by: Lucas Tadeu Teixeira <lucas@lucastadeu.com>

Pull Request #104454
buildbot/vdev-code-daily-coordinator Build done. Details
0ab3ac7a41
BLI: Math: Fix vector operator * with `MutableMatView`
This was caused by operator priority trying to use
`friend VecBase operator*(const VecBase &a, FactorT b)`.

Adding tests as these were not covered.
buildbot/vdev-code-daily-coordinator Build done. Details
a0f5240089
EEVEE-Next: Virtual Shadow Map initial implementation
Implements virtual shadow mapping for EEVEE-Next primary shadow solution.
This technique aims to deliver really high precision shadowing for many
lights while keeping a relatively low cost.

The technique works by splitting each shadows in tiles that are only
allocated & updated on demand by visible surfaces and volumes.
Local lights use cubemap projection with mipmap level of detail to adapt
the resolution to the receiver distance.
Sun lights use clipmap distribution or cascade distribution (depending on
which is better) for selecting the level of detail with the distance to
the camera.

Current maximum shadow precision for local light is about 1 pixel per 0.01
degrees.
For sun light, the maximum resolution is based on the camera far clip
distance which sets the most coarse clipmap.

## Limitation:
Alpha Blended surfaces might not get correct shadowing in some corner
casses. This is to be fixed in another commit.
While resolution is greatly increase, it is still finite. It is virtually
equivalent to one 8K shadow per shadow cube face and per clipmap level.
There is no filtering present for now.

## Parameters:
Shadow Pool Size: In bytes, amount of GPU memory to dedicate to the
shadow pool (is allocated per viewport).
Shadow Scaling: Scale the shadow resolution. Base resolution should
target subpixel accuracy (within the limitation of the technique).

Related to #93220
Related to #104472
buildbot/vdev-code-daily-coordinator Build done. Details
9c03a1c92f
Fix Cycles link error with debug/asan builds after recent bugfix
Pull Request #104487
buildbot/vdev-code-daily-coordinator Build done. Details
9103978952
EEVEE-Next: Shadow: Fix issue with last merge
The merge with master updated the code to use the new matrix API. This
introduce some regressions.

For sunlights make sure there is enough tilemaps in orthographic mode
to cover the depth range and fix the level offset in perspective.
buildbot/vdev-code-daily-coordinator Build done. Details
94d280fc3f
EEVEE-Next: Shadows: Add global switch
This allow to bypass all cost associated with shadow mapping.

This can be useful in certain situation, such as opening a scene on a
lower end system or just to gain performance in some situation (lookdev).
9fd71d470e PyAPI: minor change to rna_manual_reference loading
- Use bpy.utils.execfile instead of importing then deleting from
  sys.modules.
- Add a note for why keeping this cached in memory isn't necessary.

This has the advantage of not interfering with any scripts that import
`rna_manual_reference` as a module.
buildbot/vdev-code-daily-coordinator Build done. Details
0381fe7bfe
Cleanup: update username in code-comments: campbellbarton -> ideasman42
Gitea migration changed my username, update code-comments.
buildbot/vdev-code-daily-coordinator Build done. Details
3c8f7b1a64
Cleanup: Remove unused/redundant includes from BKE_curves.hh
Avoid including headers that are obviously redundant, and don't
include BLI_task.hh in the header file, since it isn't really related.
buildbot/vdev-code-daily-coordinator Build done. Details
f3d7de709f
Cycles: update Intel Graphics Compiler to 1.0.13064.7 on Linux
Linux side of 8afcecdf1f.

Reviewed by: LazyDodo, sergey, campbellbarton
Ref !104458, 16984
buildbot/vdev-code-daily-coordinator Build done. Details
7effc6ffc4
Cleanup: solve compiler warnings.
Classes were predefined as structs.
buildbot/vdev-code-daily-coordinator Build done. Details
8b35db914e
GPU: Fix assert when using light gizmo.
Blender was reporting that the GPU_TEXTURE_USAGE_HOST_READ wasn't set.
This is used to indicate that the textures needs to be read back to
CPU. Textures that don't need to be read back can be optimized by the
GPU backend.

Found during investigation of #104282.
buildbot/vdev-code-daily-coordinator Build done. Details
f222fe6a3a
Build: enable Python optimizations (PGO & LTO) on Linux
This is used for most Python release builds and has been reported to
give a modest 5-10% speedup (depending on the workload).

This could be enabled on macOS too but needs to be tested.
buildbot/vdev-code-daily-coordinator Build done. Details
0e196bab76
Build: disable LTO for Python builds
LTO compiled libpython3.10.a failed to link with GCC 12.0,
disable since these libraries are intended for developers to link
against.
buildbot/vdev-code-daily-coordinator Build done. Details
ca183993a5
Fix freeing uninitialized pointer in GHOST/Wayland + X11 fallback
Freeing the timer manager didn't account for Wayland being partially
initialized.
Jacques Lucke added 1 commit 2023-02-09 15:39:59 +01:00
Jacques Lucke added 4 commits 2023-02-09 16:12:27 +01:00
Jacques Lucke changed title from WIP: Curves: cage overlay for sculpt mode to Curves: cage overlay for sculpt mode 2023-02-13 15:10:46 +01:00
Hans Goudey requested changes 2023-02-13 19:15:01 +01:00
Hans Goudey left a comment
Member

Besides 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.

Besides 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,
Member

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.

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.
JacquesLucke marked this conversation as resolved
@ -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>(
Member

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.

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.
JacquesLucke marked this conversation as resolved
@ -172,2 +172,4 @@
struct Mesh *editmesh_eval_cage;
/** Evaluated curve cage in edit and sculpt mode. */
struct Curves *editcurves_eval_cage;
Member

Better to combine this with editmesh_eval_cage which can be made an ID *.

Better to combine this with `editmesh_eval_cage` which can be made an `ID *`.
JacquesLucke marked this conversation as resolved
@ -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");
Member

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?

`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?
JacquesLucke marked this conversation as resolved
Jacques Lucke added 3 commits 2023-02-14 14:38:26 +01:00
Jacques Lucke added 5 commits 2023-02-14 17:28:06 +01:00
Jacques Lucke requested review from Hans Goudey 2023-02-14 17:31:18 +01:00
Jacques Lucke requested review from Simon Thommes 2023-02-14 17:31:19 +01:00
Hans Goudey approved these changes 2023-02-14 17:43:31 +01:00
Hans Goudey left a comment
Member

This seems to work well in my tests and the code is much more like I had hoped now, nice job.

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);
Member

It might be nicer if these color -> selection renames were committed separately (including edit_points_data -> edit_points_selection, etc)? That might make the commit clearer.

It might be nicer if these `color` -> `selection` renames were committed separately (including `edit_points_data` -> `edit_points_selection`, etc)? That might make the commit clearer.
JacquesLucke marked this conversation as resolved
@ -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);
Member

Not a big deal if you don't know, but isn't ID_RECALC_COPY_ON_WRITE redundant after ID_RECALC_GEOMETRY is added?

Not a big deal if you don't know, but isn't `ID_RECALC_COPY_ON_WRITE` redundant after `ID_RECALC_GEOMETRY` is added?
Author
Member

Seems to work without it now. I'm pretty sure that it was needed at some point. Not sure what changed exactly.

Seems to work without it now. I'm pretty sure that it was needed at some point. Not sure what changed exactly.
JacquesLucke marked this conversation as resolved
Jacques Lucke added 1 commit 2023-02-14 17:55:10 +01:00
Jacques Lucke added 1 commit 2023-02-14 18:01:08 +01:00
Jacques Lucke merged commit c9f02569c7 into main 2023-02-14 18:10:24 +01:00
Jacques Lucke deleted branch sculpt-edit-overlay 2023-02-14 18:10:25 +01:00
Sign in to join this conversation.
No reviewers
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 Assignees
3 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#104467
No description provided.