Fix #116138: make hidden bones show up in Properties when selected in Outliner #117012

Cedric-Hutchings · 2024-01-11T07:32:04+01:00

Cedric-Hutchings commented

2024-01-11 07:32:04 +01:00

First-time contributor

Bones selected in the Outliner are shown in the Properties panel, even if Hidden.

Fixes #116138

Bones selected in the Outliner are shown in the Properties panel, even if Hidden. Fixes #116138

Cedric-Hutchings force-pushed hidden-bones from a281217482 to 229f1eb610

2024-01-11 07:45:22 +01:00

Compare

Sybren A. Stüvel requested review from Sybren A. Stüvel 2024-01-11 09:29:23 +01:00

Sybren A. Stüvel added the

Module

Animation & Rigging

label 2024-01-11 09:29:29 +01:00

Sybren A. Stüvel commented

2024-01-11 14:38:18 +01:00

Thanks for your PR! I've put it on today's Animation & Rigging module meeting agenda to discuss.

In Blender Chat @lichtwerk wrote:

that removal of active being tied to not-hidden might require a whole lot of more testing

This I agree with. #116138 is about the general issue that a bone cannot be active and hidden at the same time. After all, there are multiple ways in which to hide a bone, and those could potentially all un-mark it as 'active'. Your PR certainly helps to resolve this, but seems to be limited to the Outliner. Because of that, I'm weary that it's not enough to fully fix #116138.

the PR description also would need to be a bit more in line with https://wiki.blender.org/wiki/Process/Contributing_Code

This would also be appreciated.

Thanks for your PR! I've put it on today's Animation & Rigging module meeting agenda to discuss. In Blender Chat @lichtwerk wrote: > that removal of active being tied to not-hidden might require a whole lot of more testing This I agree with. #116138 is about the general issue that a bone cannot be active and hidden at the same time. After all, there are multiple ways in which to hide a bone, and those could potentially all un-mark it as 'active'. Your PR certainly helps to resolve this, but seems to be limited to the Outliner. Because of that, I'm weary that it's not enough to fully fix #116138. > the PR description also would need to be a bit more in line with https://wiki.blender.org/wiki/Process/Contributing_Code This would also be appreciated.

Cedric-Hutchings commented

2024-01-16 08:06:57 +01:00

First-time contributor

a bone cannot be active and hidden at the same time.
After all, there are multiple ways in which to hide a bone,
and those could potentially all un-mark it as 'active'.

I went to every occurrence of "BONE_HIDDEN_A" in the codebase and:

found a sequence of actions that would trigger a breakpoint set in this code,
evaluated its relevance to allowing bones to be active and hidden,
and made sure that bones stayed "active" if BONE_HIDDEN_A was modified.

Here is one where I need your help/guidance:

