Details

Reviewers

Jacques Lucke (JacquesLucke)
Brecht Van Lommel (brecht)

Maniphest Tasks

T95967: Struct of Arrays Refactor for Mesh Polygons

Commits

rBf1c0249f34c4: Mesh: Move material indices to a generic attribute

Summary

This patch moves material indices from the mesh MPoly struct to a
generic integer attribute. The builtin material index was already
exposed in geometry nodes, but this makes it a "proper" attribute
accessible with Python and visible in the "Attributes" panel.

The goals of the refactor are code simplification and memory and
performance improvements, mainly because the attribute doesn't have
to be stored and processed if there are no materials. However, until
4.0, material indices will still be read and written in the old
format, meaning there may be a temporary increase in memory usage.

Further notes:

Completely removing the MPoly.mat_nr after 4.0 may require changes to DNA or introducing a new MPoly type.
Geometry nodes regression tests didn't look at material indices, so the change reveals a bug in the realize instances node that I fixed.
Access to material indices from the RNA MeshPolygon type is slower with this patch. The material_index attribute can be used instead.
Cycles is changed to read from the attribute instead.
BMesh isn't changed in this patch. Theoretically it could be though, to save 2 bytes per face when no materials are used.
Eventually we could use a 16 bit integer attribute type instead.

Ref T95967

Branch: refactor-mesh-material-index-generic

Diff Detail

Repository: rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Aug 12 2022, 6:01 PM

Hans Goudey (HooglyBoogly) created this revision.

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Aug 14 2022, 1:25 AM

Compiling, basics working

Add comment
Fix legacy conversion
Fix crash in bmesh to mesh conversion
Use attribute API for retrieving material indices in Cycles
Fix empty material mapping in the realize instances node
Cleanup
Fix realize instances node
Avoid retrieving material indices many times in draw code
Temporarily disable layer count mismatch test
Fix typo causing crash
Fix: Account for null material indices
Cleanup: Use attribute API
Cleanup: Add using namespaces
Cleanup: Remove unnecessary subsurf DerivedMesh material index handling
Fix old attribute collision warning
Fix missing BMesh table
Fix last use of deprecated material index struct member
Fix build error
Compiling, basics working

This patch is mostly just like the other similar refactors. But maybe @Brecht Van Lommel (brecht) wants to take a look at the Cycles change.
And maybe Jacques is interested in a the few geometry nodes changes. The realize instances node is the most complex change.

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Aug 28 2022, 8:52 PM

Brecht Van Lommel (brecht) requested changes to this revision.Aug 29 2022, 3:22 PM

Brecht Van Lommel (brecht) added inline comments.

source/blender/blenkernel/intern/mesh.cc
1422	Should this ensure the attribute is the on the face domain?
1448	Wrong domain
1461	Should this ensure the attribute is the on the face domain?
source/blender/blenkernel/intern/pbvh.c
151	This should be `if (pbvh->material_indices[a] != pbvh->material_indices[b]) return false;`. Otherwise it skips the smooth test.

This revision now requires changes to proceed.Aug 29 2022, 3:22 PM

Fix face materials match PBVH check
Use correct domain
Add an assert that material indices are on the correct domain, don't crash
Merge branch 'master' into refactor-mesh-material-index-generic

source/blender/blenkernel/intern/mesh.cc
1422	The attribute system (`create_attribute_providers_for_mesh`) ensures that doesn't happen. But the conversion from BMesh to Mesh doesn't use that system, so I guess it doesn't hurt to be safe for now.

https://builder.blender.org/admin/#/builders/57/builds/1473

Brecht Van Lommel (brecht) accepted this revision.Aug 30 2022, 10:45 AM

This revision is now accepted and ready to land.Aug 30 2022, 10:45 AM

Tests fail currently, so that should be fixed.

This revision now requires changes to proceed.Aug 30 2022, 1:14 PM

Merge master

Tests are passing locally, so I'm not sure what's up with that. I'll try running them again with this patch.

https://builder.blender.org/admin/#/builders/57/builds/1475

Fix cycles retrieval of material indices (and tests)

I wasn't compiling with Freestyle, which disabled some tests.

Brecht Van Lommel (brecht) accepted this revision.Aug 31 2022, 3:03 PM

Merge master

