griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Anim: start slot handles at a high number

Browse Source 
The `Action::last_slot_handle` field is set to a high-ish value, to
disambiguate slot handles from array indices.

Slot handles are opaque integers, and are just used to find a slot
with that particular handle. Its exact value is irrelevant; Blender
only ensures that slot handles are never reused within the same
Action.

This particular value was obtained by taking the 31 most significant
bits of the TentHash value (Nathan's hash function) for the string
"Quercus&Laksa" (Sybren's cats).

Pull Request: https://projects.blender.org/blender/blender/pulls/131310
This commit is contained in:
Sybren A. Stüvel
2024-12-03 16:15:25 +01:00
parent 62897317bd
commit be921b8ddb
4 changed files with 62 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
source/blender/animrig/intern/action_test.cc
View File

@@ -12,6 +12,7 @@
#include "BKE_main.hh"
#include "BKE_object.hh"
#include "DNA_action_defaults.h"
#include "DNA_anim_types.h"
#include "DNA_object_types.h"
@@ -27,6 +28,17 @@
#include "testing/testing.h"
namespace blender::animrig::tests {
TEST(action, low_level_initialisation)
{
bAction *action = static_cast<bAction *>(BKE_id_new_nomain(ID_AC, "ACNewAction"));
EXPECT_NE(action->last_slot_handle, 0)
<< "bAction::last_slot_handle should not be initialised to 0";
BKE_id_free(nullptr, action);
}
class ActionLayersTest : public testing::Test {
public:
Main *bmain;
@@ -275,8 +287,8 @@ TEST_F(ActionLayersTest, add_slot)
{
{ /* Creating an 'unused' Slot should just be called 'Slot'. */
Slot &slot = action->slot_add();
EXPECT_EQ(1, action->last_slot_handle);
EXPECT_EQ(1, slot.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 1, action->last_slot_handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 1, slot.handle);
EXPECT_STREQ("XXSlot", slot.identifier);
EXPECT_EQ(0, slot.idtype);
@@ -284,8 +296,8 @@ TEST_F(ActionLayersTest, add_slot)
{ /* Creating a Slot for a specific ID should name it after the ID. */
Slot &slot = action->slot_add_for_id(cube->id);
EXPECT_EQ(2, action->last_slot_handle);
EXPECT_EQ(2, slot.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 2, action->last_slot_handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 2, slot.handle);
EXPECT_STREQ(cube->id.name, slot.identifier);
EXPECT_EQ(ID_OB, slot.idtype);
@@ -315,20 +327,20 @@ TEST_F(ActionLayersTest, add_slot_multiple)
EXPECT_TRUE(assign_action(action, suzanne->id));
EXPECT_EQ(assign_action_slot(&slot_suzanne, suzanne->id), ActionSlotAssignmentResult::OK);
EXPECT_EQ(2, action->last_slot_handle);
EXPECT_EQ(1, slot_cube.handle);
EXPECT_EQ(2, slot_suzanne.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 2, action->last_slot_handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 1, slot_cube.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 2, slot_suzanne.handle);
}
TEST_F(ActionLayersTest, slot_remove)
{
{ /* Canary test: removing a just-created slot on an otherwise empty Action should work. */
Slot &slot = action->slot_add();
EXPECT_EQ(1, slot.handle);
EXPECT_EQ(1, action->last_slot_handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 1, slot.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 1, action->last_slot_handle);
EXPECT_TRUE(action->slot_remove(slot));
EXPECT_EQ(1, action->last_slot_handle)
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 1, action->last_slot_handle)
<< "Removing a slot should not change the last-used slot handle.";
EXPECT_EQ(0, action->slot_array_num);
}
@@ -341,8 +353,8 @@ TEST_F(ActionLayersTest, slot_remove)
{ /* Removing a slot should remove its Channelbag. */
Slot &slot = action->slot_add();
const slot_handle_t slot_handle = slot.handle;
EXPECT_EQ(2, slot.handle);
EXPECT_EQ(2, action->last_slot_handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 2, slot.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 2, action->last_slot_handle);
/* Create an F-Curve in a Channelbag for the slot. */
action->layer_keystrip_ensure();
@@ -352,7 +364,7 @@ TEST_F(ActionLayersTest, slot_remove)
/* Remove the slot. */
EXPECT_TRUE(action->slot_remove(slot));
EXPECT_EQ(2, action->last_slot_handle)
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 2, action->last_slot_handle)
<< "Removing a slot should not change the last-used slot handle.";
EXPECT_EQ(0, action->slot_array_num);
@@ -365,10 +377,10 @@ TEST_F(ActionLayersTest, slot_remove)
Slot &slot1 = action->slot_add();
Slot &slot2 = action->slot_add();
Slot &slot3 = action->slot_add();
EXPECT_EQ(3, slot1.handle);
EXPECT_EQ(4, slot2.handle);
EXPECT_EQ(5, slot3.handle);
EXPECT_EQ(5, action->last_slot_handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 3, slot1.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 4, slot2.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 5, slot3.handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 5, action->last_slot_handle);
/* For referencing the slot handle after the slot is removed. */
const slot_handle_t slot2_handle = slot2.handle;
@@ -382,7 +394,7 @@ TEST_F(ActionLayersTest, slot_remove)
/* Remove the slot. */
EXPECT_TRUE(action->slot_remove(slot2));
EXPECT_EQ(5, action->last_slot_handle);
EXPECT_EQ(DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE + 5, action->last_slot_handle);
/* Check the correct slot + channel-bag are removed. */
EXPECT_EQ(action->slot_for_handle(slot1.handle), &slot1);

12
source/blender/blenkernel/intern/action.cc
View File

@@ -20,6 +20,7 @@
#include "DNA_anim_types.h"
#include "DNA_armature_types.h"
#include "DNA_constraint_types.h"
#include "DNA_defaults.h"
#include "DNA_object_types.h"
#include "DNA_scene_types.h"
@@ -96,6 +97,15 @@ using namespace blender;
/*********************** Armature Datablock ***********************/
namespace blender::bke {
static void action_init_data(ID *action_id)
{
BLI_assert(GS(action_id->name) == ID_AC);
bAction *action = reinterpret_cast<bAction *>(action_id);
BLI_assert(MEMCMP_STRUCT_AFTER_IS_ZERO(action, id));
MEMCPY_STRUCT_AFTER(action, DNA_struct_default_get(bAction), id);
}
/**
* Only copy internal data of Action ID from source
* to already allocated/initialized destination.
@@ -745,7 +755,7 @@ IDTypeInfo IDType_ID_AC = {
/*flags*/ IDTYPE_FLAGS_NO_ANIMDATA,
/*asset_type_info*/ &blender::bke::AssetType_AC,
/*init_data*/ nullptr,
/*init_data*/ blender::bke::action_init_data,
/*copy_data*/ blender::bke::action_copy_data,
/*free_data*/ blender::bke::action_free_data,
/*make_local*/ nullptr,

19
source/blender/makesdna/DNA_action_defaults.h
View File

@@ -12,6 +12,25 @@
/* clang-format off */
/* -------------------------------------------------------------------- */
/** \name Action Struct
* \{ */
/* The last_slot_handle is set to a high value to disambiguate slot handles from
* array indices.
*
* This particular value was obtained by taking the 31 most-significant bits of
* the TentHash value (Nathan's hash function) for the string "Quercus&Laksa"
* (Sybren's cats). */
#define DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE 0x37627bf5
#define _DNA_DEFAULT_bAction \
{ \
.last_slot_handle = DNA_DEFAULT_ACTION_LAST_SLOT_HANDLE, \
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name ActionLayer Struct
* \{ */

2
source/blender/makesdna/intern/dna_defaults.c
View File

@@ -149,6 +149,7 @@
static const struct_name DNA_DEFAULT_##struct_name = _DNA_DEFAULT_##struct_name
/* DNA_action_defaults.h */
SDNA_DEFAULT_DECL_STRUCT(bAction);
SDNA_DEFAULT_DECL_STRUCT(ActionLayer);
SDNA_DEFAULT_DECL_STRUCT(ActionStrip);
@@ -399,6 +400,7 @@ extern const bTheme U_theme_default;
const void *DNA_default_table[SDNA_TYPE_MAX] = {
/* DNA_anim_defaults.h */
SDNA_DEFAULT_DECL(bAction),
SDNA_DEFAULT_DECL(ActionLayer),
SDNA_DEFAULT_DECL(ActionStrip),