Page MenuHome

Fix T103400: Transfer Mesh Data Layout broken for color attributes
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Jan 6 2023, 4:39 PM.

Details

Summary

This was the case when using the operator outside of the modifiers panel.

Caused by rBeae36be372a6: Refactor: Unify vertex and sculpt colors into new.

In above commit, DT_layer_items shared both DT_TYPE_MPROPCOL_LOOP |
DT_TYPE_MLOOPCOL_LOOP in a single EnumPropertyItem value "Colors".
This is a bit unusual, but probably allowed.
As a consequence, checks for specific datatypes would fail when selecting
such EnumPropertyItem:

  • DT_DATATYPE_IS_MULTILAYERS (uses ELEM to check distinct entries, would return false)
  • BKE_object_data_transfer_dttype_to_srcdst_index (would return DT_MULTILAYER_INDEX_INVALID)

These places have now been corrected to take these "special" values into
account.

Another issue was that multiple EnumPropertyItems with the same value
could be created in dt_add_vcol_layers() if attributes of the same
domain, but different color types are in play (could lead to crashes)
and that has also been corrected.

Also: above commit did not give the choice of transfering color
attributes from the vertex domain (only face corner attributes could be
chosen), this has now been added. DT_layer_vert_items (used from the
modifier) already had this included so this was only an issue when using
the operator outside of the modifiers panel.

Since we now feature two domains, the single "VCOL" in the enum has been
split into "COLOR_VERTEX" and "COLOR_CORNER". This will break existing
scripts calling bpy.ops.object.datalayout_transfer and will be marked as
a breaking change in the release notes.

NOTE: there is another bug here when attributes of the same domain, but different color types are in play and you want to transfer just a single specific layer (but that is for a separate commit)

Diff Detail

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

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Jan 6 2023, 4:39 PM

In D12587 I advocated for adding the data transfer feature as a separate commit... and here we are!
Thanks for fixing this anyway. The changes look reasonable.

source/blender/editors/object/object_data_transfer.c
63

Is there a reason to use VCOL here? What about COLOR_ATTRIBUTES?

97

What about color_attribute_index?

This revision is now accepted and ready to land.Jan 9 2023, 6:54 PM
source/blender/editors/object/object_data_transfer.c
63

Hm.

COLOR_ATTRIBUTES_VERT / COLOR_ATTRIBUTES_FACE_CORNER (see case below) would be good I think.
Using VCOL for face corners is not great either.

This would break existing scripts though (this is what you use when calling bpy.ops.object.datalayout_transfer(data_type='VCOL'))

@Joseph Eagar (joeedh) @Campbell Barton (campbellbarton) Hey, can you please review this patch, since this fixes a high priority report. Thanks in advance.

source/blender/editors/object/object_data_transfer.c
63

Okay, for the data transfer modifier it's probably better to prefer backwards compatibility with scripts, since it's being replaced by geometry nodes for new development anyway.

We could go with COLOR_VERTEX and COLOR_CORNER I suppose, dunno if we need 'attribute' in the name.

well I suppose we will go with COLOR_VERTEX and COLOR_CORNER then (this seems to be more correct than using VCOL twice in an enum -- even though it might break existing scripts)?
Will update Diff shortly, ask for review again and also mention this as a breaking change in the release notes once committed.

use "COLOR_VERTEX" and "COLOR_CORNER" to distinguish domains in the enum

Philipp Oeser (lichtwerk) requested review of this revision.Wed, Jan 18, 1:44 PM
Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)
Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)

Sorry, would like green light again after last change

This revision is now accepted and ready to land.Wed, Jan 18, 11:23 PM