mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-07-31 14:52:06 +00:00
fix: prevent out-of-bounds crash in Arachne beading interpolation (#14656)
* fix: prevent out-of-bounds crash in Arachne beading interpolation SkeletalTrapezoidation::interpolate() derives an inset index from `left` but uses it to index the merged beading, which follows the thicker of left/right. When the thicker side has fewer insets, the index runs past the end and the slicer crashes during "Generating walls". Skip the adjustment when the index is out of range, as the adjacent guards already do. interpolate() uses no instance state, so make it static and add a regression test that exercises it directly. Fixes #14584
This commit is contained in:
@@ -1660,7 +1660,7 @@ void SkeletalTrapezoidation::propagateBeadingsDownward(edge_t* edge_to_peak, ptr
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
SkeletalTrapezoidation::Beading SkeletalTrapezoidation::interpolate(const Beading& left, double ratio_left_to_whole, const Beading& right, coord_t switching_radius) const
|
SkeletalTrapezoidation::Beading SkeletalTrapezoidation::interpolate(const Beading& left, double ratio_left_to_whole, const Beading& right, coord_t switching_radius)
|
||||||
{
|
{
|
||||||
assert(ratio_left_to_whole >= 0.0 && ratio_left_to_whole <= 1.0);
|
assert(ratio_left_to_whole >= 0.0 && ratio_left_to_whole <= 1.0);
|
||||||
Beading ret = interpolate(left, ratio_left_to_whole, right);
|
Beading ret = interpolate(left, ratio_left_to_whole, right);
|
||||||
@@ -1684,6 +1684,12 @@ SkeletalTrapezoidation::Beading SkeletalTrapezoidation::interpolate(const Beadin
|
|||||||
{ // We cant adjust to fit the next edge because there is no previous one?!
|
{ // We cant adjust to fit the next edge because there is no previous one?!
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
// ret follows the thicker of left/right, which can hold fewer insets than left when bead
|
||||||
|
// count and thickness disagree; skip the adjustment rather than index ret past its end.
|
||||||
|
if (next_inset_idx >= coord_t(ret.toolpath_locations.size()))
|
||||||
|
{
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
assert(next_inset_idx < coord_t(left.toolpath_locations.size()));
|
assert(next_inset_idx < coord_t(left.toolpath_locations.size()));
|
||||||
assert(left.toolpath_locations[next_inset_idx] <= switching_radius);
|
assert(left.toolpath_locations[next_inset_idx] <= switching_radius);
|
||||||
assert(left.toolpath_locations[next_inset_idx + 1] >= switching_radius);
|
assert(left.toolpath_locations[next_inset_idx + 1] >= switching_radius);
|
||||||
@@ -1703,7 +1709,7 @@ SkeletalTrapezoidation::Beading SkeletalTrapezoidation::interpolate(const Beadin
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
SkeletalTrapezoidation::Beading SkeletalTrapezoidation::interpolate(const Beading& left, double ratio_left_to_whole, const Beading& right) const
|
SkeletalTrapezoidation::Beading SkeletalTrapezoidation::interpolate(const Beading& left, double ratio_left_to_whole, const Beading& right)
|
||||||
{
|
{
|
||||||
assert(ratio_left_to_whole >= 0.0 && ratio_left_to_whole <= 1.0);
|
assert(ratio_left_to_whole >= 0.0 && ratio_left_to_whole <= 1.0);
|
||||||
float ratio_right_to_whole = 1.0 - ratio_left_to_whole;
|
float ratio_right_to_whole = 1.0 - ratio_left_to_whole;
|
||||||
|
|||||||
@@ -488,7 +488,7 @@ protected:
|
|||||||
* beads.
|
* beads.
|
||||||
* \return The beading at the interpolated location.
|
* \return The beading at the interpolated location.
|
||||||
*/
|
*/
|
||||||
Beading interpolate(const Beading& left, double ratio_left_to_whole, const Beading& right, coord_t switching_radius) const;
|
static Beading interpolate(const Beading& left, double ratio_left_to_whole, const Beading& right, coord_t switching_radius);
|
||||||
|
|
||||||
/*!
|
/*!
|
||||||
* Subroutine of \ref interpolate(const Beading&, Ratio, const Beading&, coord_t)
|
* Subroutine of \ref interpolate(const Beading&, Ratio, const Beading&, coord_t)
|
||||||
@@ -501,7 +501,7 @@ protected:
|
|||||||
* \param right One of the beadings to interpolate between.
|
* \param right One of the beadings to interpolate between.
|
||||||
* \return The beading at the interpolated location.
|
* \return The beading at the interpolated location.
|
||||||
*/
|
*/
|
||||||
Beading interpolate(const Beading& left, double ratio_left_to_whole, const Beading& right) const;
|
static Beading interpolate(const Beading& left, double ratio_left_to_whole, const Beading& right);
|
||||||
|
|
||||||
/*!
|
/*!
|
||||||
* Get the beading at a certain node of the skeletal graph, or create one if
|
* Get the beading at a certain node of the skeletal graph, or create one if
|
||||||
|
|||||||
@@ -18,6 +18,7 @@
|
|||||||
#include <catch2/catch_all.hpp>
|
#include <catch2/catch_all.hpp>
|
||||||
|
|
||||||
#include "libslic3r/Arachne/WallToolPaths.hpp"
|
#include "libslic3r/Arachne/WallToolPaths.hpp"
|
||||||
|
#include "libslic3r/Arachne/SkeletalTrapezoidation.hpp"
|
||||||
#include "libslic3r/Arachne/utils/ExtrusionLine.hpp"
|
#include "libslic3r/Arachne/utils/ExtrusionLine.hpp"
|
||||||
#include "libslic3r/Arachne/BeadingStrategy/BeadingStrategyFactory.hpp"
|
#include "libslic3r/Arachne/BeadingStrategy/BeadingStrategyFactory.hpp"
|
||||||
#include "libslic3r/Arachne/BeadingStrategy/BeadingStrategy.hpp"
|
#include "libslic3r/Arachne/BeadingStrategy/BeadingStrategy.hpp"
|
||||||
@@ -265,3 +266,46 @@ TEST_CASE("Arachne widening keeps two beads in transition band (#14376)", "[Arac
|
|||||||
for (const coord_t w : beading.bead_widths)
|
for (const coord_t w : beading.bead_widths)
|
||||||
CHECK(w <= inner_width);
|
CHECK(w <= inner_width);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
// Exposes the protected static interpolate() for a focused unit test.
|
||||||
|
struct InterpolateProbe : SkeletalTrapezoidation {
|
||||||
|
using SkeletalTrapezoidation::interpolate;
|
||||||
|
};
|
||||||
|
} // anonymous namespace
|
||||||
|
|
||||||
|
// interpolate() indexes the merged beading with an index derived from `left`. The merged beading
|
||||||
|
// follows the thicker of left/right, so when the thicker side has fewer insets the index runs past
|
||||||
|
// its end.
|
||||||
|
TEST_CASE("Beading interpolation tolerates a thicker side with fewer insets", "[Arachne][Regression]") {
|
||||||
|
using Beading = BeadingStrategy::Beading;
|
||||||
|
|
||||||
|
// Thicker side (right) has fewer insets, so the merged beading holds only 2 toolpath locations.
|
||||||
|
const coord_t w = scaled<coord_t>(0.42);
|
||||||
|
Beading left;
|
||||||
|
left.total_thickness = scaled<coord_t>(1.0);
|
||||||
|
left.bead_widths = { w, w, w, w };
|
||||||
|
left.toolpath_locations = { scaled<coord_t>(0.1), scaled<coord_t>(0.3), scaled<coord_t>(0.5), scaled<coord_t>(0.7) };
|
||||||
|
left.left_over = 0;
|
||||||
|
|
||||||
|
Beading right;
|
||||||
|
right.total_thickness = scaled<coord_t>(2.0);
|
||||||
|
right.bead_widths = { w, w };
|
||||||
|
right.toolpath_locations = { scaled<coord_t>(0.1), scaled<coord_t>(0.3) };
|
||||||
|
right.left_over = 0;
|
||||||
|
|
||||||
|
// Just past left's location [2] (0.5), so the derived index is 2, past the end of the 2-inset merged beading.
|
||||||
|
const coord_t switching_radius = scaled<coord_t>(0.6);
|
||||||
|
|
||||||
|
Beading result;
|
||||||
|
REQUIRE_NOTHROW(result = InterpolateProbe::interpolate(left, 0.5, right, switching_radius));
|
||||||
|
|
||||||
|
// With the guard the adjustment is skipped, so the result is the plain interpolation.
|
||||||
|
const Beading expected = InterpolateProbe::interpolate(left, 0.5, right);
|
||||||
|
REQUIRE(result.toolpath_locations.size() == expected.toolpath_locations.size());
|
||||||
|
REQUIRE(result.bead_widths.size() == expected.bead_widths.size());
|
||||||
|
for (size_t i = 0; i < expected.toolpath_locations.size(); ++i) {
|
||||||
|
CHECK(result.toolpath_locations[i] == expected.toolpath_locations[i]);
|
||||||
|
CHECK(result.bead_widths[i] == expected.bead_widths[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user