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
4 changed files with 80 additions and 83 deletions
Showing only changes of commit 41fa00848f - Show all commits

View File

@ -196,6 +196,30 @@ 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

Put out of line.

Put out of line.
* 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:
*
* {0: 0, 5: 1, 10: -1, 12: 2, 16: -1}
*

Put out of line.

Put out of line.
* In this example there are three drawings (drawing #0, drawing #1 and drawing #2). The first
* drawing starts at frame 0 and ends at frame 5 (excusive). The second drawing starts at

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.
* frame 5 and ends at frame 10. Finally, the third drawing starts at frame 12 and ends at
* frame 16.
*
* | | | | | | | | | | |1|1|1|1|1|1|1|
* Time: |0|1|2|3|4|5|6|7|8|9|0|1|2|3|4|5|6|...
* Frame: [#0 ][#1 ] [#2 ]
*
* \note If a drawing references another data-block, all of the drawings in that data-block are
* mapped sequentially to the frames (frame-by-frame). If another frame starts, the rest of the
* 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.
*/

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

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.

I think emplace_layer would be a better name indeed.

I think `emplace_layer` would be a better name indeed.
Map<int, int> frames_;
public:
@ -224,6 +248,16 @@ class Layer : public TreeNode, ::GreasePencilLayer {
{
return this != &other;
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.
}
bool insert_frame(int frame_number, int index)
{
return frames_.add(frame_number, index);
}
bool overwrite_frame(int frame_number, int index)
{
return frames_.add_overwrite(frame_number, index);
}
};
class LayerGroup : public TreeNode {
@ -264,14 +298,16 @@ class LayerGroup : public TreeNode {
children_.append(std::make_unique<LayerGroup>(group));
}
void add_layer(Layer &layer)
Layer &add_layer(Layer &layer)
{
children_.append(std::make_unique<Layer>(layer));
int64_t index = children_.append_and_get_index(std::make_unique<Layer>(layer));

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

Looks like this should be private maybe? It has a `_` suffix.
return children_[index].get()->as_layer();
}
void add_layer(Layer &&layer)
Layer &add_layer(Layer &&layer)
{
children_.append(std::make_unique<Layer>(layer));
int64_t index = children_.append_and_get_index(std::make_unique<Layer>(layer));
return children_[index].get()->as_layer();

Put out of line.

Put out of line.
}
int num_children()
@ -286,24 +322,30 @@ class LayerGroup : public TreeNode {
}
};
filedescriptor marked this conversation as resolved Outdated

Same remark as above about commented out code.

Same remark as above about commented out code.
class LayerTree {
private:
LayerGroup root_;
};
namespace convert {

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.
CurvesGeometry legacy_gpencil_frame_to_curves_geometry(bGPDframe &gpf);
void legacy_gpencil_frame_to_curves_geometry(GreasePencilDrawing &drawing, bGPDframe &gpf);
void legacy_gpencil_to_grease_pencil(bGPdata &gpd, GreasePencil &grease_pencil);
void legacy_gpencil_to_grease_pencil(GreasePencil &grease_pencil, bGPdata &gpd);
} // namespace convert

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
} // namespace gpencil
using namespace blender::bke::gpencil;
class GreasePencilRuntime {
private:
gpencil::LayerTree layer_tree_;
LayerGroup root_group_;
public:
GreasePencilRuntime()
{
}
LayerGroup &root_group()
{
return root_group_;

Put out of line.

Put out of line.
}
};
} // namespace blender::bke

View File

