Allow compute_intersections supports circular arcs by DusKing1 · Pull Request #185 · fontanf/packingsolver

DusKing1 · 2025-03-31T14:50:54Z

But I'm struggling make proper test code for it.. It would be nice if you can input some help on it.

What are you struggling with? Here is another test using circular arcs and line segments that you can start from:

packingsolver/test/irregular/shape_test.cpp

Lines 64 to 131 in 9bba8e6

struct ApproximateCircularArcByLineSegmentsTestParams

{

ShapeElement circular_arc;

ElementPos number_of_line_segments;

bool outer;

std::vector<ShapeElement> expected_line_segments;

};

class IrregularApproximateCircularArcByLineSegmentsTest: public testing::TestWithParam<ApproximateCircularArcByLineSegmentsTestParams> { };

TEST_P(IrregularApproximateCircularArcByLineSegmentsTest, ApproximateCircularArcByLineSegments)

{

ApproximateCircularArcByLineSegmentsTestParams test_params = GetParam();

std::cout << "circular_arc" << std::endl;

std::cout << test_params.circular_arc.to_string() << std::endl;

std::cout << "expected_line_segments" << std::endl;

for (const ShapeElement& line_segment: test_params.expected_line_segments)

std::cout << line_segment.to_string() << std::endl;

std::vector<ShapeElement> line_segments = approximate_circular_arc_by_line_segments(

test_params.circular_arc,

test_params.number_of_line_segments,

test_params.outer);

std::cout << "line_segments" << std::endl;

for (const ShapeElement& line_segment: line_segments)

std::cout << line_segment.to_string() << std::endl;

ASSERT_EQ(line_segments.size(), test_params.number_of_line_segments);

for (ElementPos pos = 0; pos < test_params.number_of_line_segments; ++pos) {

//std::cout << std::setprecision (15) << line_segments[pos].start.x << std::endl;

EXPECT_TRUE(equal(line_segments[pos], test_params.expected_line_segments[pos]));

}

}

INSTANTIATE_TEST_SUITE_P(

Irregular,

IrregularApproximateCircularArcByLineSegmentsTest,

testing::ValuesIn(std::vector<ApproximateCircularArcByLineSegmentsTestParams>{

{

build_shape({{1, 0}, {0, 0, 1}, {0, 1}}, true).elements.front(),

1,

false,

build_shape({{1, 0}, {0, 1}}, true).elements

}, {

build_shape({{1, 0}, {1, 1, -1}, {0, 1}}, true).elements.front(),

1,

true,

build_shape({{1, 0}, {0, 1}}, true).elements

}, {

build_shape({{1, 0}, {0, 0, 1}, {0, 1}}, true).elements.front(),

2,

true,

build_shape({{1, 0}, {1, 1}, {0, 1}}, true).elements

}, {

build_shape({{1, 0}, {1, 1, -1}, {0, 1}}, true).elements.front(),

2,

false,

build_shape({{1, 0}, {0, 0}, {0, 1}}, true).elements

}, {

build_shape({{1, 0}, {0, 0, 1}, {0, 1}}, true).elements.front(),

3,

true,

build_shape({{1, 0}, {1, 0.414213562373095}, {0.414213562373095, 1}, {0, 1}}, true).elements

}, {

build_shape({{1, 0}, {1, 1, -1}, {0, 1}}, true).elements.front(),

3,

false,

build_shape({{1, 0}, {0.585786437626905, 0}, {0, 0.585786437626905}, {0, 1}}, true).elements

}}));

I'm not familiar with the test framework. I have no idea how to add proper tests for this. I can show you my attempts with no luck....

fontanf · 2025-03-31T14:55:50Z

Could you allow me to commit on your branch?

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

DusKing1 · 2025-03-31T14:59:48Z

Could you allow me to commit on your branch?

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

OMG! That's the gate? I thought I turned it on as default. Lemme check that!

DusKing1 · 2025-03-31T15:00:43Z

I think I've turned that on, please try to commit to this branch.

fontanf · 2025-03-31T17:34:20Z

I've added some tests. Is it more clear now for you how to add some more?

To run the tests, go into the build/test directory and run:

ctest --verbose -R "IrregularComputeIntersections"

EDIT: I added some failing tests as well. You implementation seems to only consider "strict" intersections. For the shape self-intersection removal, it needs to also add contact points. I added some tests with strict = true and strict = false. Several ones with strict = false are failing.

fontanf · 2025-04-01T18:44:12Z

I did some refactoring and included your functions to determine if a point is on a line and to determine if a point is inside a shape. I hope it's ok for you, let me know otherwise. I added some tests as well.

There are still some failing tests for the compute_intersections. Once they are fixed, the pull request should be ready to merge.

DusKing1 · 2025-04-02T01:11:03Z

Wow really appreciate your effort on this, last night I've been busy on other higher priority things and didn't have chance to code. I'll check it today if i have spare time for this.

DusKing1 · 2025-04-02T12:30:00Z

