blender
/
blender
133
398
Fork 579
Code Issues 6.3k Pull Requests 747 Projects 19 Wiki Activity

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
Conversation 8 Commits 224 Files Changed 94 +71 -77
2 changed files with 71 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1e85673349 - Show all commits

87
source/blender/blenkernel/BKE_grease_pencil.hh
View File

@ -126,9 +126,7 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
TreeNode *_root;
public:
explicit PreOrderRange(TreeNode *root) : _root(root)
{
}
explicit PreOrderRange(TreeNode *root) : _root(root) {}
Iterator begin()
{
@ -191,16 +189,6 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
}
}
void save_to_storage(GreasePencilLayerTreeNode *dst)
{
dst->type = this->type;
copy_v3_v3_uchar(dst->color, this->color);
dst->flag = this->flag;
if (this->name) {
dst->name = BLI_strdup(this->name);
}
}
private:
void foreach_children_pre_order_recursive_(TreeNodeIterFn function)
filedescriptor marked this conversation as resolved Outdated
Bastien Montagne commented 2023-05-24 18:36:55 +02:00
Outdated
Review
Copy Link

Would be good to know why this is commented out, or it should be cleaned up before merge in main.

Would be good to know why this is commented out, or it should be cleaned up before merge in main.
{
@ -251,12 +239,8 @@ class Layer : public TreeNode, ::GreasePencilLayer {
mutable SharedCache<Vector<int>> sorted_keys_cache_;
public:
Layer() : TreeNode(GREASE_PENCIL_LAYER_TREE_LEAF), frames_()
{
}
Layer(StringRefNull name) : TreeNode(GREASE_PENCIL_LAYER_TREE_LEAF, name), frames_()
{
}
Layer() : TreeNode(GREASE_PENCIL_LAYER_TREE_LEAF), frames_() {}
Layer(StringRefNull name) : TreeNode(GREASE_PENCIL_LAYER_TREE_LEAF, name), frames_() {}
Layer(const Layer &other) : TreeNode(other)
{
frames_ = other.frames_;
@ -265,9 +249,7 @@ class Layer : public TreeNode, ::GreasePencilLayer {
{
filedescriptor marked this conversation as resolved Outdated
Falk David commented 2023-04-12 10:41:05 +02:00
Outdated
Review
Copy Link

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.
frames_ = std::move(other.frames_);
}
~Layer()
{
}
~Layer() {}
bool operator==(const Layer &other) const
{
@ -283,6 +265,11 @@ class Layer : public TreeNode, ::GreasePencilLayer {
return frames_;
}
Map<int, int> &frames_for_write()
{
return frames_;
}
bool insert_frame(int frame_number, int index)
{
sorted_keys_cache_.tag_dirty();
@ -307,29 +294,6 @@ class Layer : public TreeNode, ::GreasePencilLayer {
return sorted_keys_cache_.data().as_span();
}
Hans Goudey commented 2023-05-02 23:36:18 +02:00
Outdated
Review
Copy Link

= {} 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
void save_to_storage(GreasePencilLayerTreeNode **dst)
{
GreasePencilLayerTreeLeaf *new_leaf = MEM_cnew<GreasePencilLayerTreeLeaf>(__func__);
/* Save properties. */
TreeNode::save_to_storage(&new_leaf->base);
/* Save frames map. */
int frames_size = 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__);
MutableSpan<int> keys{new_leaf->layer.frames_storage.keys, frames_size};
MutableSpan<int> values{new_leaf->layer.frames_storage.values, frames_size};
keys.copy_from(sorted_keys());
for (int i : keys.index_range()) {
values[i] = frames_.lookup(keys[i]);
}
/* Store pointer. */
*dst = reinterpret_cast<GreasePencilLayerTreeNode *>(new_leaf);
}
void load_from_storage(GreasePencilLayer &node)
Falk David commented 2023-04-12 10:41:37 +02:00
Outdated
Review
Copy Link

Put out of line.

Put out of line.
{
for (int i = 0; i < node.frames_storage.size; i++) {
@ -362,12 +326,8 @@ class Layer : public TreeNode, ::GreasePencilLayer {
Hans Goudey commented 2023-04-25 04:52:58 +02:00
Outdated
Review
Copy Link

These two functions should never have to be called outside of the ID callbacks in grease_pencil.cc; IMO they make more sense as static functions there. Probably better to keep that storage for DNA thing as localized as possible.

These two functions should never have to be called outside of the `ID` callbacks in `grease_pencil.cc`; IMO they make more sense as static functions there. Probably better to keep that storage for DNA thing as localized as possible.
class LayerGroup : public TreeNode {
public:
LayerGroup() : TreeNode(GREASE_PENCIL_LAYER_TREE_GROUP)
{
}
LayerGroup(StringRefNull name) : TreeNode(GREASE_PENCIL_LAYER_TREE_GROUP, name)
{
}
LayerGroup() : TreeNode(GREASE_PENCIL_LAYER_TREE_GROUP) {}
LayerGroup(StringRefNull name) : TreeNode(GREASE_PENCIL_LAYER_TREE_GROUP, name) {}
LayerGroup(const LayerGroup &other) : TreeNode(other)
Hans Goudey commented 2023-04-25 04:53:30 +02:00
Outdated
Review
Copy Link

Member variables come before functions https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Class_Layout

Member variables come before functions https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Class_Layout
{
children_.reserve(other.children_.size());
@ -394,9 +354,7 @@ class LayerGroup : public TreeNode {
// }
// other.children_.clear();
}
~LayerGroup()
{
}
~LayerGroup() {}
public:
Falk David commented 2023-04-12 10:42:09 +02:00
Outdated
Review
Copy Link

This needs to be implemented.

This needs to be implemented.
bool operator==(const LayerGroup &other) const
@ -440,19 +398,6 @@ class LayerGroup : public TreeNode {
BLI_assert(index >= 0 && index < children_.size());
children_.remove(index);
}
void save_to_storage(GreasePencilLayerTreeNode **dst)
{
GreasePencilLayerTreeGroup *new_group = MEM_cnew<GreasePencilLayerTreeGroup>(__func__);
/* Save properties. */
TreeNode::save_to_storage(&new_group->base);
/* Save number of children. */
new_group->children_num = this->num_children();
/* Store pointer. */
*dst = reinterpret_cast<GreasePencilLayerTreeNode *>(new_group);
}
};
namespace convert {
@ -477,12 +422,8 @@ class GreasePencilRuntime {
LayerGroup root_group_;
public:
GreasePencilRuntime()
{
}
GreasePencilRuntime(const GreasePencilRuntime &other) : root_group_(other.root_group_)
{
}
GreasePencilRuntime() {}
GreasePencilRuntime(const GreasePencilRuntime &other) : root_group_(other.root_group_) {}
LayerGroup &root_group()
{

61
source/blender/blenkernel/intern/grease_pencil.cc
View File

@ -499,6 +499,56 @@ void GreasePencil::free_drawing_array()
this->drawing_array_size = 0;
}
static void save_tree_node_to_storage(blender::bke::gpencil::TreeNode &node,
GreasePencilLayerTreeNode *dst)
{
dst->type = node.type;
copy_v3_v3_uchar(dst->color, node.color);
dst->flag = node.flag;
if (node.name) {
dst->name = BLI_strdup(node.name);
}
}
static void save_layer_to_storage(blender::bke::gpencil::Layer &node,
GreasePencilLayerTreeNode **dst)
{
using namespace blender;
GreasePencilLayerTreeLeaf *new_leaf = MEM_cnew<GreasePencilLayerTreeLeaf>(__func__);
/* Save properties. */
save_tree_node_to_storage(node, &new_leaf->base);
/* Save frames map. */
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__);
MutableSpan<int> keys{new_leaf->layer.frames_storage.keys, frames_size};
MutableSpan<int> 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]);
}
/* Store pointer. */
*dst = reinterpret_cast<GreasePencilLayerTreeNode *>(new_leaf);
}
static void save_layer_group_to_storage(blender::bke::gpencil::LayerGroup &node,
GreasePencilLayerTreeNode **dst)
{
GreasePencilLayerTreeGroup *new_group = MEM_cnew<GreasePencilLayerTreeGroup>(__func__);
/* Save properties. */
save_tree_node_to_storage(node, &new_group->base);
/* Save number of children. */
new_group->children_num = node.num_children();
/* Store pointer. */
*dst = reinterpret_cast<GreasePencilLayerTreeNode *>(new_group);
}
void GreasePencil::save_layer_tree_to_storage()
{
using namespace blender::bke::gpencil;
@ -509,16 +559,16 @@ void GreasePencil::save_layer_tree_to_storage()
__func__);
int i = 0;
this->runtime->root_group().save_to_storage(&this->layer_tree_storage.nodes[i++]);
save_layer_group_to_storage(this->runtime->root_group(), &this->layer_tree_storage.nodes[i++]);
for (TreeNode &node : this->runtime->root_group().children_in_pre_order()) {
GreasePencilLayerTreeNode **dst = &this->layer_tree_storage.nodes[i];
if (node.is_group()) {
LayerGroup &group = node.as_group();
group.save_to_storage(dst);
save_layer_group_to_storage(group, dst);
}
else if (node.is_layer()) {
Layer &layer = node.as_layer();
layer.save_to_storage(dst);
save_layer_to_storage(layer, dst);
}
i++;
}
@ -534,7 +584,10 @@ static void read_layer_node_recursive(blender::bke::gpencil::LayerGroup &current
case GREASE_PENCIL_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
Layer new_layer(node_leaf->base.name);
new_layer.load_from_storage(node_leaf->layer);
for (int i = 0; i < node_leaf->layer.frames_storage.size; i++) {
new_layer.frames_for_write().add(node_leaf->layer.frames_storage.keys[i],
node_leaf->layer.frames_storage.values[i]);
}
current_group.add_layer(std::move(new_layer));
break;
}