[ ] source/blender/editors/space_outliner/outliner_select.cc:655 if (!(ebone->flag & BONE_HIDDEN_A)) {

To run this code

??? - I couldn't figure out how to get "extend" to be set; shift clicking didn't work :(
It's stored as an RNA property in the Operator ...

Relevance to Active | Hidden:

It DOES seem to set the bone to active if it isn't hidden ... so it may be important for this PR?

This one was also of particular note:

[ ] source/blender/animrig/ANIM_bone_collections.hh:296 const bool bone_itself_visible = (ebone->flag & BONE_HIDDEN_A) == 0;

To run this code

It's everywhere!

Relevance to Active | Hidden:

Used exclusively in EBONE_VISIBLE, which defines 50 new locations (mostly related to armature selection)
where HIDDEN is read and bone "active" state is (or isn't!) written to.

Will investigate in a subsequent post. Most of these are extra hairy,
because they involve selecting multiple bones, so it isn't clear which should appear in the Property Panel.
(Probably just the one you clicked?)

Here is a list of locations that use BONE_HIDDEN_A that I ruled out:

[x] source/blender/editors/armature/armature_edit.cc:1505 ebone->flag |= BONE_HIDDEN_A;

To run this code

Select bone in the editor, press H.

Relevance to Active | Hidden:

N/A. Bone stays active.

[x] source/blender/editors/armature/armature_edit.cc:1564 if (ebone->flag & BONE_HIDDEN_A) {
[x] source/blender/editors/armature/armature_edit.cc:1568 ebone->flag &= ~BONE_HIDDEN_A;

To run this code

Add a bone, hide it, press Alt + H

Relevance to Active | Hidden:

N/A. "Selects" the bone in the editor (orange outline), but doesn't mark it as active (yellow outline)

[x] source/blender/editors/object/object_select.cc:315 ebone->flag &= ~BONE_HIDDEN_A;

To run this code

Add two bones. Parent one bone to the other in the property panel.
Right-click the parent's widget in the property panel, select "Jump to Target."

Relevance to Active | Hidden:

This automatically unhides the parent for you, if applicable.
Also selects it as active in the Property Panel.

NOTE

Un-parented bones seem to be unselectable in the 3D editor after this? It's quite strange.

[x] source/blender/editors/space_outliner/outliner_draw.cc:223 if (ebone->flag & BONE_HIDDEN_A) {
[x] source/blender/editors/space_outliner/outliner_draw.cc:228 restrictbutton_recursive_ebone(arm, ebone, BONE_HIDDEN_A, (ebone->flag & BONE_HIDDEN_A) != 0);
[x] source/blender/editors/space_outliner/outliner_draw.cc:1419 BONE_HIDDEN_A,

To run this code

Click the filter icon at the top of the outliner.
Activate the "computer screen" icon.
Click the "computer screen" icon next to a bone in the outliner

Relevance to Active | Hidden:

Hides the bone. Leaves it as "active" in the Property Panel, but it doesn't come back with the yellow glow in the 3D view.

[x] source/blender/editors/space_outliner/outliner_tools.cc:2172 ebone->flag |= BONE_HIDDEN_A;
[x] source/blender/editors/space_outliner/outliner_tools.cc:2176 ebone->flag &= ~BONE_HIDDEN_A;

To run this code

Right click your bone in the outliner, select HIDE

Relevance to Active | Hidden:

The moment you right click the Bone to hide/unhide, it's immediately selected in the property panel.
This seems to be the correct behavior.

[x] source/blender/editors/transform/transform_snap_object_armature.cc:62 if (eBone->flag & BONE_HIDDEN_A) {

To run this code

Move bones around with snapping enabled.

Relevance to Active | Hidden:

N/A, Snapping against hidden bones is disabled

[x] source/blender/makesdna/DNA_armature_types.h:402 BONE_HIDDEN_A = (1 << 10),

To run this code

N/A, definition of BONE_HIDDEN_A

Relevance to Active | Hidden:

N/A

[x] source/blender/makesrna/intern/rna_armature.cc:1844 RNA_def_property_boolean_sdna(prop, nullptr, "flag", BONE_HIDDEN_A);

To run this code

Load in a Blender project with hidden bones.

Relevance to Active | Hidden:

N/A

> a bone cannot be active and hidden at the same time. > After all, there are multiple ways in which to hide a bone, > and those could potentially all un-mark it as 'active'. I went to every occurrence of "BONE_HIDDEN_A" in the codebase and: - found a sequence of actions that would trigger a breakpoint set in this code, - evaluated its relevance to allowing bones to be active and hidden, - and made sure that bones stayed "active" if BONE_HIDDEN_A was modified. # Here is one where I need your help/guidance: ``` [ ] source/blender/editors/space_outliner/outliner_select.cc:655 if (!(ebone->flag & BONE_HIDDEN_A)) { ``` ### To run this code ??? - I couldn't figure out how to get "extend" to be set; shift clicking didn't work :( It's stored as an RNA property in the Operator ... ### Relevance to Active | Hidden: It DOES seem to set the bone to active if it isn't hidden ... so it may be important for this PR? ## This one was also of particular note: ``` [ ] source/blender/animrig/ANIM_bone_collections.hh:296 const bool bone_itself_visible = (ebone->flag & BONE_HIDDEN_A) == 0; ``` ### To run this code It's everywhere! ### Relevance to Active | Hidden: Used exclusively in EBONE_VISIBLE, which defines 50 new locations (mostly related to armature selection) where HIDDEN is read and bone "active" state is (or isn't!) written to. Will investigate in a subsequent post. Most of these are extra hairy, because they involve selecting multiple bones, so it isn't clear which should appear in the Property Panel. (Probably just the one you clicked?) # Here is a list of locations that use BONE_HIDDEN_A that I ruled out: ``` [x] source/blender/editors/armature/armature_edit.cc:1505 ebone->flag |= BONE_HIDDEN_A; ``` ### To run this code Select bone in the editor, press H. ### Relevance to Active | Hidden: N/A. Bone stays active. ``` [x] source/blender/editors/armature/armature_edit.cc:1564 if (ebone->flag & BONE_HIDDEN_A) { [x] source/blender/editors/armature/armature_edit.cc:1568 ebone->flag &= ~BONE_HIDDEN_A; ``` ### To run this code Add a bone, hide it, press Alt + H ### Relevance to Active | Hidden: N/A. "Selects" the bone in the editor (orange outline), but doesn't mark it as active (yellow outline) ``` [x] source/blender/editors/object/object_select.cc:315 ebone->flag &= ~BONE_HIDDEN_A; ``` ### To run this code Add two bones. Parent one bone to the other in the property panel. Right-click the parent's widget in the property panel, select "Jump to Target." ### Relevance to Active | Hidden: This automatically unhides the parent for you, if applicable. Also selects it as active in the Property Panel. # NOTE Un-parented bones seem to be unselectable in the 3D editor after this? It's quite strange. ``` [x] source/blender/editors/space_outliner/outliner_draw.cc:223 if (ebone->flag & BONE_HIDDEN_A) { [x] source/blender/editors/space_outliner/outliner_draw.cc:228 restrictbutton_recursive_ebone(arm, ebone, BONE_HIDDEN_A, (ebone->flag & BONE_HIDDEN_A) != 0); [x] source/blender/editors/space_outliner/outliner_draw.cc:1419 BONE_HIDDEN_A, ``` ### To run this code - Click the filter icon at the top of the outliner. - Activate the "computer screen" icon. - Click the "computer screen" icon next to a bone in the outliner ### Relevance to Active | Hidden: Hides the bone. Leaves it as "active" in the Property Panel, but it doesn't come back with the yellow glow in the 3D view. ``` [x] source/blender/editors/space_outliner/outliner_tools.cc:2172 ebone->flag |= BONE_HIDDEN_A; [x] source/blender/editors/space_outliner/outliner_tools.cc:2176 ebone->flag &= ~BONE_HIDDEN_A; ``` ### To run this code Right click your bone in the outliner, select HIDE ### Relevance to Active | Hidden: The moment you right click the Bone to hide/unhide, it's immediately selected in the property panel. This seems to be the correct behavior. ``` [x] source/blender/editors/transform/transform_snap_object_armature.cc:62 if (eBone->flag & BONE_HIDDEN_A) { ``` ### To run this code Move bones around with snapping enabled. ### Relevance to Active | Hidden: N/A, Snapping against hidden bones is disabled ``` [x] source/blender/makesdna/DNA_armature_types.h:402 BONE_HIDDEN_A = (1 << 10), ``` ### To run this code N/A, definition of BONE_HIDDEN_A ### Relevance to Active | Hidden: N/A ``` [x] source/blender/makesrna/intern/rna_armature.cc:1844 RNA_def_property_boolean_sdna(prop, nullptr, "flag", BONE_HIDDEN_A); ``` ### To run this code Load in a Blender project with hidden bones. ### Relevance to Active | Hidden: N/A

Sybren A. Stüvel commented

2024-01-18 12:16:14 +01:00

I went to every occurrence of "BONE_HIDDEN_A" in the codebase and:

Being thorough, nice! :)

There's also BONE_HIDDEN_P and BONE_HIDDEN_PG, which are used in different situations:

BONE_HIDDEN_A: in Armature Edit mode (I think that's what the A is for)
BONE_HIDDEN_P: Pose mode and object mode
BONE_HIDDEN_PG: I don't think this one is ever set anywhere, I can only find code that reads it.

??? - I couldn't figure out how to get "extend" to be set; shift clicking didn't work :(

That seems to be triggered when using Ctrl.

It DOES seem to set the bone to active if it isn't hidden ... so it may be important for this PR?

Yup, that seems relevant.

[ ] source/blender/animrig/ANIM_bone_collections.hh:296 const bool bone_itself_visible = (ebone->flag & BONE_HIDDEN_A) == 0;
Used exclusively in EBONE_VISIBLE, which defines 50 new locations (mostly related to armature selection)
where HIDDEN is read and bone "active" state is (or isn't!) written to.

A bit of context: what you're looking at here is the old macro (EBONE_VISIBLE) that used to check armature layers. I changed the macro to call the new ANIM_bonecoll_is_visible_editbone() function to account for the new bone collections, but otherwise left the call sites of the macro as-is.

Will investigate in a subsequent post. Most of these are extra hairy,
because they involve selecting multiple bones, so it isn't clear which should appear in the Property Panel.
(Probably just the one you clicked?)

I don't think the logic of which bone becomes the active one should change.

For this task, I think the most important change is to go over the "hide this bone" code paths, and ensure that they do not touch the bArmature::act_bone or bArmature::act_edbone pointers. As in, they shouldn't deactivate the active bone, just because it got hidden.

Here is a list of locations that use BONE_HIDDEN_A that I ruled out:

👍

NOTE

Un-parented bones seem to be unselectable in the 3D editor after this? It's quite strange.

That might be worth some further investigation and a separate bug report. Would you be up for that?

[x] source/blender/editors/space_outliner/outliner_draw.cc:223 if (ebone->flag & BONE_HIDDEN_A) {
[x] source/blender/editors/space_outliner/outliner_draw.cc:228 restrictbutton_recursive_ebone(arm, ebone, BONE_HIDDEN_A, (ebone->flag & BONE_HIDDEN_A) != 0);
[x] source/blender/editors/space_outliner/outliner_draw.cc:1419 BONE_HIDDEN_A,
To run this code

Click the filter icon at the top of the outliner.

Activate the "computer screen" icon.

Click the "computer screen" icon next to a bone in the outliner

Relevance to Active | Hidden:

Hides the bone. Leaves it as "active" in the Property Panel, but it doesn't come back with the yellow glow in the 3D view.

How do you mean "it doesn't come back"?

[x] source/blender/editors/space_outliner/outliner_tools.cc:2172 ebone->flag |= BONE_HIDDEN_A;
[x] source/blender/editors/space_outliner/outliner_tools.cc:2176 ebone->flag &= ~BONE_HIDDEN_A;
To run this code

Right click your bone in the outliner, select HIDE

Relevance to Active | Hidden:

The moment you right click the Bone to hide/unhide, it's immediately selected in the property panel.
This seems to be the correct behavior.

If it seems correct, let's just assume it is correct, unless there are complaints / bug reports that say otherwise ;-) And yeah, it seems correct to me too.

> I went to every occurrence of "BONE_HIDDEN_A" in the codebase and: Being thorough, nice! :) There's also `BONE_HIDDEN_P` and `BONE_HIDDEN_PG`, which are used in different situations: - `BONE_HIDDEN_A`: in Armature Edit mode (I think that's what the `A` is for) - `BONE_HIDDEN_P`: Pose mode and object mode - `BONE_HIDDEN_PG`: I don't think this one is ever set anywhere, I can only find code that reads it. > ??? - I couldn't figure out how to get "extend" to be set; shift clicking didn't work :( That seems to be triggered when using Ctrl. > It DOES seem to set the bone to active if it isn't hidden ... so it may be important for this PR? Yup, that seems relevant. > ``` > [ ] source/blender/animrig/ANIM_bone_collections.hh:296 const bool bone_itself_visible = (ebone->flag & BONE_HIDDEN_A) == 0; > ``` > > Used exclusively in EBONE_VISIBLE, which defines 50 new locations (mostly related to armature selection) > where HIDDEN is read and bone "active" state is (or isn't!) written to. A bit of context: what you're looking at here is the old macro (`EBONE_VISIBLE`) that used to check armature layers. I changed the macro to call the new `ANIM_bonecoll_is_visible_editbone()` function to account for the new bone collections, but otherwise left the call sites of the macro as-is. > Will investigate in a subsequent post. Most of these are extra hairy, > because they involve selecting multiple bones, so it isn't clear which should appear in the Property Panel. > (Probably just the one you clicked?) I don't think the logic of which bone becomes the active one should change. For this task, I think the most important change is to go over the "hide this bone" code paths, and ensure that they do not touch the `bArmature::act_bone` or `bArmature::act_edbone` pointers. As in, they shouldn't deactivate the active bone, just because it got hidden. > # Here is a list of locations that use BONE_HIDDEN_A that I ruled out: :+1: > # NOTE > Un-parented bones seem to be unselectable in the 3D editor after this? It's quite strange. That might be worth some further investigation and a separate bug report. Would you be up for that? > ``` > [x] source/blender/editors/space_outliner/outliner_draw.cc:223 if (ebone->flag & BONE_HIDDEN_A) { > [x] source/blender/editors/space_outliner/outliner_draw.cc:228 restrictbutton_recursive_ebone(arm, ebone, BONE_HIDDEN_A, (ebone->flag & BONE_HIDDEN_A) != 0); > [x] source/blender/editors/space_outliner/outliner_draw.cc:1419 BONE_HIDDEN_A, > ``` > > ### To run this code > - Click the filter icon at the top of the outliner. > - Activate the "computer screen" icon. > - Click the "computer screen" icon next to a bone in the outliner > > ### Relevance to Active | Hidden: > Hides the bone. Leaves it as "active" in the Property Panel, but it doesn't come back with the yellow glow in the 3D view. How do you mean "it doesn't come back"? > ``` > [x] source/blender/editors/space_outliner/outliner_tools.cc:2172 ebone->flag |= BONE_HIDDEN_A; > [x] source/blender/editors/space_outliner/outliner_tools.cc:2176 ebone->flag &= ~BONE_HIDDEN_A; > ``` > > ### To run this code > Right click your bone in the outliner, select HIDE > > ### Relevance to Active | Hidden: > The moment you right click the Bone to hide/unhide, it's immediately selected in the property panel. > This seems to be the correct behavior. If it *seems* correct, let's just assume it *is* correct, unless there are complaints / bug reports that say otherwise ;-) And yeah, it seems correct to me too.

Sybren A. Stüvel requested changes 2024-03-25 10:27:50 +01:00

Sybren A. Stüvel left a comment

The ED_armature_edit_validate_active() function is now empty, so it can be removed along with all the calls to it.

Also make sure that either your IDE is configured to correctly auto-format the code, or run make format before committing. That way your PR will be in tip-top shape.

Finally, if you merge the current main branch into your PR you'll see there's a small confict. That shouldn't be too hard to solve.

Once that's done, let's get the buildbot building your changes, so that we have a test build that animators & riggers can test with.

The `ED_armature_edit_validate_active()` function is now empty, so it can be removed along with all the calls to it. Also make sure that either your IDE is configured to correctly auto-format the code, or run `make format` before committing. That way your PR will be in tip-top shape. Finally, if you merge the current `main` branch into your PR you'll see there's a small confict. That shouldn't be too hard to solve. Once that's done, let's get the buildbot building your changes, so that we have a test build that animators & riggers can test with.

This pull request has changes conflicting with the target branch.

source/blender/editors/space_outliner/outliner_select.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u hidden-bones:Cedric-Hutchings-hidden-bones

git checkout Cedric-Hutchings-hidden-bones

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

Fix #116138: make hidden bones show up in Properties when selected in Outliner #117012

Here is one where I need your help/guidance:

To run this code

Relevance to Active | Hidden:

This one was also of particular note:

To run this code

Relevance to Active | Hidden:

Here is a list of locations that use BONE_HIDDEN_A that I ruled out:

To run this code

Relevance to Active | Hidden:

To run this code

Relevance to Active | Hidden:

To run this code

Relevance to Active | Hidden:

NOTE

To run this code

Relevance to Active | Hidden:

To run this code

Relevance to Active | Hidden:

To run this code

Relevance to Active | Hidden:

To run this code

Relevance to Active | Hidden:

To run this code

Relevance to Active | Hidden:

Here is a list of locations that use BONE_HIDDEN_A that I ruled out:

NOTE

To run this code

Relevance to Active | Hidden:

To run this code

Relevance to Active | Hidden:

Checkout