About the test 67, I saw the failure cause is:

67: element_1 CircularArc start (1.000000, 0.000000) end (-1.000000, 0.000000) center (0.000000, 0.000000) anticlockwise
67: element_2 LineSegment start (-1.000000, 1.000000) end (1.000000, 1.000000)
67: strict 1
67: expected_intersections
67: intersections
67: - 0 1

But I think the expected intersection should be just (0,1), right?

And for the test 58, I saw:

element_1 LineSegment start (0.000000, 0.000000) end (0.000000, 2.000000)
element_2 LineSegment start (2.000000, 0.000000) end (0.000000, 0.000000)
strict 0
expected_intersections
- (0.000000, 0.000000)
- (0.000000, 2.000000)
intersections
- 0 0

So I guess you mis-typed the element_2 or the expected_intersections....

DusKing1 · 2025-04-02T12:33:18Z

I've fixed other failed tests, the 58 and 67 are the only failed tests for ctest -R "rregularComputeIntersectionsTest" -V now:

93% tests passed, 2 tests failed out of 30

Total Test time (real) =   0.26 sec

The following tests FAILED:
         58 - Irregular/IrregularComputeIntersectionsTest.IrregularComputeIntersections/160-byte object <00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-40 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 01-00 00-00 00-00 00-00 ... 00-00 00-00 00-00 00-00 00-00 00-00 0... (Failed)
         67 - Irregular/IrregularComputeIntersectionsTest.IrregularComputeIntersections/160-byte object <01-00 00-00 00-00 00-00 00-00 00-00 00-00 F0-3F 00-00 00-00 00-00 00-00 00-00 00-00 00-00 F0-BF 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 01-00 00-00 00-00 00-00 ... 00-00 00-00 00-00 F0-3F 00-00 00-00 0... (Failed)

fontanf · 2025-04-02T12:59:52Z

About the test 67, I saw the failure cause is:

67: element_1 CircularArc start (1.000000, 0.000000) end (-1.000000, 0.000000) center (0.000000, 0.000000) anticlockwise
67: element_2 LineSegment start (-1.000000, 1.000000) end (1.000000, 1.000000)
67: strict 1
67: expected_intersections
67: intersections
67: - 0 1

But I think the expected intersection should be just (0,1), right?

When strict == 1, contact points should not be returned. So the test looks correct.

And for the test 58, I saw:

element_1 LineSegment start (0.000000, 0.000000) end (0.000000, 2.000000)
element_2 LineSegment start (2.000000, 0.000000) end (0.000000, 0.000000)
strict 0
expected_intersections
- (0.000000, 0.000000)
- (0.000000, 2.000000)
intersections
- 0 0

So I guess you mis-typed the element_2 or the expected_intersections....

That's right, element_2 should start with (0, 2)

fontanf · 2025-04-02T14:06:29Z

For test 67, you changed the expected output, but the previous expected output (no intersection) was the right one.
The circular arc and the line segment are in contact at (0, 1), but they do not strictly intersect

DusKing1 · 2025-04-02T14:08:13Z

For test 67, you changed the expected output, but the previous expected output (no intersection) was the right one. The circular arc and the line segment are in contact at (0, 1), but they do not strictly intersect

oh, ok, I mis-read your reply.. lemme check that one.

And I also found another failed test:

The following tests FAILED:
        104 - Irregular/AlgorithmTest.Algorithm/"data\irregular\users\2024-08-13.json" (Failed)

I think this one is something I can't figure out why easily

fontanf · 2025-04-02T14:16:02Z

And I also found another failed test:
The following tests FAILED:
        104 - Irregular/AlgorithmTest.Algorithm/"data\irregular\users\2024-08-13.json" (Failed)
I think this one is something I can't figure out why easily

That's strange. The changes in this pull request shouldn't change the optimization. I'll take care of fixing it.

fontanf · 2025-04-02T16:46:46Z

Thank you for the update.

I removed the code you added in the compute_splitted_elements. It's not in the scope of this pull request, and I think it is not necessary anymore: with the way the compute_intersections works now, I expect the remove_self_intersections to work out-of-the-box with circular arcs. Let me know if you find counter-examples.

I launched the pipelines. If they pass, I'll merge the pull request

fontanf · 2025-04-02T18:42:35Z

Everything's good. Thank you for this contribution.

DusKing1 · 2025-04-03T00:50:59Z

Thanks for your effort on helping me and this PR. I'll pick #169 later.

allow compute_intersections supports circular arcs

8706886

fontanf added 2 commits March 31, 2025 19:14

Move compute_intersections into own file

37ebfdc

Add some tests

0a0e2a4

fontanf added 9 commits March 31, 2025 19:52

Add 'strict' parameter and additional tests

059d3a9

Implement strict for line segments and add tests

c507099

Add some failing tests

ed6fa4b

Add more tests

17bb72a

Add more tests

7caf31f

Move is_point_on_arc and is_point_on_line to ShapeElement::contains

