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
13 changed files with 399 additions and 1 deletions
Showing only changes of commit 4f1d550ee8 - Show all commits

View File

@ -0,0 +1,6 @@
#include "BKE_grease_pencil.hh"
Span<GreasePencilDrawing> blender::bke::GreasePencil::drawings() const
{
return Span<GreasePencilDrawing>{this->drawing_array, this->drawing_array_size};
}

View File

@ -0,0 +1,57 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
filedescriptor marked this conversation as resolved Outdated

Needs copyright.

Needs copyright.
#pragma once
/** \file
* \ingroup bke
* \brief Low-level operations for grease pencil.
*/
#include "BLI_map.hh"
#include "DNA_grease_pencil_types.h"
namespace blender::bke {
class GreasePencilLayerRuntime {
private:
Map<int, int> frames;
};
// class GreasePencilLayer : ::GreasePencilLayer {
// public:
// GreasePencilLayer();
// GreasePencilLayer(const GreasePencilLayer &other);
filedescriptor marked this conversation as resolved Outdated

Decide on the namespace here. Should it maybe be just gp?

Decide on the namespace here. Should it maybe be just `gp`?
// GreasePencilLayer(GreasePencilLayer &&other);
// GreasePencilLayer &operator=(const GreasePencilLayer &other);
// GreasePencilLayer &operator=(GreasePencilLayer &&other);
// ~GreasePencilLayer();
// /* --------------------------------------------------------------------
// * Accessors.
// */
// int frames_num() const;
// };
class GreasePencil : ::GreasePencil {
public:
GreasePencil() = delete;
GreasePencil(const GreasePencil &other);

Is there a particular reason you remove copy assignment but keep copy construction defined?

Is there a particular reason you remove copy assignment but keep copy construction defined?

Just because copying should be explicit. And removing the copy assignment constructor avoids errors.

Just because copying should be explicit. And removing the copy assignment constructor avoids errors.
GreasePencil(GreasePencil &&other);
GreasePencil &operator=(const GreasePencil &other);
GreasePencil &operator=(GreasePencil &&other);
~GreasePencil();
/* --------------------------------------------------------------------
* Accessors.
*/
Span<GreasePencilDrawing> drawings() const;
};

Is defining these as constexpr helpful? I sort of doubt any real computation is done on these nodes at compile time. But maybe?

Is defining these as `constexpr` helpful? I sort of doubt any real computation is done on these nodes at compile time. But maybe?

The const in the const bool return type means nothing here

The `const` in the `const bool` return type means nothing here
} // namespace blender::bke
inline blender::Map<int, int> GreasePencilLayer::frames() const
{
return this->runtime->frames;
}

View File