Jacques Lucke (JacquesLucke) accepted this revision.Aug 31 2022, 3:26 PM

This revision is now accepted and ready to land.Aug 31 2022, 3:26 PM

Closed by commit rBf1c0249f34c4: Mesh: Move material indices to a generic attribute (authored by Hans Goudey (HooglyBoogly)). · Explain WhyAug 31 2022, 4:09 PM

This revision was automatically updated to reflect the committed changes.

Hans Goudey (HooglyBoogly) added a commit: rBf1c0249f34c4: Mesh: Move material indices to a generic attribute.

Iliya Katueshenock (Moder) mentioned this in T103663: Regression: The Material Selection node in Geometry Nodes doesn't select any geometry.Jan 5 2023, 5:07 PM

Iliya Katueshenock (Moder) mentioned this in D16926: Fix T103663: Fill single selected material not by false.Jan 5 2023, 5:19 PM

Mesh: Move material indices to a generic attributeClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 55092

intern/cycles/blender/mesh.cpp

release/scripts/startup/bl_ui/properties_data_mesh.py

source/blender/blenkernel/BKE_mesh.h

source/blender/blenkernel/BKE_mesh_legacy_convert.h

source/blender/blenkernel/intern/customdata.cc

source/blender/blenkernel/intern/fluid.c

source/blender/blenkernel/intern/geometry_component_mesh.cc

source/blender/blenkernel/intern/gpencil_geom.cc

source/blender/blenkernel/intern/mesh.cc

source/blender/blenkernel/intern/mesh_boolean_convert.cc

source/blender/blenkernel/intern/mesh_convert.cc

source/blender/blenkernel/intern/mesh_legacy_convert.cc

source/blender/blenkernel/intern/mesh_validate.cc

source/blender/blenkernel/intern/pbvh.c

source/blender/blenkernel/intern/pbvh_intern.h

source/blender/blenkernel/intern/subdiv_ccg_material.c

source/blender/blenkernel/intern/subsurf_ccg.c

source/blender/blenlib/BLI_vector_set.hh

source/blender/bmesh/intern/bmesh_mesh_convert.cc

source/blender/draw/intern/draw_cache_extract_mesh_render_data.cc

source/blender/draw/intern/draw_cache_impl_subdivision.cc

source/blender/draw/intern/mesh_extractors/extract_mesh.hh

source/blender/editors/mesh/meshtools.cc

source/blender/editors/object/object_bake.c

source/blender/editors/sculpt_paint/paint_image_proj.c

source/blender/editors/sculpt_paint/paint_utils.c

source/blender/freestyle/intern/blender_interface/BlenderFileLoader.cpp

source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp

source/blender/geometry/intern/realize_instances.cc

source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c

source/blender/gpu/intern/gpu_buffers.c

source/blender/io/alembic/exporter/abc_writer_mesh.cc

source/blender/io/alembic/intern/abc_reader_mesh.h

source/blender/io/alembic/intern/abc_reader_mesh.cc

source/blender/io/collada/GeometryExporter.cpp

source/blender/io/collada/MeshImporter.h

source/blender/io/collada/MeshImporter.cpp

source/blender/io/usd/intern/usd_reader_mesh.h

source/blender/io/usd/intern/usd_reader_mesh.cc

source/blender/io/usd/intern/usd_writer_mesh.cc

source/blender/io/wavefront_obj/exporter/obj_export_file_writer.cc

source/blender/io/wavefront_obj/exporter/obj_export_mesh.hh

source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc

source/blender/io/wavefront_obj/importer/obj_import_mesh.cc

source/blender/makesdna/DNA_mesh_types.h

source/blender/makesdna/DNA_meshdata_types.h

source/blender/makesrna/intern/rna_mesh.c

source/blender/modifiers/intern/MOD_screw.c

source/blender/modifiers/intern/MOD_solidify_extrude.c

source/blender/modifiers/intern/MOD_solidify_nonmanifold.c

source/blender/nodes/geometry/nodes/node_geo_material_selection.cc

source/blender/nodes/geometry/nodes/node_geo_set_material.cc

source/blender/render/intern/bake.c

source/blender/render/intern/multires_bake.c

Mesh: Move material indices to a generic attribute
ClosedPublic
Actions

Revision Contents
Changeset List