Replace Bone Layers+Groups with Bone Collections #109976
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
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
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 project
No Assignees
5 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#109976
Loading…
Reference in New Issue
No description provided.
Delete Branch "dr.sybren/blender:anim/armature-collections"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This implements the design described in #108941, and then specifically the targets for 4.0-bcon1
A demo file
108941-bone-collections-test.blend
is attached. The108941-saved-with-main-branch.blend
is the same file, but then saved with Blender 4.0main
branch (so without any bone collections) to test the versioning of armature layers & bone groups.Note: the cleanup to remove all traces of bone layers & groups (except where necessary for versioning) is not yet complete. That'll happen in a later patch, early September.
blender/source/animrig
module for new Animation & Rigging code (e604f3db91
).source/blender/animrig/ANIM_bone_collections.h
file for (e604f3db91
):e604f3db91
)source/blender/editors/armature/bone_collections.cc
file for operations on bone collections, mimicking what's currently possible for bone groups.Removal of bone group colors from RNA.not going to do this, as this would also remove access to action group colors, and this PR does not provide any alternative implementation for those.could be split between pose mode and edit modevisibility of bone layers is also shared between pose & edit mode, so keeping that behaviour for now)It is intended that this PR will result in multiple commits:
New API
User Interface
Bone Colors can be configured in the Bone property panel:
Bone Collections can be managed in the Armature property panel:
M
menu in pose mode (extending to edit mode is for bcon2). This will un-assign the selected bones from all their bone collections, then assign to the selected one (i.e. 'move to collection').Shift+M
menu in pose mode (extending to edit mode is for bcon2). This will simply assign to the chosen collection, keeping the other assignments intact.Ctrl+G
menu in pose mode (mimicking the current Ctrl+G for bone group assignments). This one will likely disappear before the release; I ported it just for completeness sake. Contrary to the above menus, it requires setting the active bone collection in the properties panel, the convenience of which is dubious.0d2ba6097e
to688ea60721
e405f1ee06
to90c27798e1
WIP: Armature Collectionsto Replace Bone Layers+Groups with Bone CollectionsReplace Bone Layers+Groups with Bone Collectionsto WIP: Replace Bone Layers+Groups with Bone Collections@blender-bot package
Package build started. Download here when ready.
406aafb056
toaf89de7139
af89de7139
toe3f1af6fee
e3f1af6fee
tocd1a5e8df4
cd1a5e8df4
toeef2629628
WIP: Replace Bone Layers+Groups with Bone Collectionsto Replace Bone Layers+Groups with Bone Collections@blender-bot package
Package build started. Download here when ready.
Nothing blocking jumped out at me looking through the code again. So mainly just some nits that can be addressed later.
@ -170,2 +138,2 @@
# row.operator("pose.bone_group_remove_from", text="Remove")
sub.operator("pose.group_unassign", text="Remove")
sub.operator("armature.collection_assign", text="Assign")
sub.operator("armature.collection_unassign", text="Remove")
These should be disabled if there is no
active_bcoll
. (Just needs to be added to poll.)When I looked at the actual poll functions, that doesn't actually look like the appropriate place, since these operators can be called with an explicitly specified collection rather than the active one. So it probably makes sense to disable them in the Python code here after all. Same for the other comment about this below.
@ -174,2 +142,2 @@
sub.operator("pose.group_select", text="Select")
sub.operator("pose.group_deselect", text="Deselect")
sub.operator("armature.collection_select", text="Select")
sub.operator("armature.collection_deselect", text="Deselect")
These should be disabled if there is no
active_bcoll
. (Just needs to be added to poll.)@ -0,0 +16,4 @@
#include "DNA_anim_types.h"
struct ThemeWireColor;
It looks like this is defined in
DNA_userdef_types.h
. I'd be more comfortable just including that instead to make it explicit whereThemeWireColor
is coming from. I guess the concern here is bloating build times?@ -19,3 +26,4 @@
#endif
struct AnimData;
struct BoneCollection;
Same comment as before, about preferring includes of the relevant header(s) to make it explicit where things come from. Although see this in other places as well, it appears this is a common pattern in Blender's code base. So maybe never mind.
@ -31,0 +45,4 @@
*/
int8_t palette_index;
uint8_t _pad0[7];
ThemeWireColor custom;
When we were looking through this in person, I noted that we could eliminate this padding by arranging the fields from largest to smallest. However, looking at other DNA structs in this PR, it looks the strategy is to ensure a multiple-of-8-bytes size anyway, which I assume is a general pattern/requirement in DNA (which totally makes sense).
So moving
palette_index
to the end here wouldn't actually save us any space with these struct members.While testing, I got a reproducible crash in debug:
The steps are fairly simple. In the provided example blend file: delete a bone in edit mode, leave edit mode, and then ctrl-z undo.
I'll investigate and fix this before landing.
can confirm that crash.
So I tried to review this patch but given its size I am sure I missed something. Two things I noticed though
Thanks for looking it over! And yeah, it's quite a lot to go through.
Poking around, I'm not able to trigger a crash with that either. If you can figure out a repro, that would be awesome!
As for Bone Group's still being there: I believe the intention is to finish removing Bone Groups in a follow up PR. They also show up under
Bone -> Relations
in the properties panel.Looks like an oversight to me.
ok so I did a bit more testing
the crash happens for me when deleting a bone collection and trying to undo a bunch of times
crash is in
bone_collections.cc/325
so maybe it's the same issue you discovered
and a usability feedback I have that doesn't need to be changed for this PR
calling it "Pose Mode override" instead of just "Pose Bone Color"
I feel like atm the UI is lying to me because it says "Pose Bone Color - Default Color" while it's not actually the default color but the edit mode color
Will the UI be messy if I have like 20 layers and 8 groups?
Or I should wirte my own python script to have a better view?
The layer panel from cloudrig is too good, I have to mention it.
https://gitlab.com/blender/CloudRig
I wish I cloud have something like the picture when I press M, rather than write my own script.
@yujiaxin Those are good thoughts, but are questions for future work. Right now we're just working on getting the basic functionality in.
eef2629628
to3cdbfd6e9a
@blender-bot package
Package build started. Download here when ready.
I've been digging into the crashes. And there is odd behavior around the edit mode.
Deleting a bone collection cannot be undone in edit mode. (In fact none of the operators can be undone in edit mode)
Deleting it in edit mode and then going to object mode to undo causes a crash.
As I learned there is a special undo mode for armature edit modes in
editarmature_undo.c
which doesn't take into account the new collections3cdbfd6e9a
toac648f9e49
@blender-bot package
Package build started. Download here when ready.
69f1ca3904
toc6e026a72f
c6e026a72f
toab07c48b81
Okay, so I've figured out what the crashes are, thanks to the investigation from @ChrisLend. The issue is simply that the armature undo code doesn't handle bone collections at all. There are two aspects to this:
Addressing this properly will require some thought, so in the interest of getting the PR landed today, I'm going to just prevent the crash by clearing bone collection membership of the copied undo edit bones. This means that undoing in edit mode will lose collection membership, but undo already isn't working with collections in edit mode, so this seems like a reasonable stop gap.
A proper fix is of course necessary before release, so I'll add it as the top priority task on my TODO list.
ab07c48b81
tod4af8e7f14
All the outstanding issues have been addressed well enough for initial landing. There are still outstanding issues that will need to be addressed after landing. Here are the outstanding issues I'm currently aware of, to be addressed after this lands:
edit_bones.new()
shouldn't add the bone to any collection. Done:ab67d410a9
ab67d410a9
830572a1bb