Changeset View
Changeset View
Standalone View
Standalone View
source/blender/io/alembic/intern/abc_reader_mesh.cc
| Show First 20 Lines • Show All 41 Lines • ▼ Show 20 Lines | |||||
| #include "BKE_object.h" | #include "BKE_object.h" | ||||
| using Alembic::Abc::Int32ArraySamplePtr; | using Alembic::Abc::Int32ArraySamplePtr; | ||||
| using Alembic::Abc::P3fArraySamplePtr; | using Alembic::Abc::P3fArraySamplePtr; | ||||
| using Alembic::AbcGeom::IFaceSet; | using Alembic::AbcGeom::IFaceSet; | ||||
| using Alembic::AbcGeom::IFaceSetSchema; | using Alembic::AbcGeom::IFaceSetSchema; | ||||
| using Alembic::AbcGeom::IN3fGeomParam; | using Alembic::AbcGeom::IN3fGeomParam; | ||||
| using Alembic::AbcGeom::IC3fGeomParam; | |||||
| using Alembic::AbcGeom::IC4fGeomParam; | |||||
| using Alembic::AbcGeom::IObject; | using Alembic::AbcGeom::IObject; | ||||
| using Alembic::AbcGeom::IPolyMesh; | using Alembic::AbcGeom::IPolyMesh; | ||||
| using Alembic::AbcGeom::IPolyMeshSchema; | using Alembic::AbcGeom::IPolyMeshSchema; | ||||
| using Alembic::AbcGeom::ISampleSelector; | using Alembic::AbcGeom::ISampleSelector; | ||||
| using Alembic::AbcGeom::ISubD; | using Alembic::AbcGeom::ISubD; | ||||
| using Alembic::AbcGeom::ISubDSchema; | using Alembic::AbcGeom::ISubDSchema; | ||||
| using Alembic::AbcGeom::IV2fGeomParam; | using Alembic::AbcGeom::IV2fGeomParam; | ||||
| using Alembic::AbcGeom::kWrapExisting; | using Alembic::AbcGeom::kWrapExisting; | ||||
| ▲ Show 20 Lines • Show All 434 Lines • ▼ Show 20 Lines | |||||
| bool AbcMeshReader::valid() const | bool AbcMeshReader::valid() const | ||||
| { | { | ||||
| return m_schema.valid(); | return m_schema.valid(); | ||||
| } | } | ||||
| /* Specialisation of has_animations() as defined in abc_reader_object.h. */ | /* Specialisation of has_animations() as defined in abc_reader_object.h. */ | ||||
| template<> bool has_animations(Alembic::AbcGeom::IPolyMeshSchema &schema, ImportSettings *settings) | template<> bool has_animations(Alembic::AbcGeom::IPolyMeshSchema &schema, ImportSettings *settings) | ||||
| { | { | ||||
| if (settings->is_sequence || !schema.isConstant()) { | if (settings->is_sequence || !schema.isConstant()) { | ||||
| return true; | return true; | ||||
sybren: Given the parameters, it's no longer given that this is related to colour at all. Probably it's… | |||||
| } | } | ||||
| IV2fGeomParam uvsParam = schema.getUVsParam(); | IV2fGeomParam uvsParam = schema.getUVsParam(); | ||||
| IN3fGeomParam normalsParam = schema.getNormalsParam(); | IN3fGeomParam normalsParam = schema.getNormalsParam(); | ||||
| bool has_animated_vcols = false; | |||||
Done Inline ActionsHere you can flip the conditions and return early. if (!typedGeomParam::matches(prop_header)) {
return false;
}
typedGeomParam color_param(arbGeomParams, prop_header.getName());
return color_param.valid() && !color_param.isConstant();sybren: Here you can flip the conditions and return early.
```
if (!typedGeomParam::matches… | |||||
| ICompoundProperty arbGeomParams = schema.getArbGeomParams(); | |||||
| const size_t num_props = arbGeomParams.getNumProperties(); | |||||
| for (size_t i = 0; i < num_props; i++) { | |||||
Done Inline Actionshas_animated_... reads more naturally. There is no guarantee that this function will only pick up on animated vertex colors, though. There could be other IC3fGeomParam or IC4fGeomParam that cause the function to return true. This should be reflected in the name of the function. sybren: `has_animated_...` reads more naturally.
There is no guarantee that this function will only… | |||||
| const Alembic::Abc::PropertyHeader &prop_header = arbGeomParams.getPropertyHeader(i); | |||||
Done Inline ActionsI don't see how an ICompoundProperty can be nullptr. Maybe you mean to call .valid()? sybren: I don't see how an `ICompoundProperty` can be `nullptr`. Maybe you mean to call `.valid()`? | |||||
| if (IC3fGeomParam::matches(prop_header)) { | |||||
| IC3fGeomParam color_param(arbGeomParams, prop_header.getName()); | |||||
| if (color_param.valid() && !color_param.isConstant()) { | |||||
| has_animated_vcols = true; | |||||
| break; | |||||
| } | |||||
| } | |||||
| else if (IC4fGeomParam::matches(prop_header)) { | |||||
| IC3fGeomParam color_param(arbGeomParams, prop_header.getName()); | |||||
| if (color_param.valid() && !color_param.isConstant()) { | |||||
| has_animated_vcols = true; | |||||
Done Inline ActionsNo else after return. sybren: No `else` after `return`. | |||||
| break; | |||||
| } | |||||
| } | |||||
| } | |||||
| return (uvsParam.valid() && !uvsParam.isConstant()) || | return (uvsParam.valid() && !uvsParam.isConstant()) || | ||||
| (normalsParam.valid() && !normalsParam.isConstant()); | (normalsParam.valid() && !normalsParam.isConstant() || has_animated_vcols); | ||||
| } | } | ||||
| void AbcMeshReader::readObjectData(Main *bmain, const Alembic::Abc::ISampleSelector &sample_sel) | void AbcMeshReader::readObjectData(Main *bmain, const Alembic::Abc::ISampleSelector &sample_sel) | ||||
| { | { | ||||
Done Inline ActionsDoes this always return a valid ICompoundProperty? sybren: Does this always return a valid `ICompoundProperty`? | |||||
| Mesh *mesh = BKE_mesh_add(bmain, m_data_name.c_str()); | Mesh *mesh = BKE_mesh_add(bmain, m_data_name.c_str()); | ||||
| m_object = BKE_object_add_only_object(bmain, OB_MESH, m_object_name.c_str()); | m_object = BKE_object_add_only_object(bmain, OB_MESH, m_object_name.c_str()); | ||||
| m_object->data = mesh; | m_object->data = mesh; | ||||
| Mesh *read_mesh = this->read_mesh(mesh, sample_sel, MOD_MESHSEQ_READ_ALL, NULL); | Mesh *read_mesh = this->read_mesh(mesh, sample_sel, MOD_MESHSEQ_READ_ALL, NULL); | ||||
| if (read_mesh != mesh) { | if (read_mesh != mesh) { | ||||
| /* XXX fixme after 2.80; mesh->flag isn't copied by BKE_mesh_nomain_to_mesh() */ | /* XXX fixme after 2.80; mesh->flag isn't copied by BKE_mesh_nomain_to_mesh() */ | ||||
| /* read_mesh can be freed by BKE_mesh_nomain_to_mesh(), so get the flag before that happens. */ | /* read_mesh can be freed by BKE_mesh_nomain_to_mesh(), so get the flag before that happens. */ | ||||
| short autosmooth = (read_mesh->flag & ME_AUTOSMOOTH); | short autosmooth = (read_mesh->flag & ME_AUTOSMOOTH); | ||||
| BKE_mesh_nomain_to_mesh(read_mesh, mesh, m_object, &CD_MASK_MESH, true); | BKE_mesh_nomain_to_mesh(read_mesh, mesh, m_object, &CD_MASK_MESH, true); | ||||
| mesh->flag |= autosmooth; | mesh->flag |= autosmooth; | ||||
| } | } | ||||
| if (m_settings->validate_meshes) { | if (m_settings->validate_meshes) { | ||||
| BKE_mesh_validate(mesh, false, false); | BKE_mesh_validate(mesh, false, false); | ||||
Done Inline ActionsThe code in both if block is exactly the same, except for the variable type. This can be abstracted into a templated function. sybren: The code in both `if` block is exactly the same, except for the variable type. This can be… | |||||
| } | } | ||||
Done Inline ActionsThis should be its own function. sybren: This should be its own function. | |||||
| readFaceSetsSample(bmain, mesh, sample_sel); | readFaceSetsSample(bmain, mesh, sample_sel); | ||||
| if (has_animations(m_schema, m_settings)) { | if (has_animations(m_schema, m_settings)) { | ||||
| addCacheModifier(); | addCacheModifier(); | ||||
| } | } | ||||
| } | } | ||||
| ▲ Show 20 Lines • Show All 415 Lines • Show Last 20 Lines | |||||
Given the parameters, it's no longer given that this is related to colour at all. Probably it's better to rename color_param to geom_param.