@ -16,7 +16,7 @@
namespace blender::bke::gpencil::convert {
CurvesGeometry legacy_gpencil_frame_to_curves_geometry(bGPDframe &gpf)
void legacy_gpencil_frame_to_curves_geometry(GreasePencilDrawing &drawing, bGPDframe &gpf)
filedescriptor marked this conversation as resolved Outdated

legacy_gpencil_frame_to_grease_pencil_drawing

`legacy_gpencil_frame_to_grease_pencil_drawing`
{
/* Get the number of points, number of strokes and the offsets for each stroke. */
Vector<int> offsets;

Order return argument last, make the gpf argument const

Order return argument last, make the `gpf` argument const
@ -88,11 +88,13 @@ CurvesGeometry legacy_gpencil_frame_to_curves_geometry(bGPDframe &gpf)
curves.tag_topology_changed();
return curves;
drawing.geometry.wrap() = std::move(curves);
}
void legacy_gpencil_to_grease_pencil(bGPdata &gpd, GreasePencil &grease_pencil)
void legacy_gpencil_to_grease_pencil(GreasePencil &grease_pencil, bGPdata &gpd)
{
using namespace blender::bke::gpencil;
int num_layers = 0;
int num_drawings = 0;
LISTBASE_FOREACH (bGPDlayer *, gpl, &gpd.layers) {
@ -102,21 +104,22 @@ void legacy_gpencil_to_grease_pencil(bGPdata &gpd, GreasePencil &grease_pencil)
grease_pencil.drawing_array_size = num_drawings;
grease_pencil.drawing_array = reinterpret_cast<GreasePencilDrawingOrReference **>(
MEM_cnew_array<GreasePencilDrawing>(num_drawings, __func__));
MEM_cnew_array<GreasePencilDrawing *>(num_drawings, __func__));
int i = 0;
LISTBASE_FOREACH (bGPDlayer *, gpl, &gpd.layers) {
/* TODO: create a new layer here. */
Layer &new_layer = grease_pencil.runtime->root_group().add_layer(Layer(gpl->info));
LISTBASE_FOREACH (bGPDframe *, gpf, &gpl->frames) {
grease_pencil.drawing_array[i] = reinterpret_cast<GreasePencilDrawingOrReference *>(
MEM_new<GreasePencilDrawing>(__func__));
GreasePencilDrawing &drawing = *reinterpret_cast<GreasePencilDrawing *>(
grease_pencil.drawing_array[i]);
drawing.base.type = GREASE_PENCIL_DRAWING;
/* TODO: copy flag. */
/* Convert the frame to a drawing. */
drawing.geometry = legacy_gpencil_frame_to_curves_geometry(*gpf);
/* TODO: add drawing to layer. Using the gpf->framenum. */
legacy_gpencil_frame_to_curves_geometry(drawing, *gpf);
new_layer.insert_frame(gpf->framenum, i);
i++;
}
}

View File

@ -2767,8 +2767,8 @@ static const EnumPropertyItem convert_target_items[] = {
{OB_GPENCIL_LEGACY,
"GPENCIL",
ICON_OUTLINER_OB_GREASEPENCIL,
"Grease Pencil",
"Grease Pencil from Curve or Mesh objects"},
"Grease Pencil (legacy)",
"Grease Pencil (legacy) from Curve or Mesh objects"},
#ifdef WITH_POINT_CLOUD
{OB_POINTCLOUD,
"POINTCLOUD",
@ -2777,6 +2777,11 @@ static const EnumPropertyItem convert_target_items[] = {
"Point Cloud from Mesh objects"},
#endif
{OB_CURVES, "CURVES", ICON_OUTLINER_OB_CURVES, "Curves", "Curves from evaluated curve data"},
{OB_GREASE_PENCIL,
"GREASEPENCIL",
ICON_OUTLINER_OB_GREASEPENCIL,
"Grease Pencil",
"Grease Pencil from Grease Pencil (legacy)"},
{0, nullptr, 0, nullptr, nullptr},
};
@ -3063,37 +3068,10 @@ static int object_convert_exec(bContext *C, wmOperator *op)
}
ob_gpencil->actcol = actcol;
}
else if (ob->type == OB_GPENCIL_LEGACY && target == OB_CURVES) {
ob->flag |= OB_DONE;
bGPdata *gpd = static_cast<bGPdata *>(ob->data);
bGPDlayer *gpl = BKE_gpencil_layer_active_get(gpd);
bGPDframe &gpf = *BKE_gpencil_layer_frame_find(gpl, scene->r.cfra);
if (keep_original) {
BLI_assert_unreachable();
}
else {
newob = ob;
}
Curves *new_curves = static_cast<Curves *>(BKE_id_new(bmain, ID_CV, newob->id.name + 2));
newob->data = new_curves;
newob->type = OB_CURVES;
new_curves->geometry.wrap() = std::move(
bke::gpencil::convert::legacy_gpencil_frame_to_curves_geometry(gpf));
BKE_object_free_derived_caches(newob);
BKE_object_free_modifiers(newob, 0);
}
else if (ob->type == OB_GPENCIL_LEGACY && target == OB_GREASE_PENCIL) {
ob->flag |= OB_DONE;
bGPdata *gpd = static_cast<bGPdata *>(ob->data);
bGPDlayer *gpl = BKE_gpencil_layer_active_get(gpd);
bGPDframe &gpf = *BKE_gpencil_layer_frame_find(gpl, scene->r.cfra);
if (keep_original) {
BLI_assert_unreachable();
@ -3102,13 +3080,12 @@ static int object_convert_exec(bContext *C, wmOperator *op)
newob = ob;
}
Curves *new_curves = static_cast<Curves *>(BKE_id_new(bmain, ID_CV, newob->id.name + 2));
GreasePencil *new_grease_pencil = static_cast<GreasePencil *>(
BKE_id_new(bmain, ID_GP, newob->id.name + 2));
newob->data = new_grease_pencil;
newob->type = OB_GREASE_PENCIL;
newob->data = new_curves;
newob->type = OB_CURVES;
new_curves->geometry.wrap() = std::move(
bke::gpencil::convert::legacy_gpencil_frame_to_curves_geometry(gpf));
bke::gpencil::convert::legacy_gpencil_to_grease_pencil(*new_grease_pencil, *gpd);
BKE_object_free_derived_caches(newob);
BKE_object_free_modifiers(newob, 0);

View File

@ -68,28 +68,8 @@ typedef struct GreasePencilDrawingReference {
} GreasePencilDrawingReference;
filedescriptor marked this conversation as resolved Outdated

Move below runtime data pointer.

Move below runtime data pointer.
/**
* 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.
*
* \example:
*
* {0: 0, 5: 1, 10: -1, 12: 2, 16: -1}
*
* In this example there are three drawings (drawing #0, drawing #1 and drawing #2). The first
* drawing starts at frame 0 and ends at frame 5 (excusive). The second drawing starts at
* frame 5 and ends at frame 10. Finally, the third drawing starts at frame 12 and ends at
* frame 16.
*
* | | | | | | | | | | |1|1|1|1|1|1|1|
* Time: |0|1|2|3|4|5|6|7|8|9|0|1|2|3|4|5|6|...
* Frame: [#0 ][#1 ] [#2 ]
*
* \note If a drawing references another data-block, all of the drawings in that data-block are
* mapped sequentially to the frames (frame-by-frame). If another frame starts, the rest of the
* 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.
* Storage for the Map in `blender::bke::gpencil::Layer`.
* See the description there for more detail.
*/
typedef struct GreasePencilLayerFramesMapStorage {
/* Array of `frames` keys (sorted in ascending order). */
@ -173,12 +153,7 @@ typedef struct GreasePencil {
int drawing_array_size;
char _pad[4];
#ifdef __cplusplus
/**
* The layer tree.
*/
// const bke::gpencil::LayerTree &layer_tree() const;
#endif
/* Only used for storage in the .blend file. */
GreasePencilLayerTreeStorage layer_tree_storage;
/**