Page MenuHome

GPencil: Prevent RNA assignment of invalid materials in modifiers
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Apr 1 2021, 11:49 AM.

Details

Summary

Materials used in grease pencil modifiers have the requirement that they
are already used on the object. In the UI dropdown, this restriction is
ensured by calling uiItemPointerR with appropriate searchptr and
searchpropname, so only giving the user the choice of materials already
used on the object.

From python though, it was still possible to assign materials outside of
this this restriction. This led to reports like T86981 [which have been
partially solved by clamping the material index in the modifier code to
be in the valid range].

Now make sure we dont assign "invalid" materials through RNA by
appropriate RNA pointer functions.

This also adds a proper warning (red, alert) in case of the LineArt
modifier if such a invalid material is still in the file [same as other
modifiers already do].

Diff Detail

Repository
rB Blender
Branch
gpencil_lineart_materials (branched from master)
Build Status
Buildable 13848
Build 13848: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Apr 1 2021, 11:49 AM
Philipp Oeser (lichtwerk) created this revision.

LGTM!

@Antonio Vazquez (antoniov) do you have any issues with it?

This revision is now accepted and ready to land.Apr 1 2021, 11:56 AM
Sebastian Parborg (zeddb) requested changes to this revision.Apr 1 2021, 12:00 PM

Ops spoke too soon.

source/blender/gpencil_modifiers/intern/MOD_gpencillineart.c
286

Shouldn't this be false?

So the check should actually be:

if (!RNA_pointer_is_null) {
 Material *current_material = material_ptr.data;
 ...

Right?

Because if the pointer is null, it is not a valid material?
(The lineart modifier is disabled in this case)

You have to select line style and material for the modifier to evaluate and generate lines currently.

This revision now requires changes to proceed.Apr 1 2021, 12:00 PM

warn also when material is cleared for the lineart modifier

Philipp Oeser (lichtwerk) marked an inline comment as done.Apr 1 2021, 12:08 PM
This revision is now accepted and ready to land.Apr 1 2021, 12:09 PM

LGTM. Really a good cleanup of the RNA Api.

Note: I agree with @Sebastian Parborg (zeddb), the RNA is NULL looks is inverted.