Page MenuHome

T66924 : Move GPencil Modifiers evaluation to Depsgraph
Needs ReviewPublic

Authored by Antonio Vazquez (antoniov) on Tue, Aug 13, 1:09 PM.

Details

Summary

Before, the evaluation of modifers were done in draw manager. The reason of the old design was grease pencil was designed before depsgraph was in place.

This commit moves this logic to depsgraph to follow general design and reduce Draw Manager complexity. Also, this is required in order to use modifiers in Edit modes.

Really, there is nothing really new in the creation of derived data, only the logic has been moved to depsgraph, but the main logic is the same. In order to get a reference to the original stroke and points, a pointer is added to Runtime data as part of the evaluated data. These pointers allow to know and use the original data.

As the modifiers now are evaluated in Depsgraph, the evaluated stroke is usable in Edit modes, so now it's possible to work with the evaluated version instead to use a "ghost" of the final image over the original geometry as work today.

I have more patches to use the evaluated data in Edit mode, but I will submit for review after apply this patch.

Diff Detail

Repository
rB Blender
Branch
temp-gpencil-depsgraph (branched from master)
Build Status
Buildable 4438
Build 4438: arc lint + arc unit

Event Timeline

The way the system works for meshes is that you make a copy of the Mesh datablock, and then apply modifiers to it, and then set ob->data to point to it. Then from the point of view of the drawing code or exporters, there is just an Object and Mesh to use, without a DerivedMesh. This seems to go a bit into the direction of the old DerivedMesh.

To be more similar to meshes, what you do is make a copy of bGPdata stored as gpencil_eval in Object_Runtime. Then you would modify the frames in its layers, instead of storing them separately as derived_frames.

Is there a specific reason this was implemented in a different way, or was it a matter of not being familiar with the way this works for meshes?

I know the mesh use a evaluated mesh datablock, but there are reasons to do in this way:

The complexity of data is more than meshes. In meshes you have "one data level", here you have layers/frames/strokes/points/weights. In meshes you can use a very dense mesh, but you have only a level, but here we have more.

The amount of data can be huge, so for any change, recalc all data is not an option. Here is easy to have several layers, with several frames, with hundreds of strokes with hundreds of points, so the result would be: nlayers * nframes * nStrokes * (nPoints & n Weights). Also, the GPU data must be by frame , not a simple geometry load for all keyframes.

Also, the data can change very fast. In meshes you must enter in Edit mode and then edit the mesh, back to Object mode and repeat, but here you can add strokes, remove, change, etc very fast and the data is changing all the time. We could implement a cache system to update only a section but today manage the Object cahe for drawing, the GPU data cache and Fill triangulation cache is a nightmare, so add a new cache in top of that is not in my plans.

I don't say it cannot be done as meshes, but with the current logic we get a good FPS and keep the memory size controlled. For example, I had to remove a full copy of GPD datablock while drawing a stroke because there were a noticeable lag, so we cannot risk to do the same for general data evaluation.

Also, thinking is how GP data works, as you have different information in each frame, to evaluate all datablock, I would loop all frames, but if I calculate only the current frame, then make full copy of the datablock is not logic because we are coping something we are not going to use.

Brecht Van Lommel (brecht) requested changes to this revision.Wed, Aug 14, 5:06 PM

Ok. I expect that you can make it so the amount of copying is no more than it would be with this derived frames system, by e.g. leaving only the current frame in the evaluated grease pencil data and other optimizations. But if this works well as is I don't really have a problem with it.

Mainly the copy-on-write thing becomes more important when you think about arbitrary node systems modifying grease pencil data, but if that ever happens it can be solved then.

source/blender/blenkernel/BKE_gpencil.h
53

Did all these structs really need to move out of the draw module and into blenkernel?

As far as I know for other data structures this stays in the draw module. Probably any code outside that needs to access it can call into some function in the draw module?

source/blender/makesdna/DNA_object_types.h
183

Add a gpencil_ prefix to tot_layers and derived_frames.

This revision now requires changes to proceed.Wed, Aug 14, 5:06 PM
source/blender/blenkernel/BKE_gpencil.h
53

You are right, better in draw module.

source/blender/makesdna/DNA_object_types.h
183

I will rename them.

  • GPencil: Rename variables adding gpencil_ prefix
  • GPencil: Move code outside BKE module
  • GPencil: Cleanup Marcro names adding GPENCIL prefix
  • Cleanup: Move struct definition

I moved the struct definition to old place to make patch small.

  • GPencil: More cleanup to reduce patch size

Not sure if the naming would be better with the changes above.

source/blender/blenkernel/intern/gpencil_modifier.c
782

maybe better naming is gpencil_frame_copy

816

@brech maybe gpencil_derived_frame_ensure is better naming?