Expose Face Sets In Python API
AbandonedPublic
Actions

Authored by Joseph Eagar (joeedh) on May 25 2022, 6:03 PM.

Details

Reviewers

Campbell Barton (campbellbarton)

Maniphest Tasks

T98382: Expose Face Sets In Python API

Summary

This patch exposes face sets in the python API, for both BMesh and normal RNA.

Diff Detail

Event Timeline

Joseph Eagar (joeedh) requested review of this revision.May 25 2022, 6:03 PM

Joseph Eagar (joeedh) created this revision.

Julien Kaspar (JulienKaspar) added a task: T98382: Expose Face Sets In Python API.May 25 2022, 7:42 PM

This seems mostly fine, two things though:

I don't think we should be making the values absolute, besides silently changing the value being confusing for script authors, it means you can't (for example) backup/restore values, or copy one face set to another face.

Instead, negative values could documented as for use by sculpt internal logic.

Settle on a name: face_set ? sculpt_face_set ? If it's likely we'll use them in other modes, I suppose face_set is reasonable, but in that case existing use of sculpt_face_sets should be changed too.

Campbell Barton (campbellbarton) requested changes to this revision.Jun 28 2022, 7:22 AM

This revision now requires changes to proceed.Jun 28 2022, 7:22 AM

Andy (MooKorea) added a subscriber: Andy (MooKorea).Jul 18 2022, 6:05 AM

Make requested changes

In D15025#408344, @Campbell Barton (campbellbarton) wrote:

This seems mostly fine, two things though:

I don't think we should be making the values absolute, besides silently changing the value being confusing for script authors, it means you can't (for example) backup/restore values, or copy one face set to another face.

Instead, negative values could documented as for use by sculpt internal logic.

Settle on a name: face_set ? sculpt_face_set ? If it's likely we'll use them in other modes, I suppose face_set is reasonable, but in that case existing use of sculpt_face_sets should be changed too.

I removed the absolute value calls and also renamed sculpt_face_sets to face_sets. Still need to update doc strings though.

This patch seems to imply that we might have multiple face set layers, but as far as I know there is only one. This makes sense since it's basically UI data for editing. I had assumed that the way to save multiple face set layers for later use would be through attributes (like T99297).

@Campbell Barton (campbellbarton) I'm aware Bcon3 started today, but I hope it's not too much to ask to have this implemented for 3.3

As noted by Hans, unless there is a plan to support multiple face-sets, the RNA property should expose this as a single layer.

There are also some warnings from this patch.

WARN (rna.define): rna_define.c:1647 RNA_def_property_ui_text: 'Face set value, starts at 1. 0 is a special value reserved for sculpt mode.' description from 'value' '' ends with a '.' !
[ 83%: -51] Building C object sour...eFiles/bf_rna.dir/rna_mesh_gen.c.o
/src/cmake_debug/source/blender/makesrna/intern/rna_mesh_gen.c:13472:9: warning: initialization of ‘char * (*)(const struct PointerRNA *)’ from incompatible pointer type ‘char * (*)(PointerRNA *)’ [-Wincompatible-pointer-types]
13472 |         rna_MeshFaceSetLayer_path,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~
/src/cmake_debug/source/blender/makesrna/intern/rna_mesh_gen.c:13472:9: note: (near initialization for ‘RNA_MeshFaceSetLayer.path’)
/src/cmake_debug/source/blender/makesrna/intern/rna_mesh_gen.c:13526:9: warning: initialization of ‘char * (*)(const struct PointerRNA *)’ from incompatible pointer type ‘char * (*)(PointerRNA *)’ [-Wincompatible-pointer-types]
13526 |         rna_MeshFaceSet_path,
      |         ^~~~~~~~~~~~~~~~~~~~
/src/cmake_debug/source/blender/makesrna/intern/rna_mesh_gen.c:13526:9: note: (near nitialization for ‘RNA_MeshFaceSet.path’)