Skip to content

Commit

Permalink
Fixes issue #5979 - NULL pointer crash in ModelObject::split()
Browse files Browse the repository at this point in the history
ModelObject::split() expects a non-NULL new_objects vector where it adds pointers to the new models resulting from the split.
But in the CLI case the caller does not care about this and passes NULL which causes a crash. To fix the crash we could pass
a dummy vector but it turns out that we actually have a use for the results because we should assign a unique name to each
new model the same way as the GUI does. These names show up as comments in the gcode so this change makes the gcode produced
by the GUI and the CLI more similar and diffable.

@lukasmatena has amended the original commit by @combolek (pull request #5991) in order to avoid code duplication
  • Loading branch information
combolek authored and lukasmatena committed Feb 10, 2021
1 parent 3472c8d commit 22b2ccc
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 6 deletions.
3 changes: 2 additions & 1 deletion src/PrusaSlicer.cpp
Expand Up @@ -402,7 +402,8 @@ int CLI::run(int argc, char **argv)
for (Model &model : m_models) {
size_t num_objects = model.objects.size();
for (size_t i = 0; i < num_objects; ++ i) {
model.objects.front()->split(nullptr);
ModelObjectPtrs new_objects;
model.objects.front()->split(&new_objects);
model.delete_object(size_t(0));
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/libslic3r/Model.cpp
Expand Up @@ -1305,6 +1305,7 @@ void ModelObject::split(ModelObjectPtrs* new_objects)

ModelVolume* volume = this->volumes.front();
TriangleMeshPtrs meshptrs = volume->mesh().split();
size_t counter = 1;
for (TriangleMesh *mesh : meshptrs) {

// FIXME: crashes if not satisfied
Expand All @@ -1314,7 +1315,8 @@ void ModelObject::split(ModelObjectPtrs* new_objects)

// XXX: this seems to be the only real usage of m_model, maybe refactor this so that it's not needed?
ModelObject* new_object = m_model->add_object();
new_object->name = this->name;
new_object->name = this->name + (meshptrs.size() > 1 ? "_" + std::to_string(counter++) : "");

// Don't copy the config's ID.
new_object->config.assign_config(this->config);
assert(new_object->config.id().valid());
Expand Down
4 changes: 0 additions & 4 deletions src/slic3r/GUI/Plater.cpp
Expand Up @@ -2892,10 +2892,6 @@ void Plater::priv::split_object()
{
Plater::TakeSnapshot snapshot(q, _L("Split to Objects"));

unsigned int counter = 1;
for (ModelObject* m : new_objects)
m->name = current_model_object->name + "_" + std::to_string(counter++);

remove(obj_idx);

// load all model objects at once, otherwise the plate would be rearranged after each one
Expand Down

0 comments on commit 22b2ccc

Please sign in to comment.