blender
/
blender
134
398
Fork 579
Code Issues 6.3k Pull Requests 750 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 +4 -4
1 changed files with 4 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 229e5a54b4 - Show all commits

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

@ -146,15 +146,15 @@ static void grease_pencil_blend_write(BlendWriter *writer, ID *id, const void *i
{
GreasePencil *grease_pencil = reinterpret_cast<GreasePencil *>(id);
Bastien Montagne commented 2023-05-06 19:06:50 +02:00
Outdated
Review
Copy Link

This has one important implication that should be noted in comments imho: it is crucial to not free that storage until the whole Main has been written (otherwise it could lead to same memory being re-used several time for different data, which would break .blend files in unpredictable ways). AFAICT current code is correct on that regard, but this should still be clearly documented.

There is also a serious issue here with regard to memfile undo: Since this data is re-generated on every file write (i.e. also on every undo step storage), it will automatically always mark the GP ID as modified for all undo steps. Think this should be re-used as much as possible instead.

This has one important implication that should be noted in comments imho: it is crucial to not free that storage until the whole Main has been written (otherwise it could lead to same memory being re-used several time for different data, which would break .blend files in unpredictable ways). AFAICT current code is correct on that regard, but this should still be clearly documented. There is also a serious issue here with regard to memfile undo: Since this data is re-generated on every file write (i.e. also on every undo step storage), it will automatically always mark the GP ID as modified for _all_ undo steps. Think this should be re-used as much as possible instead.
/* Write LibData */
BLO_write_id_struct(writer, GreasePencil, id_address, &grease_pencil->id);
filedescriptor marked this conversation as resolved Outdated
Bastien Montagne commented 2023-05-25 12:30:53 +02:00
Outdated
Review
Copy Link

EEEEEK! ID data itself should always be written first, before absolutely anything else!

EEEEEK! ID data itself should always be written first, before absolutely anything else!
BKE_id_blend_write(writer, &grease_pencil->id);
/* Write animation data. */
if (grease_pencil->adt) {
BKE_animdata_blend_write(writer, grease_pencil->adt);
}
/* Write LibData */
BLO_write_id_struct(writer, GreasePencil, id_address, &grease_pencil->id);
BKE_id_blend_write(writer, &grease_pencil->id);
/* Write drawings. */
Hans Goudey commented 2023-04-25 04:54:27 +02:00
Outdated
Review
Copy Link

Here the comment says "Write drawings" and the code says "write drawing array". That's basically the same thing-- the comment isn't really necessary. Same with the others here IMO.

Here the comment says "Write drawings" and the code says "write drawing array". That's basically the same thing-- the comment isn't really necessary. Same with the others here IMO.
Hans Goudey commented 2023-05-02 23:45:36 +02:00
Outdated
Review
Copy Link

Maybe this comment was missed? Or do you disagree?

Maybe this comment was missed? Or do you disagree?
grease_pencil->write_drawing_array(writer);
/* Write layer tree. */