From bbf40d214cddf5c3399f11e20cae09565fa43a4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 15 Oct 2025 12:54:36 +0200 Subject: [PATCH 1/2] Fix #147729: Crash scrubbing w/ snapping to sec and FPS < 0.5 Fix playhead snapping to seconds, when the frame rate is less than 0.5 FPS. This makes it possible to snap to fractions of frames, to support setups with multiple seconds per frame. Of course this only has any effect (apart from not crashing) when sub-frames are enabled. I've also added unit tests, and verified that the values are the same from before this refactor. Pull Request: https://projects.blender.org/blender/blender/pulls/148074 --- source/blender/blenkernel/BKE_scene.hh | 2 +- source/blender/blenkernel/CMakeLists.txt | 1 + source/blender/blenkernel/intern/scene.cc | 19 +++++--- .../blender/blenkernel/intern/scene_test.cc | 44 +++++++++++++++++++ 4 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 source/blender/blenkernel/intern/scene_test.cc diff --git a/source/blender/blenkernel/BKE_scene.hh b/source/blender/blenkernel/BKE_scene.hh index 7b6986ada01..2273ca70e50 100644 --- a/source/blender/blenkernel/BKE_scene.hh +++ b/source/blender/blenkernel/BKE_scene.hh @@ -138,7 +138,7 @@ const char *BKE_scene_find_marker_name(const Scene *scene, int frame); */ const char *BKE_scene_find_last_marker_name(const Scene *scene, int frame); -int BKE_scene_frame_snap_by_seconds(Scene *scene, double interval_in_seconds, int frame); +float BKE_scene_frame_snap_by_seconds(const Scene *scene, double interval_in_seconds, float frame); /** * Checks for cycle, returns true if it's all OK. diff --git a/source/blender/blenkernel/CMakeLists.txt b/source/blender/blenkernel/CMakeLists.txt index 8e1dee7613a..c3fdc0bfddb 100644 --- a/source/blender/blenkernel/CMakeLists.txt +++ b/source/blender/blenkernel/CMakeLists.txt @@ -814,6 +814,7 @@ if(WITH_GTESTS) intern/main_test.cc intern/nla_test.cc intern/path_templates_test.cc + intern/scene_test.cc intern/subdiv_ccg_test.cc intern/tracking_test.cc intern/volume_test.cc diff --git a/source/blender/blenkernel/intern/scene.cc b/source/blender/blenkernel/intern/scene.cc index 2adeea30ee2..82015441756 100644 --- a/source/blender/blenkernel/intern/scene.cc +++ b/source/blender/blenkernel/intern/scene.cc @@ -2291,14 +2291,19 @@ const char *BKE_scene_find_last_marker_name(const Scene *scene, int frame) return best_marker ? best_marker->name : nullptr; } -int BKE_scene_frame_snap_by_seconds(Scene *scene, double interval_in_seconds, int frame) +float BKE_scene_frame_snap_by_seconds(const Scene *scene, + const double interval_in_seconds, + const float frame) { - const int fps = round_db_to_int(scene->frames_per_second() * interval_in_seconds); - const int second_prev = frame - mod_i(frame, fps); - const int second_next = second_prev + fps; - const int delta_prev = frame - second_prev; - const int delta_next = second_next - frame; - return (delta_prev < delta_next) ? second_prev : second_next; + BLI_assert(interval_in_seconds > 0); + BLI_assert(scene->frames_per_second() > 0); + + const double interval_in_frames = scene->frames_per_second() * interval_in_seconds; + const double second_prev = interval_in_frames * floor(frame / interval_in_frames); + const double second_next = second_prev + ceil(interval_in_frames); + const double delta_prev = frame - second_prev; + const double delta_next = second_next - frame; + return float((delta_prev < delta_next) ? second_prev : second_next); } void BKE_scene_remove_rigidbody_object(Main *bmain, Scene *scene, Object *ob, const bool free_us) diff --git a/source/blender/blenkernel/intern/scene_test.cc b/source/blender/blenkernel/intern/scene_test.cc new file mode 100644 index 00000000000..965f0855cf4 --- /dev/null +++ b/source/blender/blenkernel/intern/scene_test.cc @@ -0,0 +1,44 @@ +/* SPDX-FileCopyrightText: 2025 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +#include "BKE_scene.hh" + +#include "DNA_scene_types.h" + +#include "testing/testing.h" + +namespace blender::bke::tests { + +TEST(scene, frame_snap_by_seconds) +{ + Scene fake_scene = {}; + + /* Regular 24 FPS snapping. */ + fake_scene.r.frs_sec = 24; + fake_scene.r.frs_sec_base = 1.0; + EXPECT_FLOAT_EQ(48.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 47)); + EXPECT_FLOAT_EQ(48.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 49)); + EXPECT_FLOAT_EQ(48.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 59)); + EXPECT_FLOAT_EQ(72.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 60)); + EXPECT_FLOAT_EQ(9984.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 2.0, 10000.0)); + + /* 12 FPS snapping by incrementing the base. */ + fake_scene.r.frs_sec = 24; + fake_scene.r.frs_sec_base = 2.0; + EXPECT_FLOAT_EQ(48.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 47)); + EXPECT_FLOAT_EQ(48.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 49)); + EXPECT_FLOAT_EQ(48.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 53)); + EXPECT_FLOAT_EQ(60.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 54)); + EXPECT_FLOAT_EQ(9996.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 1.0, 10000.0)); + + /* 0.1 FPS snapping to 2-second intervals. */ + fake_scene.r.frs_sec = 1; + fake_scene.r.frs_sec_base = 10.0; + EXPECT_FLOAT_EQ(48.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 2.0, 48.0)); + EXPECT_FLOAT_EQ(48.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 2.0, 48.1)); + EXPECT_FLOAT_EQ(48.2, BKE_scene_frame_snap_by_seconds(&fake_scene, 2.0, 48.2)); + EXPECT_FLOAT_EQ(10000.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 2.0, 10000.0)); +} + +} // namespace blender::bke::tests From 40e61d4240995080691307a3d5780ed785a21cbd Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 15 Oct 2025 15:09:30 +0200 Subject: [PATCH 2/2] Fix #147978: Missed conversion to SystemProperties for children bones. Forgot that the Armature's bones list only contain root bones... Fix the versioning code. Also adding a 'recovery' extra versioning step for files that may have already been opened and re-saved in Blender 5.0 (though this step is not 100% handling all cases, in case some script or add-on already created some system properties in a bone in 5.0, the existing user properties from 4.5 and before won't be copied over anymore). Pull Request: https://projects.blender.org/blender/blender/pulls/148125 --- .../blender/blenkernel/BKE_blender_version.h | 2 +- source/blender/blenloader/intern/readfile.cc | 5 +++ .../blenloader/intern/versioning_500.cc | 33 ++++++++++++++++++- .../blenloader/intern/versioning_common.hh | 1 + 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/source/blender/blenkernel/BKE_blender_version.h b/source/blender/blenkernel/BKE_blender_version.h index 999b4d2b641..1a2cab248ac 100644 --- a/source/blender/blenkernel/BKE_blender_version.h +++ b/source/blender/blenkernel/BKE_blender_version.h @@ -27,7 +27,7 @@ /* Blender file format version. */ #define BLENDER_FILE_VERSION BLENDER_VERSION -#define BLENDER_FILE_SUBVERSION 109 +#define BLENDER_FILE_SUBVERSION 110 /* Minimum Blender version that supports reading file written with the current * version. Older Blender versions will test this and cancel loading the file, showing a warning to diff --git a/source/blender/blenloader/intern/readfile.cc b/source/blender/blenloader/intern/readfile.cc index 196ad3ca5e8..9ba230ebc46 100644 --- a/source/blender/blenloader/intern/readfile.cc +++ b/source/blender/blenloader/intern/readfile.cc @@ -3501,6 +3501,11 @@ static void do_versions(FileData *fd, Library *lib, Main *main) * later during 5.0 development process. */ version_system_idprops_nodes_generate(main); } + if (!MAIN_VERSION_FILE_ATLEAST(main, 500, 110)) { + /* Same as above, but children bones were missed by initial versioning code, attempt to + * transfer idprops data still in case they have no system properties defined yet. */ + version_system_idprops_children_bones_generate(main); + } if (G.debug & G_DEBUG) { char build_commit_datetime[32]; diff --git a/source/blender/blenloader/intern/versioning_500.cc b/source/blender/blenloader/intern/versioning_500.cc index 4aca1992862..c7273c791d0 100644 --- a/source/blender/blenloader/intern/versioning_500.cc +++ b/source/blender/blenloader/intern/versioning_500.cc @@ -130,8 +130,16 @@ void version_system_idprops_generate(Main *bmain) for (BoneCollection *bcoll : armature->collections_span()) { idprops_process(bcoll->prop, &bcoll->system_properties); } - LISTBASE_FOREACH (Bone *, bone, &armature->bonebase) { + /* There is no way to iterate directly over all bones of an armature currently, use a recursive + * approach instead. */ + auto process_bone_recursive = [](const auto &process_bone_recursive, Bone *bone) -> void { idprops_process(bone->prop, &bone->system_properties); + LISTBASE_FOREACH (Bone *, bone_it, &bone->childbase) { + process_bone_recursive(process_bone_recursive, bone_it); + } + }; + LISTBASE_FOREACH (Bone *, bone_it, &armature->bonebase) { + process_bone_recursive(process_bone_recursive, bone_it); } } } @@ -145,6 +153,29 @@ void version_system_idprops_nodes_generate(Main *bmain) } FOREACH_NODETREE_END; } +/* Separate callback for non-root bones, because they were missed in the initial implementation. */ +void version_system_idprops_children_bones_generate(Main *bmain) +{ + LISTBASE_FOREACH (bArmature *, armature, &bmain->armatures) { + /* There is no way to iterate directly over all bones of an armature currently, use a recursive + * approach instead. */ + auto process_bone_recursive = [](const auto &process_bone_recursive, Bone *bone) -> void { + /* Do not overwrite children bones' system properties if they were already defined by some + * scripts or add-on e.g. */ + if (bone->system_properties == nullptr) { + idprops_process(bone->prop, &bone->system_properties); + } + LISTBASE_FOREACH (Bone *, bone_it, &bone->childbase) { + process_bone_recursive(process_bone_recursive, bone_it); + } + }; + LISTBASE_FOREACH (Bone *, bone_it, &armature->bonebase) { + LISTBASE_FOREACH (Bone *, bone_child_it, &bone_it->childbase) { + process_bone_recursive(process_bone_recursive, bone_child_it); + } + } + } +} static CustomDataLayer *find_old_seam_layer(CustomData &custom_data, const blender::StringRef name) { diff --git a/source/blender/blenloader/intern/versioning_common.hh b/source/blender/blenloader/intern/versioning_common.hh index 0f83f2e48f3..0dd284787e3 100644 --- a/source/blender/blenloader/intern/versioning_common.hh +++ b/source/blender/blenloader/intern/versioning_common.hh @@ -218,6 +218,7 @@ bNode *version_eevee_output_node_get(bNodeTree *ntree, int16_t node_type); */ void version_system_idprops_generate(Main *bmain); void version_system_idprops_nodes_generate(Main *bmain); +void version_system_idprops_children_bones_generate(Main *bmain); bool all_scenes_use(Main *bmain, const blender::Span engines);