Fixed regression of 71fa411100

where the PrintObject bounding box was not invalidated correctly
when the transformation matrices of instances changed.

Also refactored PrintObject::size() for const correctness.
This commit is contained in:
bubnikv 2020-01-30 12:22:22 +01:00
parent e679bcced6
commit c5fa400208
5 changed files with 21 additions and 17 deletions

View file

@ -1091,7 +1091,7 @@ namespace DoExport {
static inline std::vector<const PrintInstance*> sort_object_instances_by_max_z(const Print &print) static inline std::vector<const PrintInstance*> sort_object_instances_by_max_z(const Print &print)
{ {
std::vector<const PrintObject*> objects(print.objects().begin(), print.objects().end()); std::vector<const PrintObject*> objects(print.objects().begin(), print.objects().end());
std::sort(objects.begin(), objects.end(), [](const PrintObject *po1, const PrintObject *po2) { return po1->size(2) < po2->size(2); }); std::sort(objects.begin(), objects.end(), [](const PrintObject *po1, const PrintObject *po2) { return po1->size()(2) < po2->size()(2); });
std::vector<const PrintInstance*> instances; std::vector<const PrintInstance*> instances;
instances.reserve(objects.size()); instances.reserve(objects.size());
for (const PrintObject *object : objects) for (const PrintObject *object : objects)

View file

@ -907,10 +907,8 @@ const BoundingBoxf3& ModelObject::raw_bounding_box() const
const Transform3d& inst_matrix = this->instances.front()->get_transformation().get_matrix(true); const Transform3d& inst_matrix = this->instances.front()->get_transformation().get_matrix(true);
for (const ModelVolume *v : this->volumes) for (const ModelVolume *v : this->volumes)
{
if (v->is_model_part()) if (v->is_model_part())
m_raw_bounding_box.merge(v->mesh().transformed_bounding_box(inst_matrix * v->get_matrix())); m_raw_bounding_box.merge(v->mesh().transformed_bounding_box(inst_matrix * v->get_matrix()));
}
} }
return m_raw_bounding_box; return m_raw_bounding_box;
} }

View file