14d9630

Clean tests

0725e34

Move ShapeElement::contains

8a4a2b5

Move is_point_strictly_inside_shape to Shape::contains

2cea7b1

DusKing1 added 3 commits April 2, 2025 20:10

handle the arc overlap properly

470eebf

remove duplicated intersection points from the result

509466a

not to return start/end point in strict mode

dfdc71b

fixed the wrong tests

4a0a448

fixed test 67

aebb4da

Fix remaining failing test and clean code

5ba8317

fontanf merged commit efc2bd7 into fontanf:master Apr 2, 2025
4 checks passed

DusKing1 deleted the handle-circular-arcs branch April 4, 2025 15:13

	struct ApproximateCircularArcByLineSegmentsTestParams
	{
	ShapeElement circular_arc;
	ElementPos number_of_line_segments;
	bool outer;
	std::vector<ShapeElement> expected_line_segments;
	};

	class IrregularApproximateCircularArcByLineSegmentsTest: public testing::TestWithParam<ApproximateCircularArcByLineSegmentsTestParams> { };

	TEST_P(IrregularApproximateCircularArcByLineSegmentsTest, ApproximateCircularArcByLineSegments)
	{
	ApproximateCircularArcByLineSegmentsTestParams test_params = GetParam();
	std::cout << "circular_arc" << std::endl;
	std::cout << test_params.circular_arc.to_string() << std::endl;
	std::cout << "expected_line_segments" << std::endl;
	for (const ShapeElement& line_segment: test_params.expected_line_segments)
	std::cout << line_segment.to_string() << std::endl;
	std::vector<ShapeElement> line_segments = approximate_circular_arc_by_line_segments(
	test_params.circular_arc,
	test_params.number_of_line_segments,
	test_params.outer);
	std::cout << "line_segments" << std::endl;
	for (const ShapeElement& line_segment: line_segments)
	std::cout << line_segment.to_string() << std::endl;

	ASSERT_EQ(line_segments.size(), test_params.number_of_line_segments);
	for (ElementPos pos = 0; pos < test_params.number_of_line_segments; ++pos) {
	//std::cout << std::setprecision (15) << line_segments[pos].start.x << std::endl;
	EXPECT_TRUE(equal(line_segments[pos], test_params.expected_line_segments[pos]));
	}
	}

	INSTANTIATE_TEST_SUITE_P(
	Irregular,
	IrregularApproximateCircularArcByLineSegmentsTest,
	testing::ValuesIn(std::vector<ApproximateCircularArcByLineSegmentsTestParams>{
	{
	build_shape({{1, 0}, {0, 0, 1}, {0, 1}}, true).elements.front(),
	1,
	false,
	build_shape({{1, 0}, {0, 1}}, true).elements
	}, {
	build_shape({{1, 0}, {1, 1, -1}, {0, 1}}, true).elements.front(),
	1,
	true,
	build_shape({{1, 0}, {0, 1}}, true).elements
	}, {
	build_shape({{1, 0}, {0, 0, 1}, {0, 1}}, true).elements.front(),
	2,
	true,
	build_shape({{1, 0}, {1, 1}, {0, 1}}, true).elements
	}, {
	build_shape({{1, 0}, {1, 1, -1}, {0, 1}}, true).elements.front(),
	2,
	false,
	build_shape({{1, 0}, {0, 0}, {0, 1}}, true).elements
	}, {
	build_shape({{1, 0}, {0, 0, 1}, {0, 1}}, true).elements.front(),
	3,
	true,
	build_shape({{1, 0}, {1, 0.414213562373095}, {0.414213562373095, 1}, {0, 1}}, true).elements
	}, {
	build_shape({{1, 0}, {1, 1, -1}, {0, 1}}, true).elements.front(),
	3,
	false,
	build_shape({{1, 0}, {0.585786437626905, 0}, {0, 0.585786437626905}, {0, 1}}, true).elements
	}}));

Conversation

DusKing1 commented Mar 31, 2025

Uh oh!

fontanf commented Mar 31, 2025

Uh oh!

DusKing1 commented Mar 31, 2025

Uh oh!

DusKing1 commented Mar 31, 2025

Uh oh!

fontanf commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fontanf commented Apr 1, 2025

Uh oh!

DusKing1 commented Apr 2, 2025

Uh oh!

DusKing1 commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DusKing1 commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fontanf commented Apr 2, 2025

Uh oh!

fontanf commented Apr 2, 2025

Uh oh!

DusKing1 commented Apr 2, 2025

Uh oh!

fontanf commented Apr 2, 2025

Uh oh!

fontanf commented Apr 2, 2025

Uh oh!

Uh oh!

fontanf commented Apr 2, 2025

Uh oh!

DusKing1 commented Apr 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fontanf commented Mar 31, 2025 •

edited

Loading

DusKing1 commented Apr 2, 2025 •

edited

Loading

DusKing1 commented Apr 2, 2025 •

edited

Loading