blender
/
blender
133
398
Fork 579
Code Issues 6.3k Pull Requests 749 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 +25 -6
2 changed files with 25 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 95eb26ff59 - Show all commits

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

@ -31,6 +31,7 @@ class Layer;
*/
class TreeNode : public ::GreasePencilLayerTreeNode, NonMovable {
friend class LayerGroup;
public:
explicit TreeNode(GreasePencilLayerTreeNodeType type);
explicit TreeNode(GreasePencilLayerTreeNodeType type, StringRefNull name);
@ -206,8 +207,17 @@ class LayerGroup : public TreeNode {
LayerGroup(const LayerGroup &other);
Falk David commented 2023-04-12 10:40:11 +02:00
Outdated
Review
Copy Link

Put out of line.

Put out of line.
private:
/**
Bastien Montagne commented 2023-05-06 17:19:44 +02:00
Outdated
Review
Copy Link

Not sure what is a 'pre-order vector'? or is a typo? Like pre-ordered vector? Same below.

Not sure what is a 'pre-order vector'? or is a typo? Like `pre-ordered vector`? Same below.
* CacheMutex for `children_cache_` and `layer_cache_`;
*/
mutable CacheMutex children_cache_mutex_;
/**
* Caches all the children of this group in a single pre-order vector.
*/
mutable Vector<TreeNode *> children_cache_;
/**
* Caches all the layers in this group in a single pre-order vector.
*/
mutable Vector<Layer *> layer_cache_;
Hans Goudey commented 2023-04-25 05:02:57 +02:00
Outdated
Review
Copy Link

Returning an element added to a vector by reference is quite dangerous since they're invalidated by further additions, we usually don't do that. Either returning an index or not having these wrapper functions seems like the best choices IMO

Returning an element added to a vector by reference is quite dangerous since they're invalidated by further additions, we usually don't do that. Either returning an index or not having these wrapper functions seems like the best choices IMO
Sergey Sharybin commented 2023-04-25 09:37:10 +02:00
Outdated
Review
Copy Link

From quick look it seems that it should actually be emplace_layer() or something like this.
And for that it is typically very handy to return reference. And not only that, since C++17 it is expected behavior for vector type containers as well.

From quick look it seems that it should actually be `emplace_layer()` or something like this. And for that it is typically very handy to return reference. And not only that, since C++17 it is expected behavior for vector type containers as well.
Falk David commented 2023-04-25 17:01:57 +02:00
Outdated
Review
Copy Link

I think emplace_layer would be a better name indeed.

I think `emplace_layer` would be a better name indeed.
public:
@ -239,17 +249,20 @@ class LayerGroup : public TreeNode {
void remove_child(int64_t index);
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.
/**
* Returns a `Vector` of pointers to all the `TreeNode`s in this group.
* Returns a `Span` of pointers to all the `TreeNode`s in this group.
*/
Span<const TreeNode *> children() const;
Span<TreeNode *> children_for_write();
/**
* Returns a `Vector` of pointers to all the `Layers`s in this group.
* Returns a `Span` of pointers to all the `Layers`s in this group.
*/
Span<const Layer *> layers() const;
Span<Layer *> layers_for_write();
/**
* Print the children. For debugging purposes.
*/
void print_children(StringRefNull header) const;
private:
@ -323,7 +336,7 @@ class GreasePencilRuntime {
bool has_active_layer() const;
const greasepencil::Layer &active_layer() const;
greasepencil::Layer &active_layer_for_write() const;
greasepencil::Layer &active_layer_for_write();
void set_active_layer_index(int index);
int active_layer_index() const;
};

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

@ -644,7 +644,13 @@ bool GreasePencilRuntime::has_active_layer() const
const greasepencil::Layer &GreasePencilRuntime::active_layer() const
{
BLI_assert(this->active_layer_index_ >= 0);
return *this->root_group().layers()[this->active_layer_index_];
return this->root_group().children()[this->active_layer_index_]->as_layer();
}
greasepencil::Layer &GreasePencilRuntime::active_layer_for_write()
{
BLI_assert(this->active_layer_index_ >= 0);
return this->root_group_for_write().children_for_write()[this->active_layer_index_]->as_layer_for_write();
}
void GreasePencilRuntime::set_active_layer_index(int index)
@ -710,7 +716,7 @@ BoundBox *BKE_grease_pencil_boundbox_get(Object *ob)
break;
}
case GP_DRAWING_REFERENCE: {
/* TODO */
/* TODO: Calculate the bounding box of the reference drawing. */
break;
}
}
@ -731,7 +737,7 @@ void BKE_grease_pencil_data_update(struct Depsgraph * /*depsgraph*/,
GreasePencil *grease_pencil = static_cast<GreasePencil *>(object->data);
/* Evaluate modifiers. */
/* TODO: modifiers. */
/* TODO: evaluate modifiers. */
/* Assign evaluated object. */
/* TODO: Get eval from modifiers geometry set. */