blender
/
blender
135
420
Fork 603
Code Issues 6.3k Pull Requests 753 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 +17 -20
1 changed files with 17 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 8a9ce0fca3 - Show all commits

37
source/blender/makesdna/DNA_grease_pencil_types.h
View File

@ -62,8 +62,8 @@ typedef struct GreasePencilDrawingReference {
GreasePencilDrawingOrReference base; GreasePencilDrawingOrReference base;
/** /**
* A reference to another GreasePencil data-block. * A reference to another GreasePencil data-block.
* If the data-block has multiple drawings, this drawing references * If the data-block has multiple drawings, this drawing references all of them sequentially.
* all of them sequentially. * See the note in `GreasePencilLayer->frames()` for a detailed expelantion of this.
*/ */
struct GreasePencil *id_reference; struct GreasePencil *id_reference;
} GreasePencilDrawingReference; } GreasePencilDrawingReference;
filedescriptor marked this conversation as resolved Outdated
Falk David commented 2023-04-12 11:05:32 +02:00
Outdated
Review
Copy Link

Move below runtime data pointer.

Move below runtime data pointer.
@ -72,34 +72,31 @@ typedef struct GreasePencilDrawingReference {
* A grease pencil layer is a collection of drawings mapped to a specific time on the timeline. * A grease pencil layer is a collection of drawings mapped to a specific time on the timeline.
*/ */
typedef struct GreasePencilLayer { typedef struct GreasePencilLayer {
#ifdef __cplusplus
/** /**
* This Map maps a scene frame number (key) to an index into * This Map maps a scene frame number (key) to an index into GreasePencil->drawings (value). The
* GreasePencil->drawings (value). The frame number indicates * frame number indicates the first frame the drawing is shown. The end time is implicitly
* 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
* defined by the next greater frame number (key) in the map. * -1, no drawing is shown at this frame.
* If the value mapped to (index) is -1, no drawing is shown
* at this frame.
* *
* Example: * \example:
* *
* {0: 0, 5: 1, 10: -1, 12: 2, 16: -1} * {0: 0, 5: 1, 10: -1, 12: 2, 16: -1}
* *
Hans Goudey commented 2023-04-25 04:49:05 +02:00
Outdated
Review
Copy Link

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.
Falk David commented 2023-04-25 17:28:39 +02:00
Outdated
Review
Copy Link

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.
Hans Goudey commented 2023-04-25 17:59:54 +02:00
Outdated
Review
Copy Link

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?
Brecht Van Lommel commented 2023-04-25 21:19:31 +02:00
Outdated
Review
Copy Link

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.
Falk David commented 2023-04-25 21:23:23 +02:00
Outdated
Review
Copy Link

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

I see, re-creating the user count when reading could work indeed.
* In this example there are three drawings (drawing #0, * In this example there are three drawings (drawing #0, drawing #1 and drawing #2). The first
* drawing #1 and drawing #2). The first drawing starts at frame 0 * drawing starts at frame 0 and ends at frame 5 (excusive). The second drawing starts at
* 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 5 and ends at frame 10. Finally, the third drawing starts * frame 16.
* at frame 12 and ends at frame 16.
* *
* | | | | | | | | | | |1|1|1|1|1|1|1| * | | | | | | | | | | |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|... * Time: |0|1|2|3|4|5|6|7|8|9|0|1|2|3|4|5|6|...
* Frame: [#0 ][#1 ] [#2 ] * Frame: [#0 ][#1 ] [#2 ]
* *
* Note: If a drawing references another data-block, all of the drawings * \note If a drawing references another data-block, all of the drawings in that data-block are
* in that data-block are mapped sequentially to the frames. * mapped sequentially to the frames (frame-by-frame). If another frame starts, the rest of the
* If another frame starts, the rest of the mapped drawings * referenced drawings are discarded. If the frame is longer than the number of referenced
* are discarded. * drawings, then the last referenced drawing is held for the rest of the duration.
*/ */
#ifdef __cplusplus
const blender::Map<int, int> &frames() const; const blender::Map<int, int> &frames() const;
#endif #endif
@ -177,10 +174,10 @@ typedef struct GreasePencil {
blender::Span<GreasePencilDrawingOrReference> drawings() const; blender::Span<GreasePencilDrawingOrReference> drawings() const;
#endif #endif
#ifdef __cplusplus
/** /**
* The layer tree. * The layer tree.
*/ */
#ifdef __cplusplus
// const bke::gpencil::LayerTree &layer_tree() const; // const bke::gpencil::LayerTree &layer_tree() const;
#endif #endif
Hans Goudey commented 2023-04-25 04:32:41 +02:00
Outdated
Review
Copy Link

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
Sergey Sharybin commented 2023-04-25 10:10:22 +02:00
Outdated
Review
Copy Link

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.
Hans Goudey commented 2023-04-25 14:52:41 +02:00
Outdated
Review
Copy Link

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.