Initial Grease Pencil 3.0 stage #106848

Merged
Falk David merged 224 commits from filedescriptor/blender:grease-pencil-v3 into main 2023-05-30 11:14:22 +02:00
5 changed files with 76 additions and 34 deletions
Showing only changes of commit 0b9a5381f9 - Show all commits

View File

@ -223,10 +223,11 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
class Layer : public TreeNode, ::GreasePencilLayer {
private:
/**
* This Map maps a scene frame number (key) to an index into GreasePencil->drawings (value). The
* frame number indicates the first frame the drawing is shown. The end time is implicitly
* defined by the next greater frame number (key) in the map. If the value mapped to (index) is
* -1, no drawing is shown at this frame.
* This Map maps a scene frame number (key) to a GreasePencilFrame. This struct holds an index
* (drawing_index) to the drawing in the GreasePencil->drawings array. The frame number indicates
* the first frame the drawing is shown. The end time is implicitly defined by the next greater
* frame number (key) in the map. If the value mapped to (index) is -1, no drawing is shown at
* this frame.
*
* \example:
*
@ -246,7 +247,7 @@ class Layer : public TreeNode, ::GreasePencilLayer {
* referenced drawings are discarded. If the frame is longer than the number of referenced
* drawings, then the last referenced drawing is held for the rest of the duration.
*/
filedescriptor marked this conversation as resolved Outdated

Since the map will have to change to store more than just an index as the value, make sure to update the comment too.

Since the map will have to change to store more than just an index as the value, make sure to update the comment too.
Map<int, int> frames_;
Map<int, GreasePencilFrame> frames_;
mutable SharedCache<Vector<int>> sorted_keys_cache_;
public:
@ -271,27 +272,39 @@ class Layer : public TreeNode, ::GreasePencilLayer {
return this != &other;
}
const Map<int, int> &frames()
const Map<int, GreasePencilFrame> &frames()
{

Not sure why the greasepencil namespace ends up here? Also means that e.g. the fairly generically-named StrokePoint struct is in the fairly generic blender::bke namespace, which does not sounds great to me? not to mention 'C-style' namespace in names like GreasePencilDrawingRuntime.

Not sure why the `greasepencil` namespace ends up here? Also means that e.g. the fairly generically-named `StrokePoint` struct is in the fairly generic `blender::bke` namespace, which does not sounds great to me? not to mention 'C-style' namespace in names like `GreasePencilDrawingRuntime`.
return frames_;
}
Map<int, int> &frames_for_write()
Map<int, GreasePencilFrame> &frames_for_write()
{
sorted_keys_cache_.tag_dirty();
return frames_;
}
bool insert_frame(int frame_number, int index)
bool insert_frame(int frame_number, GreasePencilFrame &frame)
{
sorted_keys_cache_.tag_dirty();
return frames_for_write().add(frame_number, index);
return frames_for_write().add(frame_number, frame);
}
bool overwrite_frame(int frame_number, int index)
bool insert_frame(int frame_number, GreasePencilFrame &&frame)
{
sorted_keys_cache_.tag_dirty();
return frames_for_write().add_overwrite(frame_number, index);
return frames_for_write().add(frame_number, frame);
}

= {} doesn't change anything here since Vector has a default constructor. It can be removed I think. Same with below

`= {}` doesn't change anything here since `Vector` has a default constructor. It can be removed I think. Same with below

Put out of line.

Put out of line.
bool overwrite_frame(int frame_number, GreasePencilFrame &frame)
{
sorted_keys_cache_.tag_dirty();
return frames_for_write().add_overwrite(frame_number, frame);
}

Looks like this should be private maybe? It has a _ suffix.

Looks like this should be private maybe? It has a `_` suffix.
bool overwrite_frame(int frame_number, GreasePencilFrame &&frame)
{
sorted_keys_cache_.tag_dirty();
return frames_for_write().add_overwrite(frame_number, frame);
}
Span<int> sorted_keys()

Put out of line.

Put out of line.
@ -326,14 +339,14 @@ class Layer : public TreeNode, ::GreasePencilLayer {
}
/* After or at the last drawing, return the last drawing. */
if (frame >= sorted_keys.last()) {
return frames().lookup(sorted_keys.last());
return frames().lookup(sorted_keys.last()).drawing_index;
}
/* Search for the drawing. upper_bound will get the drawing just after. */
auto it = std::upper_bound(sorted_keys.begin(), sorted_keys.end(), frame);
if (it == sorted_keys.end() || it == sorted_keys.begin()) {
return -1;

Put out of line.

Put out of line.
}
return frames().lookup(*std::prev(it));
return frames().lookup(*std::prev(it)).drawing_index;
}
};

View File