@ -836,7 +836,7 @@ Print::ApplyStatus Print::apply(const Model &model, DynamicPrintConfig new_full_
// Update the ModelObject instance, possibly invalidate the linked PrintObjects. // Update the ModelObject instance, possibly invalidate the linked PrintObjects.
assert(it_status->status == ModelObjectStatus::Old || it_status->status == ModelObjectStatus::Moved); assert(it_status->status == ModelObjectStatus::Old || it_status->status == ModelObjectStatus::Moved);
// Check whether a model part volume was added or removed, their transformations or order changed. // Check whether a model part volume was added or removed, their transformations or order changed.
// Only volume IDs, volume types and their order are checked, configuration and other parameters are NOT checked. // Only volume IDs, volume types, transformation matrices and their order are checked, configuration and other parameters are NOT checked.
bool model_parts_differ = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::MODEL_PART); bool model_parts_differ = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::MODEL_PART);
bool modifiers_differ = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::PARAMETER_MODIFIER); bool modifiers_differ = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::PARAMETER_MODIFIER);
bool support_blockers_differ = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::SUPPORT_BLOCKER); bool support_blockers_differ = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::SUPPORT_BLOCKER);
@ -899,10 +899,14 @@ Print::ApplyStatus Print::apply(const Model &model, DynamicPrintConfig new_full_
model_object.instances.emplace_back(new ModelInstance(*model_instance)); model_object.instances.emplace_back(new ModelInstance(*model_instance));
model_object.instances.back()->set_model_object(&model_object); model_object.instances.back()->set_model_object(&model_object);
} }
} else { } else if (! std::equal(model_object.instances.begin(), model_object.instances.end(), model_object_new.instances.begin(),
// Just synchronize the content of the instances. This avoids memory allocation and it does not invalidate ModelInstance pointers, [](auto l, auto r){ return l->print_volume_state == r->print_volume_state && l->printable == r->printable &&
// which may be accessed by G-code export in the meanwhile to deduce sequential print order. l->get_transformation().get_matrix().isApprox(r->get_transformation().get_matrix()); })) {
auto new_instance = model_object_new.instances.begin(); // If some of the instances changed, the bounding box of the updated ModelObject is likely no more valid.
// This is safe as the ModelObject's bounding box is only accessed from this function, which is called from the main thread only.
model_object.invalidate_bounding_box();
// Synchronize the content of instances.
auto new_instance = model_object_new.instances.begin();
for (auto old_instance = model_object.instances.begin(); old_instance != model_object.instances.end(); ++ old_instance, ++ new_instance) { for (auto old_instance = model_object.instances.begin(); old_instance != model_object.instances.end(); ++ old_instance, ++ new_instance) {
(*old_instance)->set_transformation((*new_instance)->get_transformation()); (*old_instance)->set_transformation((*new_instance)->get_transformation());
(*old_instance)->print_volume_state = (*new_instance)->print_volume_state; (*old_instance)->print_volume_state = (*new_instance)->print_volume_state;
@ -1197,7 +1201,7 @@ std::string Print::validate() const
{ {
std::vector<coord_t> object_height; std::vector<coord_t> object_height;
for (const PrintObject *object : m_objects) for (const PrintObject *object : m_objects)
object_height.insert(object_height.end(), object->instances().size(), object->size(2)); object_height.insert(object_height.end(), object->instances().size(), object->size()(2));
std::sort(object_height.begin(), object_height.end()); std::sort(object_height.begin(), object_height.end());
// Ignore the tallest *copy* (this is why we repeat height for all of them): // Ignore the tallest *copy* (this is why we repeat height for all of them):
// it will be printed as last one so its height doesn't matter. // it will be printed as last one so its height doesn't matter.
@ -1429,7 +1433,7 @@ BoundingBox Print::bounding_box() const
for (const PrintObject *object : m_objects) for (const PrintObject *object : m_objects)
for (const PrintInstance &instance : object->instances()) { for (const PrintInstance &instance : object->instances()) {
bb.merge(instance.shift); bb.merge(instance.shift);
bb.merge(instance.shift + to_2d(object->size)); bb.merge(instance.shift + to_2d(object->size()));
} }
return bb; return bb;
} }

View file

@ -120,17 +120,17 @@ public:
// so that next call to make_perimeters() performs a union() before computing loops // so that next call to make_perimeters() performs a union() before computing loops
bool typed_slices; bool typed_slices;
Vec3crd size; // XYZ in scaled coordinates // XYZ in scaled coordinates
const Vec3crd& size() const { return m_size; }
const PrintObjectConfig& config() const { return m_config; } const PrintObjectConfig& config() const { return m_config; }
const LayerPtrs& layers() const { return m_layers; } const LayerPtrs& layers() const { return m_layers; }
const SupportLayerPtrs& support_layers() const { return m_support_layers; } const SupportLayerPtrs& support_layers() const { return m_support_layers; }
const Transform3d& trafo() const { return m_trafo; } const Transform3d& trafo() const { return m_trafo; }
const PrintInstances& instances() const { return m_instances; } const PrintInstances& instances() const { return m_instances; }
const Point instance_center(size_t idx) const { return m_instances[idx].shift + m_copies_shift + Point(this->size.x() / 2, this->size.y() / 2); } const Point instance_center(size_t idx) const { return m_instances[idx].shift + m_copies_shift + Point(this->size().x() / 2, this->size().y() / 2); }
// since the object is aligned to origin, bounding box coincides with size // since the object is aligned to origin, bounding box coincides with size
BoundingBox bounding_box() const { return BoundingBox(Point(0,0), to_2d(this->size)); } BoundingBox bounding_box() const { return BoundingBox(Point(0,0), to_2d(this->size())); }
// adds region_id, too, if necessary // adds region_id, too, if necessary
void add_region_volume(unsigned int region_id, int volume_id, const t_layer_height_range &layer_range) { void add_region_volume(unsigned int region_id, int volume_id, const t_layer_height_range &layer_range) {
@ -235,6 +235,8 @@ private:
void combine_infill(); void combine_infill();
void _generate_support_material(); void _generate_support_material();
// XYZ in scaled coordinates
Vec3crd m_size;
PrintObjectConfig m_config; PrintObjectConfig m_config;
// Translation in Z + Rotation + Scaling / Mirroring. // Translation in Z + Rotation + Scaling / Mirroring.
Transform3d m_trafo = Transform3d::Identity(); Transform3d m_trafo = Transform3d::Identity();

View file

@ -43,7 +43,7 @@ namespace Slic3r {
PrintObject::PrintObject(Print* print, ModelObject* model_object, bool add_instances) : PrintObject::PrintObject(Print* print, ModelObject* model_object, bool add_instances) :
PrintObjectBaseWithState(print, model_object), PrintObjectBaseWithState(print, model_object),
typed_slices(false), typed_slices(false),
size(Vec3crd::Zero()) m_size(Vec3crd::Zero())
{ {
// Compute the translation to be applied to our meshes so that we work with smaller coordinates // Compute the translation to be applied to our meshes so that we work with smaller coordinates
{ {
@ -56,7 +56,7 @@ PrintObject::PrintObject(Print* print, ModelObject* model_object, bool add_insta
const BoundingBoxf3 modobj_bbox = model_object->raw_bounding_box(); const BoundingBoxf3 modobj_bbox = model_object->raw_bounding_box();
m_copies_shift = Point::new_scale(modobj_bbox.min(0), modobj_bbox.min(1)); m_copies_shift = Point::new_scale(modobj_bbox.min(0), modobj_bbox.min(1));
// Scale the object size and store it // Scale the object size and store it
this->size = (modobj_bbox.size() * (1. / SCALING_FACTOR)).cast<coord_t>(); this->m_size = (modobj_bbox.size() * (1. / SCALING_FACTOR)).cast<coord_t>();
} }
if (add_instances) { if (add_instances) {
@ -1450,7 +1450,7 @@ void PrintObject::update_slicing_parameters()
{ {
if (! m_slicing_params.valid) if (! m_slicing_params.valid)
m_slicing_params = SlicingParameters::create_from_config( m_slicing_params = SlicingParameters::create_from_config(
this->print()->config(), m_config, unscale<double>(this->size(2)), this->object_extruders()); this->print()->config(), m_config, unscale<double>(this->size()(2)), this->object_extruders());
} }
SlicingParameters PrintObject::slicing_parameters(const DynamicPrintConfig& full_config, const ModelObject& model_object, float object_max_z) SlicingParameters PrintObject::slicing_parameters(const DynamicPrintConfig& full_config, const ModelObject& model_object, float object_max_z)