Changeset View
Changeset View
Standalone View
Standalone View
source/blender/io/alembic/intern/abc_reader_mesh.cc
| Show All 37 Lines | |||||
| #include "BKE_main.h" | #include "BKE_main.h" | ||||
| #include "BKE_material.h" | #include "BKE_material.h" | ||||
| #include "BKE_mesh.h" | #include "BKE_mesh.h" | ||||
| #include "BKE_modifier.h" | #include "BKE_modifier.h" | ||||
| #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::Abc::PropertyHeader; | |||||
| using Alembic::AbcGeom::IC3fGeomParam; | |||||
| using Alembic::AbcGeom::IC4fGeomParam; | |||||
| 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::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; | ||||
| ▲ Show 20 Lines • Show All 434 Lines • ▼ Show 20 Lines | AbcMeshReader::AbcMeshReader(const IObject &object, ImportSettings &settings) | ||||
| get_min_max_time(m_iobject, m_schema, m_min_time, m_max_time); | get_min_max_time(m_iobject, m_schema, m_min_time, m_max_time); | ||||
| } | } | ||||
| bool AbcMeshReader::valid() const | bool AbcMeshReader::valid() const | ||||
| { | { | ||||
| return m_schema.valid(); | return m_schema.valid(); | ||||
| } | } | ||||
| template<class typedGeomParam> | |||||
| bool is_valid_animated(const ICompoundProperty arbGeomParams, const PropertyHeader &prop_header) | |||||
| { | |||||
| if (typedGeomParam::matches(prop_header)) { | |||||
| typedGeomParam color_param(arbGeomParams, prop_header.getName()); | |||||
sybren: Given the parameters, it's no longer given that this is related to colour at all. Probably it's… | |||||
| if (color_param.valid() && !color_param.isConstant()) { | |||||
| return true; | |||||
| } | |||||
| } | |||||
| return false; | |||||
sybrenUnsubmitted 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… | |||||
| } | |||||
| bool has_animations_vcols(const ICompoundProperty arbGeomParams) | |||||
sybrenUnsubmitted 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… | |||||
| { | |||||
| if (arbGeomParams == nullptr) { | |||||
sybrenUnsubmitted 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()`? | |||||
| return false; | |||||
| } | |||||
| const int num_props = arbGeomParams.getNumProperties(); | |||||
| for (int i = 0; i < num_props; i++) { | |||||
| const PropertyHeader &prop_header = arbGeomParams.getPropertyHeader(i); | |||||
| if (is_valid_animated<IC3fGeomParam>(arbGeomParams, prop_header)) { | |||||
| return true; | |||||
| } | |||||
| else if (is_valid_animated<IC4fGeomParam>(arbGeomParams, prop_header)) { | |||||
sybrenUnsubmitted Done Inline ActionsNo else after return. sybren: No `else` after `return`. | |||||
| return true; | |||||
| } | |||||
| } | |||||
| return false; | |||||
| } | |||||
| /* 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; | ||||
| } | } | ||||
| IV2fGeomParam uvsParam = schema.getUVsParam(); | IV2fGeomParam uvsParam = schema.getUVsParam(); | ||||
| if (uvsParam.valid() && !uvsParam.isConstant()) { | |||||
| return true; | |||||
| } | |||||
| IN3fGeomParam normalsParam = schema.getNormalsParam(); | IN3fGeomParam normalsParam = schema.getNormalsParam(); | ||||
| return (uvsParam.valid() && !uvsParam.isConstant()) || | if (normalsParam.valid() && !normalsParam.isConstant()) { | ||||
| (normalsParam.valid() && !normalsParam.isConstant()); | return true; | ||||
| } | |||||
| ICompoundProperty arbGeomParams = schema.getArbGeomParams(); | |||||
Done Inline ActionsDoes this always return a valid ICompoundProperty? sybren: Does this always return a valid `ICompoundProperty`? | |||||
| if (has_animations_vcols(arbGeomParams)) { | |||||
| return true; | |||||
| } | |||||
| return false; | |||||
| } | } | ||||
| void AbcMeshReader::readObjectData(Main *bmain, const Alembic::Abc::ISampleSelector &sample_sel) | void AbcMeshReader::readObjectData(Main *bmain, const Alembic::Abc::ISampleSelector &sample_sel) | ||||
| { | { | ||||
| 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) { | ||||
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… | |||||
| /* 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() */ | ||||
Done Inline ActionsThis should be its own function. sybren: This should be its own function. | |||||
| /* 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); | ||||
| ▲ Show 20 Lines • Show All 424 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.