# Grease Pencil Core Rewrite - Developer Discussion

## Introduction

When grease pencil was [first introduced](https://web.archive.org/web/20100706205911/http://www.blender.org/development/release-logs/blender-248/grease-pencil/) to Blender back in 2008(?) by Joshua Leung it was designed around drawing annotations into the viewport. Over a decade later, the community has built a feature-rich 2D animation tool out of the initial draft of grease pencil. Today, there are some challenges and limitations that we face with how grease pencil is structured. I'd like to summarize the issues and propose a way forward.

## Problems
*TODO*

- Linked lists
- Duplicated code
- Lots of nested FOREACH loops that can be simplified with macros, callbacks or any other method.
- Eval object duplication, undo
- Grouping strokes/support for holes in fill shapes
- Not compatible with Geometry Nodes
- Code needs a general review and cleanup of obsolete functions/variables

## New design requirements

- We need a structure that can hold a set of curves (strokes) per frame in the scene
- We need layers of strokes that can be used to draw strokes in a specific order
- Strokes can be grouped -> support holes in triangulation
- Layers can be grouped
- Adding/duplicating/removing/moving strokes from the data structure should be fast. Same applies to frames but it doesn't need to be as quick as for strokes and for layers it's even less important (e.g. it's ok if that takes 20 milliseconds).
- Unify looping code. Try to reduce nested FOREACH code.

## Proposal

The core idea is to replace `struct bGPDstroke` with `struct CurvesGeometry` and move to using C++ classes. This would mean that the current data stored on each point (position, pressure, strength, uv data, vertex color, time and flags such as selection) would become `CustomData` attributes. The same would apply to the strokes themselves, so thickness, hardness, material index, uv data, fill color, fill opacity, and selection would also be stored as attributes. The triangle data as well as the bounding box should be part of the runtime data and populated on load (although triangluation can be expensive so there might be some optimizations that could be done here). By default, every stroke would be treated as a polyline curve (`CURVE_TYPE_POLY`).

The structure of `bGPdata` would be kept so there would still be a set of layers, which contain a set of frames, which contain the strokes. But we would replace all the actual linked-lists data structure with arrays and replace `bGPDstroke` with `CurvesGeometry`.

Now `CurvesGeometry` could already contain multiple "strokes", so it might make sense to only put a single `CurvesGeometry` into a frame. I would suggest to keep it as an array though, so we can support groups of strokes and thus allow for holes and more.

It would be good to use C++ functionalities as much as possible instead of custom made solutions, and define wrapper classes for Layers, Frames and Strokes and Points.

## Draw Engine

The conversion must include the impact in the actual Draw Engine and the implications for the new Eevee rewrite. Maybe this topic need a separated document.

## Points to consider in new design

There are some points that must be considered in the new design because it can be important to be supported in the future.

- Layer Groups
- Stroke Groups
- Active Stroke (almost implemented already)
- Active Vertex
- What we do with GPencil modifers, move to mesh modifiers or do a custom GN modifier for grease pencil?

## Implementation details

### Initial approach:

For layers I imagine something like

typedef struct bGPDlayer {
    bGPDframe *frames;
    int totframes;
    ...
} bGPDlayer;

for the DNA struct. And then a wrapper c++ class:

namespace blender::bke::gpencil {

class GPLayer : public ::bGPDlayer {
   ...
   MutableSpan<GPFrame> frames();
   ...
}

### Idea from Hans:

Instead of having a lot of small arrays in `CurvesGeometry` you would put the entire frame in a single `CurvesGeometry`. Additionally, there would not be a structure for layers but an index attribute on each stroke.

So it would look something like:

struct bGPDlayer {
    string name;
    ...
};

struct bGPdata {
    ...
    Array<CurvesGeometry> frames;
    CustomData frame_data; // for selection, frame_number, keytype, etc.
    int frame_active_index;
    
    Array<bGPDlayer> layers;
    int layer_active_index;
}

### Points to consider in the all Strokes by Frame in one array element

Some points that must be reviewed to find a solution in the new data structure design.

#### Operators by Layers
Actually, there are lots of operators that are applied to only active layer.

The logic now is:

LISTBASE_FOREACH (bGPDlayer *, gpl, &gpd->layers) {
    LISTBASE_FOREACH (bGPDframe *, gpf, &gpl->frames) {
        LISTBASE_FOREACH (bGPDstroke *, gps, &gpf->strokes) {

or

LISTBASE_FOREACH (bGPDlayer *, gpl, &gpd->layers) {
        bGPDframe *gpf = gpl->actframe;
        LISTBASE_FOREACH (bGPDstroke *, gps, &gpf->strokes) {

With the actual structure is very easy to select only the frames and strokes of one layer, but with the new structure we would need to read all frames and all strokes to determine what strokes are affected.

The second case can be solved with the index using a code like this:

for (GPStroke gps : gpd->frames[frame_active_index].strokes()) {
    GPLayer gpl = gps->layer();

One possible improvement would be to have an offset of the start element (stroke) for each layer in the frame. If we have 20 layers, instead to read all strokes of the frame and filter, we could "jump" directly to the first stroke of the layer and stop reading when the layer index is different (it assumes strokes are sorted by layer).

#### Dopesheet

The Dopesheet visualize the keys by layer, so the logic is read one layer and for each layer read the frames.

This is done with something like:

LISTBASE_FOREACH (bGPDlayer *, gpl, &gpd->layers) {
    LISTBASE_FOREACH (bGPDframe *, gpf, &gpl->frames) {

With the new approach this is not possible because we need read all data to determine in what frames the layer has keys (strokes). If the strokes in the frame array are sorted, we could stop reading when the stroke layer index is greater than actual index, but if the strokes are not sorted, then we need read all.

If we cannot do that, maybe the dopesheet can slow all UI of Blender.

#### Draw Engine

The draw engine needs to draw the layer strokes sorted, so the loop read layer, and for each layer all strokes.

If you hide a layer in the UI, it's very easy for the draw engine skip all data...

LISTBASE_FOREACH (bGPDlayer *, gpl, &gpd->layers) {
	if (gpl->flag & HIDE) {
		continue;

If we cannot do this, the impact in the draw engine can be noticeable because all times needs to draw all data.

#### Empty Frames

A layer can have frames without strokes. This is very common.

#### Sort Frames

In some functionalities is critical to keep the array of frames sorted because some operations use a relative number of frames before or after current. Now this is easy because we have all frames of the layer linked to layer.

Other problem is when you move keyframes in the dopesheet. For example, you have 3 layers with strokes in frame 24. Now you move the layer 2 keyframe to frame 20...now you need remove the strokes of the layer 2 from frame array element and move the strokes to other frame array element.

#### Sort of Layers

We need keep the layers sorted from top to bottom for some operations. Also, if we move the layer position in the stack, the frame array will be unsorted because the array of strokes in theory is sorted with the previous layer sort.

#### Annotations

Annotations are using internally GPencil data structures, but there are differences in some variables and how they are used. The logic would be move to new structure but we need to be careful and don't overcomplicated the design because we need support annotations.

## Plan of action

I think there are basically two main approaches that we could take.

1. Work on a branch that is kept up-to-date with master. At some point merge all the changes at once.
2. Start working on master immediately doing incremental changes.

There are pros and cons for both approaches but I think I personally would prefer 2. Not a big fan of a massive merge commit.

For 2. we could start by writing the new DNA structs, create conversion functions to convert between the old DNA and new ones, and then use these convert functions when reading and writing to and from the `.blend` file. So this would be in `greasepencil_blend_write` and `greasepencil_blend_read_data`. We would keep using the old DNA when the file is read, but we would store the new DNA structs.
Another option would be to only convert to the new structs at runtime and adapt only parts of the code.

The downside of doing these conversions would be a decrease in performance (e.g. files might load longer, global undo would be slower). I would expect a conversion to take as long as a full copy of the data-block. We might be able to do these conversions in places, where a small delay would not be as noticeable.