Curves: Use deformed points in overlay (crazyspace) #104705

Falk David · 2023-02-13T17:37:08+01:00

Falk David commented

2023-02-13 17:37:08 +01:00

Uses the deformed crazyspace to populate the position buffer for edit mode overlay.

Uses the deformed crazyspace to populate the position buffer for edit mode overlay. ![image](/attachments/eb49ce8e-20d6-4175-9e75-e6cf27d70cb7)

image.png

334 KiB

👍 1

Falk David added 1 commit 2023-02-13 17:37:10 +01:00

1e832832fc Curves: Use deformed points in overlay (crazyspace)

Falk David requested review from Hans Goudey 2023-02-13 17:37:21 +01:00

Falk David added this to the Nodes & Physics project 2023-02-13 17:37:36 +01:00

Hans Goudey approved these changes 2023-02-13 18:51:52 +01:00

Hans Goudey left a comment

Works quite nicely! It's good how simple this part is. There's a bit too much unrelated cleanup going on in this commit IMO, but everything looks reasonable. Just a comment about namespace stuff (some of the unrelated changes). Should also probably get the okay from Jacques about this, since he mainly worked on the curves crazyspace stuff so far.

I'll approve anyway since the changes I mentioned are simple.

BTW, separate topic, but I noticed the transform tools don't work as well as I'd hope with transformed curves. Maybe the transform code needs to know about the original vs. deformed coordinates too.

Works quite nicely! It's good how simple this part is. There's a bit too much unrelated cleanup going on in this commit IMO, but everything looks reasonable. Just a comment about namespace stuff (some of the unrelated changes). Should also probably get the okay from Jacques about this, since he mainly worked on the curves crazyspace stuff so far. I'll approve anyway since the changes I mentioned are simple. BTW, separate topic, but I noticed the transform tools don't work as well as I'd hope with transformed curves. Maybe the transform code needs to know about the original vs. deformed coordinates too.

source/blender/draw/intern/draw_cache_impl_curves.cc

						
				@ -74,6 +75,8 @@ struct CurvesBatchCache {

				  std::mutex render_mutex;

				};

				using namespace blender;

Hans Goudey commented

2023-02-13 18:43:41 +01:00

I'd rather not add using namespace blender; at the file level. Until these files are properly namespaced (blender::draw here), we should add it at the function scope or add using for specific types at the file level. That's basically what we've been doing so far.

I'd rather not add `using namespace blender;` at the file level. Until these files are properly namespaced (`blender::draw` here), we should add it at the function scope or add `using` for specific types at the file level. That's basically what we've been doing so far.

filedescriptor marked this conversation as resolved

source/blender/draw/intern/draw_cache_impl_curves.cc

						
				@ -305,1 +305,3 @@

				                                                      CurvesBatchCache &cache)

				static void curves_batch_cache_ensure_edit_points_pos(

				    const bke::CurvesGeometry &curves,

				    const bke::crazyspace::GeometryDeformation &deformation,

Hans Goudey commented

2023-02-13 18:46:24 +01:00

Maybe it would make sense to pass around just the positions span instead of the whole deformation, since that's all that's really necessary here? That's not a strong opinion though, just a thought.

filedescriptor marked this conversation as resolved

Falk David requested review from Jacques Lucke 2023-02-13 19:09:09 +01:00

Falk David added 1 commit 2023-02-13 19:23:41 +01:00

66b6d2c0cb Cleanup

Falk David added 1 commit 2023-02-13 19:33:18 +01:00

2418400a3e Fix linker error

Jacques Lucke approved these changes 2023-02-14 16:01:47 +01:00

Jacques Lucke left a comment

Got one comment, but other than that looks fine.

source/blender/draw/intern/draw_cache_impl_curves.cc

						
				@ -753,0 +747,4 @@

				  using namespace blender;

				  Curves *curves_id = static_cast<Curves *>(ob->data);

				  Object *ob_orig = DEG_get_original_object(ob);

				  Curves *curves_orig_id = static_cast<Curves *>(ob_orig->data);

Jacques Lucke commented

2023-02-14 16:00:15 +01:00

I'm not sure we can rely on ob_orig to always be non-null for generated curves. So better add a check for that case.

I'm not sure we can rely on `ob_orig` to always be non-null for generated curves. So better add a check for that case.

Falk David added 1 commit 2023-02-14 16:31:17 +01:00

c88db30576 Null pointer check

Falk David merged commit 715acf758c into main

2023-02-14 16:33:49 +01:00

Falk David deleted branch curves-edit-overlay-crazyspace

2023-02-14 16:33:50 +01:00

Falk David referenced this issue from a commit

2023-02-14 16:33:50 +01:00

Curves: Use deformed points in edit mode overlay

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Curves: Use deformed points in overlay (crazyspace) #104705