supermerill / SuperSlicer

G-code generator for 3D printers (Prusa, Voron, Creality, etc.)
4.13k stars 520 forks source link

Range printing doesn't work with object instances #4043

Closed vovodroid closed 9 months ago

vovodroid commented 10 months ago

What happened?

PR [Merged] Semi-parallel (range) printing #3445 from very beginning didn't support object instances. I recently fix it for Prusa:

ObjectsLayerToPrint layers_to_print_range;
...
layers_to_print_range.emplace_back(ltp);
...
this->process_layers(print, tool_ordering, layers_to_print_range,
                                *print_object_instance - object.instances().data(),  smooth_path_cache_global, file);

Project file & How to reproduce

Create object instances and slice it with printing range > 0.

Version

No response

Operating system

No response

Printer model

No response

supermerill commented 10 months ago

I guess it's your commit c627b0 I replace your code by this one (with some minor changes). Should be in next nightly.

site note: don't use emplace_back() for anything but when you want to delegate the call of the constructor to the vector. just use push_back when passing an object or moving it. https://quuxplusone.github.io/blog/2021/03/03/push-back-emplace-back/ https://andreasfertig.blog/2023/04/push_back-vs-emplace_back-when-to-use-what/

side note 2: It's not useful to pass an int as const reference, just copy it, it's faster and safer.

supermerill commented 10 months ago

btw, I was also wondering about making another big change: Beeing able to print object islands in sequence, ie instead of printing objects after objects after ordering objects: ordering all object's islands and printing the island after islands. So we can have better sorting, and being able to print in range-sequence these (note: with an area threshold to avoid printing thin stuff that may overheat). More of long-term thinking, i won't do it in the near term.

vovodroid commented 10 months ago

I guess it's your commit c627b0

Yes.

don't use emplace_back() for anything but when you want to delegate the call of the constructor to the vector.

I just see that Prusa uses it like layers_to_print.emplace_back(layer_to_print);, so I did the same to be on the safe side.

It's not useful to pass an int as const reference,

Sure, if you are talking about void GCodeGenerator::_move_to_print_object(GCodeGenerator::GCodeOutputStream& file, const size_t& finished_objects) it was automatic method extraction by Visual Studio))), I never put attention on this, thanks!

supermerill commented 10 months ago

I just see that Prusa uses it like layers_to_print.emplace_back(layer_to_print);, so I did the same to be on the safe side.

I did the same initially, until I learned that it's not only useless but harmful (to compilation time). Also more difficult to debug. Prusa love to put emplace_back everywhere, but i don't know why.

vovodroid commented 10 months ago

i don't know why.

For historical reasons)))

https://intersol.ca/news/organizational-culture-and-the-5-monkeys-experiment/