Cleanup: Sculpt, use C++ vector types in internal types #114636

Sergey Sharybin · 2023-11-08T17:03:35+01:00

Sergey Sharybin commented

2023-11-08 17:03:35 +01:00

Only converted value types in the structures.

The pointer values are left unchanged as it requires more careful look
to avoid possible alignment mismatch.

Function arguments are left unchanged as well.

Only float[3] is converted as the float[4] will likely need to be
converted to some C++ rotation class. And float[4][4] often did not
compile when change is only done in the header.

Only converted value types in the structures. The pointer values are left unchanged as it requires more careful look to avoid possible alignment mismatch. Function arguments are left unchanged as well. Only float[3] is converted as the float[4] will likely need to be converted to some C++ rotation class. And float[4][4] often did not compile when change is only done in the header.

👍 1

Sergey Sharybin added 1 commit 2023-11-08 17:03:50 +01:00

95c2b28157 Cleanup: Sculpt, use C++ vector types in internal types

Only converted value types in the structures.

The pointer values are left unchanged as it requires more careful look
to avoid possible alignment mismatch.

Function arguments are left unchanged as well.

Only float[3] is converted as the float[4] will likely need to be
converted to some C++ rotation class. And float[4][4] often did not
compile when change is only done in the header.

Sergey Sharybin requested review from Hans Goudey 2023-11-08 17:03:55 +01:00

Sergey Sharybin commented

2023-11-08 17:53:40 +01:00

Unfortunately, there seems to be some artifacts caused by this change. It is a bit hard to reproduce. But is just some quick strokes with a Draw brush.
Investigating it, but if someone sees an obvious issue please let me know :)

P.S. And this time I am more sure it is the source of the artifacts.

Unfortunately, there seems to be some artifacts caused by this change. It is a bit hard to reproduce. But is just some quick strokes with a Draw brush. Investigating it, but if someone sees an obvious issue please let me know :) P.S. And this time I am more sure it is the source of the artifacts.

Screenshot 2023-11-08 at 17.44.55.png

602 KiB

Sergey Sharybin added 1 commit 2023-11-08 18:07:14 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

c80db67bf3 Fix incorrect behavior and bad memory access introduced with the refactor

At least one of the issues was the following code in SCULPT_search_sphere():
  center = data->ss->cache ? data->ss->cache->location : data->ss->cursor_location;

The `location` and `cursor_location` had different type, which is not how the ternary operator could work in this case, so the cursor_location got implicitly converted from float[3] to float3, and then to a pointer.
This lead to center being assigned to a pointer of a temporary object.

Not sure how to fully guard about such error automatically. Solution is to convert even more float[3] to float3.

Iliya Katushenock commented

2023-11-08 18:53:13 +01:00

@Sergey : MSVC (for 95c2b28157)

F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2181): error C2445: result type of conditional expression is ambiguous: types 'blender::float3' and 'float [3]' can be converted to multiple common types
F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2181): note: could be 'blender::float3'
F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2181): note: or       'float *'
F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2721): error C2445: result type of conditional expression is ambiguous: types 'blender::float3' and 'float [3]' can be converted to multiple common types
F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2721): note: could be 'blender::float3'
F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2721): note: or       'float *'
[25/33] Building CXX object source\blender\editors\sculpt_paint\CMakeFiles\bf_editor_sculpt_paint.dir\sculpt_ops.cc.obj

@Sergey : MSVC (for 95c2b28157) ``` F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2181): error C2445: result type of conditional expression is ambiguous: types 'blender::float3' and 'float [3]' can be converted to multiple common types F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2181): note: could be 'blender::float3' F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2181): note: or 'float *' F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2721): error C2445: result type of conditional expression is ambiguous: types 'blender::float3' and 'float [3]' can be converted to multiple common types F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2721): note: could be 'blender::float3' F:\Blender_build\first\blender\source\blender\editors\sculpt_paint\sculpt.cc(2721): note: or 'float *' [25/33] Building CXX object source\blender\editors\sculpt_paint\CMakeFiles\bf_editor_sculpt_paint.dir\sculpt_ops.cc.obj ```

Hans Goudey approved these changes 2023-11-08 22:30:20 +01:00

Hans Goudey left a comment

Looking through the code, definitely much nicer this way!

Sergey Sharybin commented

2023-11-09 09:23:21 +01:00

@blender-bot build

Sergey Sharybin commented

2023-11-09 10:05:03 +01:00

@mod_moder That was fixed with later update.
The build is completed, and no new warnings added. I think it is safe to land.

@mod_moder That was fixed with later update. The build is completed, and no new warnings added. I think it is safe to land.

Sergey Sharybin referenced this issue from a commit

2023-11-09 10:05:32 +01:00

Cleanup: Sculpt, use C++ vector types in internal types

Sergey Sharybin merged commit c3609ec435 into main

2023-11-09 10:05:32 +01:00

Sergey Sharybin deleted branch sculpt_cpp_vector_types

2023-11-09 10:05:34 +01:00

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

Cleanup: Sculpt, use C++ vector types in internal types #114636