@ -520,13 +520,13 @@ void GreasePencil::remove_drawing(int index_to_remove)
if (index_to_remove != last_drawing_index) {
this->root_group().foreach_layer_pre_order(
[last_drawing_index, index_to_remove](Layer &layer) {
blender::Map<int, int> &frames = layer.frames_for_write();
for (auto item : frames.items()) {
if (item.value == last_drawing_index) {
item.value = index_to_remove;
blender::Map<int, GreasePencilFrame> &frames = layer.frames_for_write();
for (auto [key, value] : frames.items()) {
if (value.drawing_index == last_drawing_index) {
value.drawing_index = index_to_remove;
}
else if (item.value == index_to_remove) {
item.value = last_drawing_index;
else if (value.drawing_index == index_to_remove) {
value.drawing_index = last_drawing_index;
}
}
});
@ -557,8 +557,10 @@ void GreasePencil::remove_drawing(int index_to_remove)
/* Remove any frame that points to the last drawing. */
this->root_group().foreach_layer_pre_order([last_drawing_index](Layer &layer) {
blender::Map<int, int> &frames = layer.frames_for_write();
frames.remove_if([last_drawing_index](auto item) { return item.value == last_drawing_index; });
blender::Map<int, GreasePencilFrame> &frames = layer.frames_for_write();
frames.remove_if([last_drawing_index](auto item) {
return item.value.drawing_index == last_drawing_index;
});
});
/* Shrink drawing array. */
@ -711,10 +713,10 @@ static void save_layer_to_storage(blender::bke::gpencil::Layer &node,
int frames_size = node.frames().size();
new_leaf->layer.frames_storage.size = frames_size;
new_leaf->layer.frames_storage.keys = MEM_cnew_array<int>(frames_size, __func__);
new_leaf->layer.frames_storage.values = MEM_cnew_array<int>(frames_size, __func__);
new_leaf->layer.frames_storage.values = MEM_cnew_array<GreasePencilFrame>(frames_size, __func__);
MutableSpan<int> keys{new_leaf->layer.frames_storage.keys, frames_size};
MutableSpan<int> values{new_leaf->layer.frames_storage.values, frames_size};
MutableSpan<GreasePencilFrame> values{new_leaf->layer.frames_storage.values, frames_size};
keys.copy_from(node.sorted_keys());
for (int i : keys.index_range()) {
values[i] = node.frames().lookup(keys[i]);
@ -820,8 +822,7 @@ void GreasePencil::read_layer_tree_storage(BlendDataReader *reader)
/* Read layer data. */
BLO_read_int32_array(
reader, node_leaf->layer.frames_storage.size, &node_leaf->layer.frames_storage.keys);
BLO_read_int32_array(
reader, node_leaf->layer.frames_storage.size, &node_leaf->layer.frames_storage.values);
BLO_read_data_address(reader, &node_leaf->layer.frames_storage.values);
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
@ -847,8 +848,10 @@ void GreasePencil::write_layer_tree_storage(BlendWriter *writer)
/* Write layer data. */
BLO_write_int32_array(
writer, node_leaf->layer.frames_storage.size, node_leaf->layer.frames_storage.keys);
BLO_write_int32_array(
writer, node_leaf->layer.frames_storage.size, node_leaf->layer.frames_storage.values);
BLO_write_struct_array(writer,
GreasePencilFrame,
node_leaf->layer.frames_storage.size,
node_leaf->layer.frames_storage.values);
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {

View File

@ -121,7 +121,11 @@ void legacy_gpencil_to_grease_pencil(GreasePencil &grease_pencil, bGPdata &gpd)
/* Convert the frame to a drawing. */
legacy_gpencil_frame_to_curves_geometry(drawing, *gpf);
new_layer.insert_frame(gpf->framenum, i);
GreasePencilFrame frame;
frame.drawing_index = i;
frame.type = gpf->key_type;
new_layer.insert_frame(gpf->framenum, std::move(frame));
i++;
}
}

View File

@ -63,11 +63,11 @@ TEST(gpencil, remove_drawing)
Layer layer1("Layer1");
Layer layer2("Layer2");
layer1.insert_frame(0, 0);
layer1.insert_frame(10, 1);
layer1.insert_frame(20, 2);
layer1.insert_frame(0, GreasePencilFrame{0});
layer1.insert_frame(10, GreasePencilFrame{1});
layer1.insert_frame(20, GreasePencilFrame{2});
layer2.insert_frame(0, 1);
layer2.insert_frame(0, GreasePencilFrame{1});
grease_pencil.root_group().add_layer(std::move(layer1));
grease_pencil.root_group().add_layer(std::move(layer2));
@ -82,7 +82,7 @@ TEST(gpencil, remove_drawing)
EXPECT_EQ(layer.frames().size(), expected_frames_size[layer_i]);
if (layer_i == 0) {
for (const int i : IndexRange(2)) {
EXPECT_EQ(layer.frames().lookup(expected_frames_pairs_layer0[i][0]),
EXPECT_EQ(layer.frames().lookup(expected_frames_pairs_layer0[i][0]).drawing_index,
expected_frames_pairs_layer0[i][1]);
}
}

View File

@ -91,6 +91,28 @@ typedef struct GreasePencilDrawingReference {
struct GreasePencil *id_reference;
} GreasePencilDrawingReference;
/**
* A GreasePencilFrame is a single keyframe in the timeline.
* It references a drawing by index into the drawing array.
*/
typedef struct GreasePencilFrame {
/**
* Index into the GreasePencil->drawings array.
*/
int drawing_index;
/**
* Flag. Used to set e.g. the selection.
*/
uint32_t flag;
/**
* Keyframe type.
*/
uint8_t type;

What about a single function that returned std::optional rather than a separate "has_" function?

What about a single function that returned `std::optional` rather than a separate "has_" function?
char _pad[3];

Looks like stroke_buffer() can be const

Looks like `stroke_buffer()` can be `const`
} GreasePencilFrame;
/**
* Storage for the Map in `blender::bke::gpencil::Layer`.
* See the description there for more detail.
@ -99,7 +121,7 @@ typedef struct GreasePencilLayerFramesMapStorage {
/* Array of `frames` keys (sorted in ascending order). */
int *keys;
/* Array of `frames` values (order matches the keys array). */
int *values;
GreasePencilFrame *values;
/* Size of the map (number of key-value pairs). */
int size;
char _pad[4];