Fix #114981 : Add box, circle and lasso selection tools to weight paint + bone select mode #115807

Open
Daiki Hashimoto wants to merge 11 commits from daisea3e1203/blender:add-select-tools-to-bone-select-mode into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Contributor

Issue

Currently, bone select mode in weight paint mode (added/made explicit in #115409) has a tweak tool, but lacks box, circle and lasso select tools, lacking consistency with the other two modes (face and vertex select modes) in Weight paint mode. This results in a lack of useful methods for selecting multiple bones in weight paint mode, which was mentioned as necessary in issue #114981 .

Note : This issue was mentioned in the Future Improvements section of PR #115409, and will address Issue #114981 together with PR #115409 which is already merged.

Solution

By adding a box, circle and lasso select tools with the following behavior, all selection tools can be added while respecting the current behavior of selecting bones using alt-click and LCS and right-click in RCS (referred to as click behavior):

  • Keep most of the behavior of selection tools in pose mode
  • When using the selection tools, cursor selecting in an empty area will not deselect bones (aligning with the current click behavior), while region selecting will (new behavior)
  • EDIT : When activating region select from hotkeys such as B for box select and C for circle select, region selecting in an empty area will not deselect bones (aligning with the behavior in Pose mode. This is different from the behavior above, but these two behaviors are consistent with the rest of Blender.)
  • In the case where multiple armatures were selected when going into weight paint mode, you cannot multi-select bones across armatures. (You can only multi-select bones on the current active armature. You can however switch the pose mode armature using cursor select. This aligns with the current additive click behavior.)

Note: this PR will also enable the B and C hotkeys for box select and circle select as the keymap from 3D View will fall through to weight paint mode.

Examples

Cursor picking with box and lasso tool will not deselect bones, but region picking will. (Clicking here will not deselect the bone, but box selecting from here to the right will. Same goes for the lasso tool.)
image

When an object has multiple armatures deforming it and both armatures were selected when going into weight paint mode, box selecting all the bones will only select the bones from the armature with the active bone. It is still possible to select the bone in the other armature by cursor picking.
image
(before box select)
image
(after box select)
image
(after clicking on a bone in the other armature)

Key areas for review

Behavior of region selecting an empty space

The ability to deselect all bones by region selecting an empty area felt reasonable, and might be useful in situations where selecting multiple bones in weight paint mode is useful. (To run a certain operation based on the selection of bones?) However, this does not align with the current behavior of alt-click bone select where clicking in an empty space will not deselect any bones. If aligning with this behavior feels more natural, then maybe region selecting an empty space should not deselect all bones.

Files

Example file used in screen shots: pr_115807_single_armature.blend, pr_115807_multiple_armatures.blend

Commit message for the squashed commit

Currently, bone select mode in weight paint mode has a tweak tool, but lacks box, circle and lasso select tools, lacking consistency with the other two modes (face and vertex select modes) in Weight paint mode. This results in a lack of useful methods for selecting multiple bones in weight paint mode.

By adding a box, circle and lasso select tools with the following behavior, all selection tools can be added while respecting the current behavior of selecting bones using alt-click and LCS and right-click in RCS (referred to as click behavior):

  • Keep most of the behavior of selection tools in pose mode
  • When using the selection tools, cursor selecting in an empty area will not deselect bones (aligning with the current click behavior), while region selecting wil.
  • When activating region select from hotkeys such as B for box select and C for circle select, region selecting in an empty area will not deselect bones (aligning with the behavior in Pose mode. This is different from the behavior above, but these two behaviors are consistent with the rest of Blender.)
  • In the case where multiple armatures were selected when going into weight paint mode, you cannot multi-select bones across armatures. (You can only multi-select bones on the current active armature. You can however switch the pose mode armature using cursor select. This aligns with the current additive click behavior.)
# Issue Currently, bone select mode in weight paint mode (added/made explicit in #115409) has a tweak tool, but lacks box, circle and lasso select tools, lacking consistency with the other two modes (face and vertex select modes) in Weight paint mode. This results in a lack of useful methods for selecting multiple bones in weight paint mode, which was mentioned as necessary in issue #114981 . **Note :** This issue was mentioned in the Future Improvements section of PR #115409, and will address Issue #114981 together with PR #115409 which is already merged. # Solution By adding a box, circle and lasso select tools with the following behavior, all selection tools can be added while respecting the current behavior of selecting bones using alt-click and LCS and right-click in RCS (referred to as click behavior): - Keep most of the behavior of selection tools in pose mode - When using the selection tools, cursor selecting in an empty area will not deselect bones (aligning with the current click behavior), while region selecting will (new behavior) - **EDIT** : When activating region select from hotkeys such as B for box select and C for circle select, region selecting in an empty area will not deselect bones (aligning with the behavior in Pose mode. This is different from the behavior above, but these two behaviors are consistent with the rest of Blender.) - In the case where multiple armatures were selected when going into weight paint mode, you cannot multi-select bones across armatures. (You can only multi-select bones on the current active armature. You can however switch the pose mode armature using cursor select. This aligns with the current additive click behavior.) **Note:** this PR will also enable the B and C hotkeys for box select and circle select as the keymap from 3D View will fall through to weight paint mode. **Examples** Cursor picking with box and lasso tool will not deselect bones, but region picking will. (Clicking here will not deselect the bone, but box selecting from here to the right will. Same goes for the lasso tool.) ![image](/attachments/241ee3d1-ed75-43ce-b6d2-3cedb50eb34b) When an object has multiple armatures deforming it and both armatures were selected when going into weight paint mode, box selecting all the bones will only select the bones from the armature with the active bone. It is still possible to select the bone in the other armature by cursor picking. ![image](/attachments/57ae411c-b8bc-4efb-a634-43cdc6d91b90) (before box select) ![image](/attachments/05b24ce1-a77a-41ba-88b5-23898acf79af) (after box select) ![image](/attachments/dc698883-602a-4a58-8893-086d56271b3f) (after clicking on a bone in the other armature) # Key areas for review ## Behavior of region selecting an empty space The ability to deselect all bones by region selecting an empty area felt reasonable, and might be useful in situations where selecting multiple bones in weight paint mode is useful. (To run a certain operation based on the selection of bones?) However, this does not align with the current behavior of alt-click bone select where clicking in an empty space will not deselect any bones. If aligning with this behavior feels more natural, then maybe region selecting an empty space should not deselect all bones. # Files Example file used in screen shots: [pr_115807_single_armature.blend](/attachments/73513382-6c02-4432-91fa-eab2b2030bb3), [pr_115807_multiple_armatures.blend](/attachments/a4aec492-5017-4372-8d53-2d30b19ff2be) # Commit message for the squashed commit Currently, bone select mode in weight paint mode has a tweak tool, but lacks box, circle and lasso select tools, lacking consistency with the other two modes (face and vertex select modes) in Weight paint mode. This results in a lack of useful methods for selecting multiple bones in weight paint mode. By adding a box, circle and lasso select tools with the following behavior, all selection tools can be added while respecting the current behavior of selecting bones using alt-click and LCS and right-click in RCS (referred to as click behavior): - Keep most of the behavior of selection tools in pose mode - When using the selection tools, cursor selecting in an empty area will not deselect bones (aligning with the current click behavior), while region selecting wil. - When activating region select from hotkeys such as B for box select and C for circle select, region selecting in an empty area will not deselect bones (aligning with the behavior in Pose mode. This is different from the behavior above, but these two behaviors are consistent with the rest of Blender.) - In the case where multiple armatures were selected when going into weight paint mode, you cannot multi-select bones across armatures. (You can only multi-select bones on the current active armature. You can however switch the pose mode armature using cursor select. This aligns with the current additive click behavior.)
Daiki Hashimoto added 5 commits 2023-12-05 16:04:47 +01:00
Iliya Katushenock added this to the Sculpt, Paint & Texture project 2023-12-05 16:07:21 +01:00
Daiki Hashimoto changed title from WIP: Add box, circle and lasso selection tools to weight paint + bone select mode to Fix #114981 : Add box, circle and lasso selection tools to weight paint + bone select mode 2023-12-06 13:47:46 +01:00
Christoph Lendenfeld requested review from Christoph Lendenfeld 2023-12-07 17:06:01 +01:00
Christoph Lendenfeld added the
Module
Animation & Rigging
label 2023-12-07 17:06:07 +01:00
Christoph Lendenfeld requested changes 2023-12-08 10:56:00 +01:00
Christoph Lendenfeld left a comment
Member

minor comment style changes.

The bigger thing is the behavior when clicking in an empty space. It feels weird to me, and it is inconsistent with the behavior in object mode. But I feel like it's not up to me to decide.

Great job pointing that out in the PR description btw. And also good job testing the edge case of multiple armatures.

@Mets need you to have an eye on this. What should happen when clicking in an empty space with e.g. the box select tool.

minor comment style changes. The bigger thing is the behavior when clicking in an empty space. It feels weird to me, and it is inconsistent with the behavior in object mode. But I feel like it's not up to me to decide. Great job pointing that out in the PR description btw. And also good job testing the edge case of multiple armatures. @Mets need you to have an eye on this. What should happen when clicking in an empty space with e.g. the box select tool.
@ -208,2 +208,4 @@
*/
bool BKE_paint_select_vert_test(const Object *ob);
// /**
// * Return true when in weight paint + bone-select mode?

The blender comment style is without the double slash (//) at the start

Also I feel like the comment should not be a question. I know this is the same as the functions above,
but it's confusing to read.
I assume its just a statement. As in it returns true when in weight paint + bone-select mode.

The blender comment style is without the double slash (//) at the start Also I feel like the comment should not be a question. I know this is the same as the functions above, but it's confusing to read. I assume its just a statement. As in it returns true when in weight paint + bone-select mode.
Author
Contributor

I applied the same change in 09a3f4b9b1 to the two functions above (change the question mark to a period) since I encountered them many times when coding for this PR, and were used as a statement to check for their respective modes.

I applied the same change in 09a3f4b9b1 to the two functions above (change the question mark to a period) since I encountered them many times when coding for this PR, and were used as a statement to check for their respective modes.
daisea3e1203 marked this conversation as resolved
@ -2914,0 +2919,4 @@
else if (BKE_paint_select_bone_test(vc.obact)) {
/* pass */
/* During weight paint mode, when there are multiple armatures deforming a single mesh,

this comment is great. I love it

this comment is great. I love it
daisea3e1203 marked this conversation as resolved
@ -5307,6 +5320,21 @@ static int view3d_circle_select_exec(bContext *C, wmOperator *op)
}
FOREACH_OBJECT_IN_MODE_END;
}
/* Select bones from pose mode armature in weight paint + bone-select mode */

comments end with a dot

comments end with a dot
Author
Contributor

Addressed in 05bb4f72e0
Thank you for spotting these mistakes! Will be careful with comments in future PRs 🙏

Addressed in 05bb4f72e0 Thank you for spotting these mistakes! Will be careful with comments in future PRs 🙏
daisea3e1203 marked this conversation as resolved
Member

Thanks for the patch!

When an object has multiple armatures deforming it

To be clear, this is practically unsupported in Blender as it is, so the fact that it doesn't work perfectly is not the biggest issue imo.
However, if it did work, that would make life easier in a lovely but unlikely to be near future, where Armature deformation is done via GeoNodes, so the weights can come from as many armature objects as desired and still be normalized. But considering this is not on anybody's roadmap, I don't feel strongly about it.

The ability to deselect all bones by region selecting an empty area felt reasonable

I would stay consistent with other areas of Blender here. This sounds a bit esoteric to me.
Deselect All is Alt+A throughout Blender. Meanwhile, box/circle/lasso select is an additive selection throughout Blender. Having it clear the selection when there's nothing in the selected region feels like it will just result in confusion, so I wouldn't do it tbh.

Thanks for the patch! > When an object has multiple armatures deforming it To be clear, this is practically unsupported in Blender as it is, so the fact that it doesn't work perfectly is not the biggest issue imo. However, if it did work, that would make life easier in a lovely but unlikely to be near future, where Armature deformation is done via GeoNodes, so the weights can come from as many armature objects as desired and still be normalized. But considering this is not on anybody's roadmap, I don't feel strongly about it. > The ability to deselect all bones by region selecting an empty area felt reasonable I would stay consistent with other areas of Blender here. This sounds a bit esoteric to me. Deselect All is Alt+A throughout Blender. Meanwhile, box/circle/lasso select is an additive selection throughout Blender. Having it clear the selection when there's nothing in the selected region feels like it will just result in confusion, so I wouldn't do it tbh.

I would stay consistent with other areas of Blender here. This sounds a bit esoteric to me.
Deselect All is Alt+A throughout Blender. Meanwhile, box/circle/lasso select is an additive selection throughout Blender. Having it clear the selection when there's nothing in the selected region feels like it will just result in confusion, so I wouldn't do it tbh.

this is another area where LCS and RCS are different. LCS definitely deselects by clicking an empty area. Will need to check RCS

> I would stay consistent with other areas of Blender here. This sounds a bit esoteric to me. > Deselect All is Alt+A throughout Blender. Meanwhile, box/circle/lasso select is an additive selection throughout Blender. Having it clear the selection when there's nothing in the selected region feels like it will just result in confusion, so I wouldn't do it tbh. this is another area where LCS and RCS are different. LCS definitely deselects by clicking an empty area. Will need to check RCS
Author
Contributor

Thank you for the feedback! Will check the RCS behavior and append to the PR description.

Thank you for the feedback! Will check the RCS behavior and append to the PR description.
Member

Oh wow, nevermind, I think it's not a left/right-click select thing, but a tool vs hotkey thing. Selection Tools, which I never used, behave differently from B/C/Ctrl+Click hotkeys... Wow. I learned something. Well, I think that's odd, but that ain't this patch's fault. But still, let's try to be consistent, and if this is consistent with the rest of Blender as it is, then so be it. 🙃

That said, I wish those hotkeys were available in weight paint mode, at least B and C. But I suppose that is a separate issue.

Oh wow, nevermind, I think it's not a left/right-click select thing, but a tool vs hotkey thing. Selection Tools, which I never used, behave differently from B/C/Ctrl+Click hotkeys... Wow. I learned something. Well, I think that's odd, but that ain't this patch's fault. But still, let's try to be consistent, and if this is consistent with the rest of Blender as it is, then so be it. 🙃 That said, I wish those hotkeys were available in weight paint mode, at least B and C. But I suppose that is a separate issue.
Author
Contributor

I forgot to mention in the description (fixed), but B and C hotkeys will be available with this patch! (the respective keymap from 3d View will be applied automatically now that the selection tools are available in weight paint mode.)
Will check if the behavior of selection tools activated from hotkeys align with the behavior you mentioned 👍

I forgot to mention in the description (fixed), but B and C hotkeys will be available with this patch! (the respective keymap from 3d View will be applied automatically now that the selection tools are available in weight paint mode.) Will check if the behavior of selection tools activated from hotkeys align with the behavior you mentioned 👍
Daiki Hashimoto added 2 commits 2023-12-08 14:20:19 +01:00
Author
Contributor

@Mets I checked the behavior of B and C hotkeys in weight paint mode, and they aligned with what you mentioned 👍 (region selecting with hotkeys doesn't deselect bones while region selecting with selection tools does. This behavior is appended to the PR description.)

@ChrisLend Thank you for spotting the comment style mistakes! They have been addressed in the last two commits. The review from @Mets should also be resolved with the check on hotkey behaviors above!

@Mets I checked the behavior of B and C hotkeys in weight paint mode, and they aligned with what you mentioned 👍 (region selecting with hotkeys doesn't deselect bones while region selecting with selection tools does. This behavior is appended to the PR description.) @ChrisLend Thank you for spotting the comment style mistakes! They have been addressed in the last two commits. The review from @Mets should also be resolved with the check on hotkey behaviors above!

Since if the feedback from @Mets is to make it consistent with the rest of blender, I am comparing weigth paint with pose mode here.

RCS

- Pose Mode Weight Paint
Right click in empty space deselects nothing happens
box select in empty space deselects deselects
box select bones only bones in box are selected only bones in box are selected
using C or B to select additive additive

LCS

- Pose Mode Weight Paint
Left click in empty space deselects nothing happens
box select in empty space deselects deselects
box select bones only bones in box are selected only bones in box are selected
using C or B to select additive additive

so the only difference I could find was the deselection behavior which is inconsistent and I think should change

Since if the feedback from @Mets is to make it consistent with the rest of blender, I am comparing weigth paint with pose mode here. ## RCS | -| Pose Mode | Weight Paint | | - | - | - | | Right click in empty space | deselects | nothing happens | | box select in empty space | deselects | deselects | | box select bones | only bones in box are selected | only bones in box are selected | | using C or B to select | additive | additive | ## LCS | -| Pose Mode | Weight Paint | | - | - | - | | Left click in empty space | deselects | nothing happens | | box select in empty space | deselects | deselects | | box select bones | only bones in box are selected | only bones in box are selected | | using C or B to select | additive | additive | so the only difference I could find was the deselection behavior which is inconsistent and I think should change
Author
Contributor

@ChrisLend
Do you think it would be clearer to create a separate PR for changing the behavior of clicking in an empty space during weight paint mode (row1 for LCS and RCS), and proceed with this PR as is?
The reason is that while the behaviors for rows 2-4 is related to the new box/circle select tools, changing row 1's behavior involves changing the existing behavior of not deselecting bones when right-clicking in an empty space during weight paint mode in RCS.

@ChrisLend Do you think it would be clearer to create a separate PR for changing the behavior of clicking in an empty space during weight paint mode (row1 for LCS and RCS), and proceed with this PR as is? The reason is that while the behaviors for rows 2-4 is related to the new box/circle select tools, changing row 1's behavior involves changing the existing behavior of not deselecting bones when right-clicking in an empty space during weight paint mode in RCS.

The reason is that while the behaviors for rows 2-4 is related to the new box/circle select tools, changing row 1's behavior involves changing the existing behavior of not deselecting bones when right-clicking in an empty space during weight paint mode in RCS.

@Mets your opinion on this? I don't mind having that as a separate PR if the behavior is already like that

> The reason is that while the behaviors for rows 2-4 is related to the new box/circle select tools, changing row 1's behavior involves changing the existing behavior of not deselecting bones when right-clicking in an empty space during weight paint mode in RCS. @Mets your opinion on this? I don't mind having that as a separate PR if the behavior is already like that
Member

Ah, so Blender is already inconsistent as it is, and right-clicking in an empty space in weight paint mode won't deselect bones like it does in pose mode. Odd indeed.

I'd say that's a separate topic though, and I wouldn't hold back this PR because of it.

Ah, so Blender is already inconsistent as it is, and right-clicking in an empty space in weight paint mode won't deselect bones like it does in pose mode. Odd indeed. I'd say that's a separate topic though, and I wouldn't hold back this PR because of it.
Christoph Lendenfeld approved these changes 2023-12-12 12:56:10 +01:00
Christoph Lendenfeld left a comment
Member

Just checked and it is consistent with the face/vertex mask selection behavior.
Thanks for pointing that out. In that case it looks good to land

Just checked and it is consistent with the face/vertex mask selection behavior. Thanks for pointing that out. In that case it looks good to land
Member

I don't know if I'm misunderstanding how these additions are supposed to work, but box/circle/lasso selecting isn't actually selecting bones in my testing. I can activate the tools just fine, and e.g. drag a box out. It's just that it doesn't actually affect the bone selection. Simply clicking does still select the bones, however.

I don't know if I'm misunderstanding how these additions are supposed to work, but box/circle/lasso selecting isn't actually selecting bones in my testing. I can activate the tools just fine, and e.g. drag a box out. It's just that it doesn't actually affect the bone selection. Simply clicking does still select the bones, however. <video src="/attachments/77138082-3050-409f-b20d-b03ab2e99369" title="box_select_not_working.mp4" controls></video>
Sybren A. Stüvel requested changes 2023-12-12 13:15:39 +01:00
Sybren A. Stüvel left a comment
Member

Just popping in to nag about a small thing.

Just popping in to nag about a small thing.
@ -1046,1 +1046,4 @@
bool BKE_paint_select_bone_test(const Object *ob)
{
return ((ob != nullptr) && (ob->type == OB_MESH) && (ob->data != nullptr) &&

This will be clearer when written as separate expressions. I know the current code is consistent with the existing code, but I find the existing code really hard to read too, and I don't think we should duplicate that.

if (ob == nullptr || ob->type != OB_MESH || (ob->mode & OB_MODE_WEIGHT_PAINT) == 0) {
  return false;
}

Mesh *mesh = static_cast<Mesh *>(ob->data);
return mesh->editflag & ~(ME_EDIT_PAINT_FACE_SEL | ME_EDIT_PAINT_VERT_SEL);

This separates the precondition checks from the actual query.

I also think the query is incorrect, as it checks whether any flag is set, ignoring ME_EDIT_PAINT_FACE_SEL and ME_EDIT_PAINT_VERT_SEL. IMO it should check that these flags are not set. I suspect this is the right check, but please do verify

return (mesh->editflag & (ME_EDIT_PAINT_FACE_SEL | ME_EDIT_PAINT_VERT_SEL)) == 0;

Note that I didn't include the check for ob->data != nullptr, as this is implied by ob->type == OB_MESH. If a Mesh object doesn't have any data, there is something very wrong and other areas in the code will have stopped working already. This function is not the place to try and solve this problem.

This will be clearer when written as separate expressions. I know the current code is consistent with the existing code, but I find the existing code really hard to read too, and I don't think we should duplicate that. ```cpp if (ob == nullptr || ob->type != OB_MESH || (ob->mode & OB_MODE_WEIGHT_PAINT) == 0) { return false; } Mesh *mesh = static_cast<Mesh *>(ob->data); return mesh->editflag & ~(ME_EDIT_PAINT_FACE_SEL | ME_EDIT_PAINT_VERT_SEL); ``` This separates the precondition checks from the actual query. I also think the query is incorrect, as it checks whether any flag is set, ignoring `ME_EDIT_PAINT_FACE_SEL` and `ME_EDIT_PAINT_VERT_SEL`. IMO it should check that these flags are *not* set. I suspect this is the right check, but please do verify ```cpp return (mesh->editflag & (ME_EDIT_PAINT_FACE_SEL | ME_EDIT_PAINT_VERT_SEL)) == 0; ``` Note that I didn't include the check for `ob->data != nullptr`, as this is implied by `ob->type == OB_MESH`. If a Mesh object doesn't have any data, there is something very wrong and other areas in the code will have stopped working already. This function is not the place to try and solve this problem.
Author
Contributor

Thank you very much for the review and the insights about mesh data 🙏

The query is incorrect as you have pointed out, and have probably lead to the tools not functioning properly in some situations as @nathanvegdahl has demonstrated below. This has been fixed in e11fce8298 .

I have also refactored the two functions above in a similar fashion in 93a58c7601, but if you think this change should occur in a different PR or occasion, please let me know.

Thank you very much for the review and the insights about mesh data 🙏 The query is incorrect as you have pointed out, and have probably lead to the tools not functioning properly in some situations as @nathanvegdahl has demonstrated below. This has been fixed in e11fce8298 . I have also refactored the two functions above in a similar fashion in 93a58c7601, but if you think this change should occur in a different PR or occasion, please let me know.

This has been fixed in e11fce8298 .
👍

if you think this change should occur in a different PR or occasion, please let me know.
Yup, that's for a different PR.

Ideally, such cleanups would happen before this PR (first cleanup, then change functionality). In this case I don't think that would be worth the effort, though.

> This has been fixed in e11fce8298 . :+1: > if you think this change should occur in a different PR or occasion, please let me know. Yup, that's for a different PR. Ideally, such cleanups would happen *before* this PR (first cleanup, then change functionality). In this case I don't think that would be worth the effort, though.
Author
Contributor

Thank you very much for the feedback!

Ideally, such cleanups would happen before this PR (first cleanup, then change functionality). In this case I don't think that would be worth the effort, though.

When I attempt to make a large change to the codebase in the future, I will make sure to create a separate clean up PR beforehand.

Thank you very much for the feedback! > Ideally, such cleanups would happen before this PR (first cleanup, then change functionality). In this case I don't think that would be worth the effort, though. When I attempt to make a large change to the codebase in the future, I will make sure to create a separate clean up PR beforehand.

Sorry for the back-and-forth, but looking back at this now, I do think that it's better to have this change as a small separate PR that we can land before this one. That way this little fix can be described by itself, and in the hypothetical case that it causes any issues, rolled back by itself, without having to deal with this entire PR.

Sorry for the back-and-forth, but looking back at this now, I do think that it's better to have this change as a small separate PR that we can land before this one. That way this little fix can be described by itself, and in the hypothetical case that it causes any issues, rolled back by itself, without having to deal with this entire PR.

I don't know if I'm misunderstanding how these additions are supposed to work, but box/circle/lasso selecting isn't actually selecting bones in my testing. I can activate the tools just fine, and e.g. drag a box out. It's just that it doesn't actually affect the bone selection. Simply clicking does still select the bones, however.

Can you try with factory preferences or share your file? I just tried on my end which has a similar setup and it's definitely working.

> I don't know if I'm misunderstanding how these additions are supposed to work, but box/circle/lasso selecting isn't actually selecting bones in my testing. I can activate the tools just fine, and e.g. drag a box out. It's just that it doesn't actually affect the bone selection. Simply clicking does still select the bones, however. Can you try with factory preferences or share your file? I just tried on my end which has a similar setup and it's definitely working.
Member

@ChrisLend Yeah, here's the blend file. (Note: in the video I only moved the bones outside the mesh to verify that it wasn't an occlusion issue.)

Resetting to factory preferences didn't make any difference for me.

@ChrisLend Yeah, here's the blend file. (Note: in the video I only moved the bones outside the mesh to verify that it wasn't an occlusion issue.) Resetting to factory preferences didn't make any difference for me.
Daiki Hashimoto added 3 commits 2023-12-12 14:23:21 +01:00
Author
Contributor

@nathanvegdahl
Thank you so much for pointing this out! This is probably related to the flag check error @dr.sybren has pointed out, and should be fixed in the latest commit.

@nathanvegdahl Thank you so much for pointing this out! This is probably related to the flag check error @dr.sybren has pointed out, and should be fixed in the latest commit.

@nathanvegdahl indeed I can reproduce the issue with your file, but I cannot see the difference to my file. Can you verify my test file works for you

Edit: didn't see your latest comment. Will check the latest commit
Edit 2: yes with the latest changes, it even works in nathans test file

@nathanvegdahl indeed I can reproduce the issue with your file, but I cannot see the difference to my file. Can you verify my test file works for you Edit: didn't see your latest comment. Will check the latest commit Edit 2: yes with the latest changes, it even works in nathans test file
Daiki Hashimoto added 1 commit 2023-12-13 10:32:23 +01:00
Member

The latest change fixes the issue for me as well.

The latest change fixes the issue for me as well.
Nathan Vegdahl approved these changes 2023-12-14 10:15:48 +01:00
Nathan Vegdahl left a comment
Member

I've only given the code a cursory code review, since @ChrisLend and @dr.sybren have already looked more closely. But it looks good to me. I spent a bit more time testing, and it now seems to work as expected as well, including when multiple armatures are selected.

I've only given the code a cursory code review, since @ChrisLend and @dr.sybren have already looked more closely. But it looks good to me. I spent a bit more time testing, and it now seems to work as expected as well, including when multiple armatures are selected.
Author
Contributor

@ChrisLend
The changes requested by @dr.sybren has been addressed and were reviewed in the conversation thread, so this PR is probably ready to land!

I have also added a "Commit message for the squashed commit" in the PR description since I noticed you had to edit the PR description into a commit message when creating the squashed commit in the last PR. I simply removed references to other issues/PRs, and revised the Issue and Solution section, editing in the information from EDITs and NOTEs.

@ChrisLend The changes requested by @dr.sybren has been addressed and were reviewed in the conversation thread, so this PR is probably ready to land! I have also added a "Commit message for the squashed commit" in the PR description since I noticed you had to edit the PR description into a commit message when creating the squashed commit in the last PR. I simply removed references to other issues/PRs, and revised the Issue and Solution section, editing in the information from EDITs and NOTEs.
Christoph Lendenfeld requested review from Sybren A. Stüvel 2023-12-19 16:18:52 +01:00

@daisea3e1203 I've re-requested review from @dr.sybren since he still had not given the green tick. once that is given I can land it.

Thanks for writing a commit message beforehand :)

@daisea3e1203 I've re-requested review from @dr.sybren since he still had not given the green tick. once that is given I can land it. Thanks for writing a commit message beforehand :)
Sybren A. Stüvel reviewed 2024-02-12 11:29:19 +01:00
Sybren A. Stüvel left a comment
Member

When an object has multiple armatures deforming it and both armatures were selected when going into weight paint mode, box selecting all the bones will only select the bones from the armature with the active bone. It is still possible to select the bone in the other armature by cursor picking.

What was the reasoning behind this choice?

About the commit message:

Currently, bone select mode in weight paint mode has a tweak tool, but lacks ...

I'd recommend describing what the commit does. Using a word like "currently" can be confusing, as as soon as that commit is made, the message does not describe Blender's current behaviour any more.

By adding a box, circle and lasso select tools with the following behavior, all selection tools can be added ...

By writing can be added, the message implies that they have not been added yet. The PR title (which is used as commit title) tells us that these tools are added by this commit, though. This would be another point of confusion.

I think the commit message can be simpler. The initial paragraph could be something like:

Add box, circle, and lasso selection tools to the bone select mode in Weight Paint mode. This makes the bone select mode consistent with vertex/face selection modes.

After that a more elaborate message can be written with more detailed changes, motivations of choices, etc.

> When an object has multiple armatures deforming it and both armatures were selected when going into weight paint mode, box selecting all the bones will only select the bones from the armature with the active bone. It is still possible to select the bone in the other armature by cursor picking. What was the reasoning behind this choice? About the commit message: > Currently, bone select mode in weight paint mode has a tweak tool, but lacks ... I'd recommend describing what the commit does. Using a word like "currently" can be confusing, as as soon as that commit is made, the message does not describe Blender's current behaviour any more. > By adding a box, circle and lasso select tools with the following behavior, all selection tools can be added ... By writing **can** be added, the message implies that they have not been added yet. The PR title (which is used as commit title) tells us that these tools are added by this commit, though. This would be another point of confusion. I think the commit message can be simpler. The initial paragraph could be something like: > Add box, circle, and lasso selection tools to the bone select mode in Weight Paint mode. This makes the bone select mode consistent with vertex/face selection modes. After that a more elaborate message can be written with more detailed changes, motivations of choices, etc.
Sybren A. Stüvel requested changes 2024-02-13 10:16:51 +01:00
Sybren A. Stüvel left a comment
Member

I'm just clicking "Request Changes" to get this off my review queue.

I'm just clicking "Request Changes" to get this off my review queue.
This pull request has changes conflicting with the target branch.
  • scripts/startup/bl_ui/space_toolsystem_toolbar.py
  • source/blender/editors/space_view3d/view3d_select.cc

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u add-select-tools-to-bone-select-mode:daisea3e1203-add-select-tools-to-bone-select-mode
git checkout daisea3e1203-add-select-tools-to-bone-select-mode
Sign in to join this conversation.
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
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
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
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
5 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#115807
No description provided.