- Amsterdam
-
Asset system (development lead), UI (developer & module coordindator), VR (initial development).
- Joined on
2013-12-12 -
-
I think this bit of grease pencil specific code is fine to have here. But why this logic instead of a simple CLAMP()? Or better even, C++ std::clamp().
I would create this wrapper object in duplicate_ids(), just like we do it for instance empties there. Technically it's not thread safe, but I think it's fine not to copy the grease pencil data, and just create a wrapper object. We do the same for objects (the object data isn't copied I think) and collections (the collection itself isn't copied). This is also to avoid the memory and performance costs of the copies, this can be quite complex data.
This change means the frame for isn't set anymore for non grease pencil object.
Such a check is super fragile. If somebody adds another case where preview_data->object is null, this will just assume we're rendering for grease pencil.
Avoid these sneaky exceptions. If you want the grease pencil dropbox to have priority, adding it first should work.
Not sure if I forgot to note this earlier or if you intentionally didn't change it. Rather than an Asset menu, this can live under Grease Pencil > Create Asset, similar to how there is Object > Asset in object mode.
This and the following descriptions should read "Create an asset...".
This tags the depsgraph and sends notifiers... stuff that should only be done when data actually changes, not when cancelling an operation without having made changes. In this case a simple freeing of data should be sufficient.
Is this really cancelling the operation? It seems gpencil_asset_append_strokes() only returns false if the grease pencil asset is empty. In this case I would just display an info report to give the user some feedback, and return OPERATOR_FINISHED.
MEM_delete() on a void pointer doesn't make sense, it shouldn't even compile. Committed 4126284e46 so this is a compiler error on all platforms in future.
In fact, there doesn't seem to be a need to use customdata at all? This just makes it hard to follow where/how the data is used. It's meant to make data available throughout multiple callbacks.
Suggest: "Add strokes from a grease pencil asset to an existing grease pencil object".
Actually, seems like this operator can only be used for ID_GD? Why make the type optional then?
Shouldn't be needed, the vector is default constructed cleanly.