ilusha/blender
1
0
Fork 0
mirror of https://github.com/blender/blender synced 2026-09-27 00:43:19 +03:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix #118509: Make sure to use valid indices in frame maps

Browse source 
Moving a set of keyframes could cause crashes by setting invalid
`drawing_index` in `GreasePencilFrame` data.

The transform operator for grease pencil keyframes can add and remove
keyframes by overwriting existing frames. The `move_duplicate_frames`
function in particular has to keep track of drawing user counts to
ensure that the drawings referenced by the frames are still alive at the
end.

This was broken when moving multiple keyframes at once, such that later
keyframes would overwrite the target positions for earlier frames (for
example moving frames [1,2,3] to [2,3,4]). The `move_duplicate_frames`
was first removing the source frame and then adding it back at the
destination. In case the source frame was already the destination of an
earlier keyframe, this will cause incorrect user count because the frame
being removed is not the same as the one being added back.

To avoid this problem, remove all the source keyframes first before any
other modification of the destination layer. That way we can be sure the
frames at the source index is actually the expected frame.

Pull Request: https://projects.blender.org/blender/blender/pulls/119207
This commit is contained in:
Lukas Tönne 2024-03-11 12:50:19 +01:00
parent 51bcaad457
commit f04bf1694f
1 changed files with 25 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

43
source/blender/blenkernel/intern/grease_pencil.cc
View file

@ -2042,30 +2042,37 @@ void GreasePencil::move_duplicate_frames(
Map<int, GreasePencilFrame> layer_frames_copy = layer.frames();
/* Copy frames durations. */
Map<int, int> layer_frames_durations;
Map<int, int> src_layer_frames_durations;
for (const auto [frame_number, frame] : layer.frames().items()) {
if (!frame.is_implicit_hold()) {
layer_frames_durations.add(frame_number, layer.get_frame_duration_at(frame_number));
src_layer_frames_durations.add(frame_number, layer.get_frame_duration_at(frame_number));
}
}
for (const auto [src_frame_number, dst_frame_number] : frame_number_destinations.items()) {
const bool use_duplicate = duplicate_frames.contains(src_frame_number);
const Map<int, GreasePencilFrame> &frame_map = use_duplicate ? duplicate_frames :
layer_frames_copy;
if (!frame_map.contains(src_frame_number)) {
continue;
}
const GreasePencilFrame src_frame = frame_map.lookup(src_frame_number);
const int drawing_index = src_frame.drawing_index;
const int duration = layer_frames_durations.lookup_default(src_frame_number, 0);
if (!use_duplicate) {
/* Remove original frames for duplicates before inserting any frames.
* This has to be done early to avoid removing frames that may be inserted
* in place of the source frames. */
for (const auto src_frame_number : frame_number_destinations.keys()) {
if (!duplicate_frames.contains(src_frame_number)) {
/* User count not decremented here, the same frame is inserted again later. */
layer.remove_frame(src_frame_number);
}
}
auto get_source_frame = [&](const int frame_number) -> const GreasePencilFrame * {
if (const GreasePencilFrame *ptr = duplicate_frames.lookup_ptr(frame_number)) {
return ptr;
}
return layer_frames_copy.lookup_ptr(frame_number);
};
for (const auto [src_frame_number, dst_frame_number] : frame_number_destinations.items()) {
const GreasePencilFrame *src_frame = get_source_frame(src_frame_number);
if (!src_frame) {
continue;
}
const int drawing_index = src_frame->drawing_index;
const int duration = src_layer_frames_durations.lookup_default(src_frame_number, 0);
/* Add and overwrite the frame at the destination number. */
if (layer.frames().contains(dst_frame_number)) {
@ -2077,7 +2084,7 @@ void GreasePencil::move_duplicate_frames(
layer.remove_frame(dst_frame_number);
}
GreasePencilFrame *frame = layer.add_frame(dst_frame_number, drawing_index, duration);
*frame = src_frame;
*frame = *src_frame;
}
/* Remove drawings if they no longer have users. */