Implementation of T71948
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- scupt-pose-brush-ik (branched from master)
- Build Status
Buildable 6187 Build 6187: arc lint + arc unit
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
| source/blender/editors/sculpt_paint/sculpt.c | ||
|---|---|---|
| 3582 | Split this function in two separate functions. We only call displacement or rotation at a single time. | |
| 3609 | add const where needed. (this needs to be done for the whole patch). adding more keywords makes code more readable. | |
| 3609 | remove the i_ and just use a different name for the local copy. | |
| 3609 | use lowercase names for function names. Except when indicating a module public function | |
| 3678–3679 | is val modified at all or is this to fix potential threading issues? | |
| 3681 | Possible index out of bound | |
| 3682 | Don't use char | |
| 3748 | copy_qt_qt? | |
| 3748 | unneeded quat_to_mat4 writes a complete mat4 | |
| 3767–3790 | unneeded copies_m4_m4. | |
| 3827–3828 | use MAX2 | |
| 4053–4054 | 0.0f | |
| 4064 | unneeded float conversion. better to let the compiler decide. Not sure if FIDIV instruction will be selected by all compilers :-) | |
| 4069–4070 | tot_segments | |
| 4137 | Can we use .orig directly and not use the tmp variable. | |
| 5967 | add TODO(pdobarro): if something is a todo item | |
| source/blender/editors/sculpt_paint/sculpt_intern.h | ||
| 85 | tot_segments | |
| 391 | put pose_ik_chain_len and pose_ik_chain in a PoseIKChain struct. | |
| source/blender/makesrna/intern/rna_brush.c | ||
| 1934 | Add a description | |
Comment Actions
Think we are almost done, just some small clean up/code readability.
| source/blender/blenkernel/BKE_paint.h | ||
|---|---|---|
| 106 | This is codewise not part of the enum, You could add it as a static const. | |
| source/blender/editors/sculpt_paint/sculpt.c | ||
| 1338 | the flip_qt_qt uses ePaintSymmetryFlags we should clean up replace all const char symm parameters with a const ePaintSymmetryFlags symm. | |
| 3724 | don't use char | |
| 3752 | Doesn't need to be casted to char. | |
| 10371 | unneeded cast to char in this function. Would it make sense to change int i to ePaintSymmetryAreas symm. Or convert the i inside the brackets to ePaintSymmetryAreas symm. The problem with casting to char is that when we want to add more stuff to the ePaintSymmetryAreas, the char might trigger undesired results that are hard to track. | |
Comment Actions
Compilation Warning
[422/799] Building C object source/blender/editors/sculpt_paint/CMakeFiles/bf_editor_sculpt_paint.dir/sculpt.c.o
/home/jeroen/blender-git/blender/source/blender/editors/sculpt_paint/sculpt.c: In function ‘sculpt_pose_ik_chain_init’:
/home/jeroen/blender-git/blender/source/blender/editors/sculpt_paint/sculpt.c:4092:64: warning: passing argument 3 of ‘sculpt_nearest_vertex_get’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
int nearest_vertex_index = sculpt_nearest_vertex_get(sd, ob, initial_location, FLT_MAX, true);
^~~~~~~~~~~~~~~~
/home/jeroen/blender-git/blender/source/blender/editors/sculpt_paint/sculpt.c:445:12: note: expected ‘float *’ but argument is of type ‘const float *’
static int sculpt_nearest_vertex_get(
^~~~~~~~~~~~~~~~~~~~~~~~~Proposed fix:
| source/blender/editors/sculpt_paint/sculpt.c | ||
|---|---|---|
| 4092 | I see this variable, but still a few calls to the same function. Could that be improved? | |
| 4134 | I got a segmentation error here. weights are not always initialized. Root cause: br->pose_ik_segments == 0 | |
Comment Actions
Yes you fixed the crash, but not the root cause :-)
when starting blender the pose brush is set to 0 segments. Check with --factory-startup fixing this should be the right solution.
| source/blender/blenloader/intern/versioning_280.c | ||
|---|---|---|
| 4321 | this code seems not be correct. When starting blender with a factory settings the pose brush still has 0 pose_ik_segments | |
| source/blender/editors/sculpt_paint/sculpt.c | ||
| 4196 | this seems to be a patch, not a fix. pose_ik_segments must never be zero. | |
| release/scripts/startup/bl_ui/properties_paint_common.py | ||
|---|---|---|
| 107 | Is this change part of this patch or should it be added to a different patch? | |
| source/blender/editors/sculpt_paint/sculpt.c | ||
| 4072 | At this point it is not clear why [0].weights isn't freed. | |
| 4197 | this should never happen, so could be removed? | |
| release/scripts/startup/bl_ui/properties_paint_common.py | ||
|---|---|---|
| 107 | These properties are needed for this patch. pose_smooth_iterations was introduced in a previous patch, but I think it was accidentally removed in the refactor. | |
Comment Actions
We're close.
I did see some additional changes were added to this patch that also seemed to be unrelated (alpha of the add_col and sub_col). best to add that to a separate patch.
Error: Not freed memory blocks: 3, total unfreed memory 0.007507 MB Pose IK Chain len: 16 0x7f13402ec728 Pose IK Chain Segments len: 1616 0x7f133fbf4338 Pose IK weights len: 2304 0x7f1338499e38
If we can fix this it is good to commit.