@ -200,6 +200,7 @@ typedef struct Main {
ListBase pointclouds;
ListBase volumes;
ListBase simulations;
ListBase greasepencils;
filedescriptor marked this conversation as resolved Outdated

Maybe grease_pencils?

Maybe `grease_pencils`?
/**
* Must be generated, used and freed by same code - never assume this is valid data unless you

View File

@ -148,6 +148,7 @@ set(SRC
intern/gpencil_geom_legacy.cc
intern/gpencil_modifier_legacy.c
intern/gpencil_update_cache_legacy.c
intern/grease_pencil.cc
intern/icons.cc
intern/icons_rasterize.c
intern/idprop.c

View File

@ -0,0 +1,102 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
/** \file
* \ingroup bke
*/
#include "BKE_grease_pencil.hh"
#include "BKE_idtype.h"
#include "BKE_lib_query.h"
#include "BLO_read_write.h"
#include "BLT_translation.h"
#include "DNA_ID.h"
#include "DNA_ID_enums.h"
#include "DNA_grease_pencil_types.h"
#include "MEM_guardedalloc.h"
static void grease_pencil_init_data(ID *id)
{
}
static void grease_pencil_copy_data(Main * /*bmain*/, ID *id_dst, const ID *id_src, const int flag)
{
}
static void grease_pencil_free_data(ID *id)
{
GreasePencil *grease_pencil = (GreasePencil *)id;
BKE_animdata_free(&grease_pencil->id, false);
// TODO: free drawing array
// TODO: free layer tree
MEM_SAFE_FREE(grease_pencil->mat);
}
static void grease_pencil_foreach_id(ID *id, LibraryForeachIDData *data)
{
GreasePencil *grease_pencil = (GreasePencil *)id;
for (int i = 0; i < grease_pencil->mat_num; i++) {
BKE_LIB_FOREACHID_PROCESS_IDSUPER(data, grease_pencil->mat[i], IDWALK_CB_USER);
filedescriptor marked this conversation as resolved Outdated

Remove printfs.

Remove printfs.
}
// TODO: walk all the referenced drawings
}
static void grease_pencil_blend_write(BlendWriter *writer, ID *id, const void *id_address)
{
}
static void grease_pencil_blend_read_data(BlendDataReader *reader, ID *id)
{
}

C++ cast here (and above, I'll stop writing it now)

C++ cast here (and above, I'll stop writing it now)
static void grease_pencil_blend_read_lib(BlendLibReader *reader, ID *id)

Personally I find grease_pencil_dst a longer name than it needs to be, compared to something like gp_dst, where it's easier to see the logic when reading the function IMO. I understand that's subjective though

Personally I find `grease_pencil_dst` a longer name than it needs to be, compared to something like `gp_dst`, where it's easier to see the logic when reading the function IMO. I understand that's subjective though

This used to be the case in the old grease pencil code, but tbh I find it better when it's more explicit. We ended up with gpf,gpd,gps etc. and it's really unreadable imo.

This used to be the case in the old grease pencil code, but tbh I find it better when it's more explicit. We ended up with `gpf`,`gpd`,`gps` etc. and it's really unreadable imo.
{
}
static void grease_pencil_blend_read_expand(BlendExpander *expander, ID *id)
{
GreasePencil *grease_pencil = (GreasePencil *)id;
for (int i = 0; i < grease_pencil->mat_num; i++) {
BLO_expand(expander, grease_pencil->mat[i]);
}
}
IDTypeInfo IDType_ID_GP = {
/*id_code*/ ID_GP,
/*id_filter*/ FILTER_ID_GP,
/*main_listbase_index*/ INDEX_ID_GP,
/*struct_size*/ sizeof(GreasePencil),
/*name*/ "GreasePencil",
/*name_plural*/ "grease_pencils_new",
/*translation_context*/ BLT_I18NCONTEXT_ID_GPENCIL,
/*flags*/ IDTYPE_FLAGS_APPEND_IS_REUSABLE,
/*asset_type_info*/ nullptr,
/*init_data*/ grease_pencil_init_data,
/*copy_data*/ grease_pencil_copy_data,
/*free_data*/ grease_pencil_free_data,
/*make_local*/ nullptr,
/*foreach_id*/ grease_pencil_foreach_id,
/*foreach_cache*/ nullptr,
/*foreach_path*/ nullptr,
/*owner_pointer_get*/ nullptr,
/*blend_write*/ grease_pencil_blend_write,
/*blend_read_data*/ grease_pencil_blend_read_data,
/*blend_read_lib*/ grease_pencil_blend_read_lib,
/*blend_read_expand*/ grease_pencil_blend_read_expand,
/*blend_read_undo_preserve*/ nullptr,
/*lib_override_apply_post*/ nullptr,
};
Span<GreasePencilDrawing> blender::bke::GreasePencil::drawings() const
{
return Span<GreasePencilDrawing>{this->drawing_array, this->drawing_array_size};
}

View File

@ -714,6 +714,7 @@ int set_listbasepointers(Main *bmain, ListBase *lb[/*INDEX_ID_MAX*/])
lb[INDEX_ID_WM] = &(bmain->wm);
lb[INDEX_ID_MSK] = &(bmain->masks);
lb[INDEX_ID_SIM] = &(bmain->simulations);
lb[INDEX_ID_GP] = &(bmain->greasepencils);
lb[INDEX_ID_NULL] = NULL;

View File

@ -2985,6 +2985,8 @@ static const char *dataname(short id_code)
return "Data from VO";
case ID_SIM:
return "Data from SIM";
case ID_GP:
return "Data from GP";
}
return "Data from Lib Block";
}

View File

@ -608,6 +608,7 @@ void DepsgraphNodeBuilder::build_id(ID *id)
case ID_CV:
case ID_PT:
case ID_VO:
case ID_GP:
build_object_data_geometry_datablock(id);
break;
case ID_SPK:

View File

@ -549,6 +549,7 @@ void DepsgraphRelationBuilder::build_id(ID *id)
case ID_PT:
case ID_VO:
case ID_GD_LEGACY:
case ID_GP:
build_object_data_geometry_datablock(id);
break;
case ID_SPK:

View File

@ -2436,6 +2436,8 @@ int UI_icon_from_idcode(const int idcode)
case ID_SIM:
/* TODO: Use correct icon. */
return ICON_PHYSICS;
case ID_GP:
return ICON_OUTLINER_DATA_GREASEPENCIL;
/* No icons for these ID-types. */
case ID_LI:

View File

@ -1097,6 +1097,7 @@ typedef enum IDRecalcFlag {
#define FILTER_ID_SCR (1ULL << 37)
#define FILTER_ID_WM (1ULL << 38)
#define FILTER_ID_LI (1ULL << 39)
#define FILTER_ID_GP (1ULL << 40)
#define FILTER_ID_ALL \
(FILTER_ID_AC | FILTER_ID_AR | FILTER_ID_BR | FILTER_ID_CA | FILTER_ID_CU_LEGACY | \
@ -1105,7 +1106,7 @@ typedef enum IDRecalcFlag {
FILTER_ID_NT | FILTER_ID_OB | FILTER_ID_PA | FILTER_ID_PAL | FILTER_ID_PC | FILTER_ID_SCE | \
FILTER_ID_SPK | FILTER_ID_SO | FILTER_ID_TE | FILTER_ID_TXT | FILTER_ID_VF | FILTER_ID_WO | \
FILTER_ID_CF | FILTER_ID_WS | FILTER_ID_LP | FILTER_ID_CV | FILTER_ID_PT | FILTER_ID_VO | \
FILTER_ID_SIM | FILTER_ID_KE | FILTER_ID_SCR | FILTER_ID_WM | FILTER_ID_LI)
FILTER_ID_SIM | FILTER_ID_KE | FILTER_ID_SCR | FILTER_ID_WM | FILTER_ID_LI | FILTER_ID_GP)
/**
* This enum defines the index assigned to each type of IDs in the array returned by
@ -1196,6 +1197,7 @@ enum {
INDEX_ID_CA,
INDEX_ID_SPK,
INDEX_ID_LP,
INDEX_ID_GP,
/* Collection and object types. */
INDEX_ID_OB,

View File

@ -82,6 +82,7 @@ typedef enum ID_Type {
ID_PT = MAKE_ID2('P', 'T'), /* PointCloud */
ID_VO = MAKE_ID2('V', 'O'), /* Volume */
ID_SIM = MAKE_ID2('S', 'I'), /* Simulation (geometry node groups) */
ID_GP = MAKE_ID2('G', 'P'), /* Grease Pencil */
} ID_Type;
/* Only used as 'placeholder' in .blend files for directly linked data-blocks. */

View File

@ -0,0 +1,221 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
/** \file
* \ingroup DNA
*/
#pragma once
#include "DNA_ID.h"
#include "DNA_curves_types.h"
#ifdef __cplusplus
# include "BLI_map.hh"
namespace blender::bke {
class GreasePencilLayerRuntime;
} // namespace blender::bke
using GreasePencilLayerRuntimeHandle = blender::bke::GreasePencilLayerRuntime;
#else
typedef struct GreasePencilLayerRuntimeHandle GreasePencilLayerRuntimeHandle;
#endif
#endif
#ifdef __cplusplus
extern "C" {
#endif
struct GreasePencil;
typedef enum GreasePencilDrawingType {
GREASE_PENCIL_DRAWING = 0,
GREASE_PENCIL_DRAWING_REFERENCE = 1,
} GreasePencilLayerType;
typedef struct GreasePencilDrawingOrReference {
char type;
/**
* Flag. Used to set e.g. the selection status.
*/
int flag;
} GreasePencilDrawingOrReference;
/**
* A grease pencil drawing is a set of strokes. The data is stored using
* the CurvesGeometry data structure and the custom attributes within it.
* It can either own the data or reference it from another GreasePencil
* data-block.
*
* Note: When a drawing references another data-block, it will always
* reference all the drawings in that data-block sequentially.
*/
typedef struct GreasePencilDrawing {
GreasePencilDrawingOrReference base;
/**
* The stroke data for this drawing. Is nullptr in case the drawing
* references its data from another data-block.
*/
CurvesGeometry geometry;
} GreasePencilDrawing;
typedef struct GreasePencilDrawingReference {
GreasePencilDrawingOrReference base;
/**
* A reference to another GreasePencil data-block. Is nullptr in
* case the drawing owns its data.
*
* If the data-block has multiple drawings, this drawing references
* all of them sequentially.
*/
struct GreasePencil *id_reference;
filedescriptor marked this conversation as resolved Outdated

Move below runtime data pointer.

Move below runtime data pointer.
} GreasePencilDrawingReference;
/**
* Properties for layers and layer groups.
*/
typedef struct GreasePencilLayerProperties {
/**
* Name of the layer/group. Dynamic length.
*/
char *name;
/**
* Flag. Used to set e.g. the selection, visibility, ... status.
*/
int flag;

I wonder if this user count could be runtime data? Or if not, maybe it's worth mentioning why it isn't in a comment.

I wonder if this user count could be runtime data? Or if not, maybe it's worth mentioning why it isn't in a comment.

I thought about this, but I don't think it can be runtime data. Since there will be keyframe instances, we need to make the user count is saved.

I thought about this, but I don't think it can be runtime data. Since there will be keyframe instances, we need to make the user count is saved.

The connection between keyframe instances and saving the user count isn't super clear to me. Maybe a comment about what the user count is used for here would help?

The connection between keyframe instances and saving the user count isn't super clear to me. Maybe a comment about what the user count is used for here would help?

Maybe it's still better to have this as runtime data. If there is ever a bug where this doesn't get counted correctly, at least a file read could fix it. It should be possible to read this as zero, and increment it for every user?

This is easy to change afterwards though.

Maybe it's still better to have this as runtime data. If there is ever a bug where this doesn't get counted correctly, at least a file read could fix it. It should be possible to read this as zero, and increment it for every user? This is easy to change afterwards though.

I see, re-creating the user count when reading could work indeed.

I see, re-creating the user count when reading could work indeed.
/**
* Color tag.
*/
uchar color[3];
} GreasePencilLayerProperties;
typedef enum GreasePencilLayerTreeElemType {
GREASE_PENCIL_LAYER_TREE_LEAF = 0,
GREASE_PENCIL_LAYER_TREE_GROUP = 1,
} GreasePencilLayerType;
typedef struct GreasePencilLayerTreeElem {
/**
*
*/
char type;
struct GreasePencilLayerGroup *parent;
} GreasePencilLayerTreeElem;
/**
* A grease pencil layer is a collection of drawings. It maps them
* to specific scene times on the timeline.
*
* A layer can be a group if it has a non-negative `children_num`.
* Layer groups do not have a frames map.
*
*/

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?
typedef struct GreasePencilLayer {

Looks like stroke_buffer() can be const

Looks like `stroke_buffer()` can be `const`
/**
* Properties of this layer or group.
*/
GreasePencilLayerProperties properties;
/**
* 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.
* If another frame starts, the rest of the mapped drawings
* are discarded.
*/
#ifdef __cplusplus
const blender::Map<int, int> &frames() const;
#endif
/* Only used for storage in the .blend file. */
struct {
/* Array of `frames` keys. */
int *keys;
int keys_num;
/* Array of `frames` values. */
int *values;
int values_num;
} frames_storage;
/**
* Runtime struct pointer.
*/
GreasePencilLayerRuntimeHandle *runtime;
} GreasePencilLayer;
typedef struct GreasePencilLayerGroup {
GreasePencilLayerTreeElem base;
/**
* Pointer to the parent layer group and to zero or more children elements.
*/
struct GreasePencilLayerTreeElem **children;
int children_num;
} GreasePencilLayerGroup;
typedef struct GreasePencilLayerLeaf {
GreasePencilLayerTreeElem base;
GreasePencilLayer layer;
} GreasePencilLayerLeaf;
/**
* The grease pencil data-block.
* It holds a set of grease pencil drawings, a tree of layer groups, and a set of layers whithin

Pretty sure null-terminated goes without saying for char * pointers in DNA, I don't think it's worth mentioning here

Pretty sure null-terminated goes without saying for `char *` pointers in DNA, I don't think it's worth mentioning here

Not sure about dynamic names for strings. It is not something typically used in the DNA.

Not sure about dynamic names for strings. It is not something typically used in the DNA.

It's been done more recently I think. It's properly integrated with RNA now too, to avoid that boilerplate. It's nice not to have to worry about choosing a future-proof length.

It's been done more recently I think. It's properly integrated with RNA now too, to avoid that boilerplate. It's nice not to have to worry about choosing a future-proof length.
* each layer group.
*/
typedef struct GreasePencil {
ID id;
/** Animation data (must be immediately after #id). */
struct AnimData *adt;
/**
* An array of GreasePencilDrawing's.
*/
GreasePencilDrawingOrReference *drawing_array;
int drawing_array_size;
/**
* The root layer group (is not shown in the UI).
* Its parent is always nullptr.
*/
GreasePencilLayerGroup *root_group;
/**
* An array of materials.
*/
struct Material **material_array;
int material_array_size;
/**
* Global flag on the data-block.
*/
int flag;
#ifdef __cplusplus
Span<GreasePencilDrawingOrReference> drawings() const;
#endif
} GreasePencil;
#ifdef __cplusplus
}
#endif