Fix polygon connector #843

BagelOrb · 2018-09-09T14:46:58Z

I've fixed some problems with the PolygonConnector and improved performance.

The performance improvement should mostly come from the fact that we now use a sparse grid to efficiently lookup where polygons are close enough to each other to be connected.

I haven't done any performance tests to see whether performance is actually increased, but the tests below certainly show that some bugs are fixed and that the code is improved a lot!

Before:
(line dist 0.76)
random connection: (happened on roughly 2% of the layers)

no connection: (happened on roughly 50% of the layers, now on roughly 5%)

After:

Project:
lil_L_x2__cross_3d.curaproject.3mf.remove_extension.zip

Ghostkeeper · 2018-09-10T06:35:21Z

Developers, see issue CURA-5705.

BagelOrb · 2018-09-10T08:44:54Z

I need to retract some claims.

I've done some tests and it turns out the new code is quite a bit slower.

Export stage processing time:
master: 5.07s, PR: 5.93s

Total fraction of polygons connected:
master: 62.5%, PR: 82.3%
Average fraction of polygons connected:
master: 65.0%, PR: 85.5%

So the core functionality of the PolygonConnector class is only enhanced marginally, at the cost of quite some computation time.

Still the PR solves the problem of random connections which can even introduce infill lines outside of the object.

I don't know yet which of my commits fixed that bug, but if you think the accuracy increase of 15% isn't worth the extra processing time of 10% then I can see if I can isolate the bug fix.

Project file for testing: (Nobody thinks that boaty-mac-bench-face is a good benchmark, right?)
Debailey_x20.curaproject.3mf.remove_extension.zip

Project files seem to be broken on master?!
In order to test it I do:

choose default TPU for UM3 profile
set the Extra Infill Wall Count to 1
set the line distance to 2.51

BagelOrb · 2018-09-10T08:48:12Z

I am getting the same results in terms of accuracy when I test the lil L project file shared above:

master: 65%, PR: 85%

BagelOrb · 2018-09-13T10:38:09Z

Okay I decided I need to really fix the implementation, because these fail rates are just too damn high!

Now I am satisfied with the fail rates:
master: 35%, PR: 0.5% !!!

alekseisasin · 2018-09-27T11:59:35Z

Can you fix the branch conflicts?

BagelOrb · 2018-09-27T12:07:43Z

The conflicts were auto-resolved.

BagelOrb · 2018-09-27T12:26:09Z

Why is Jenkins crying?

alekseisasin · 2018-09-27T15:31:15Z

[ 81%] Building CXX object CMakeFiles/ArcusCommunicationTest.dir/tests/arcus/MockSocket.cpp.obj

In file included from C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.cpp:4:0:

C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:23:10: error: 'void cura::MockSocket::connect(const string&, int)' marked 'override', but does not override

 void connect(const std::string& address, int port) override;

      ^

C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:24:10: error: 'void cura::MockSocket::listen(const string&, int)' marked 'override', but does not override

 void listen(const std::string& address, int port) override;

      ^

C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:25:10: error: 'void cura::MockSocket::close()' marked 'override', but does not override

 void close() override;

      ^

C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:26:10: error: 'void cura::MockSocket::reset()' marked 'override', but does not override

 void reset() override;

      ^

C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:29:10: error: 'void cura::MockSocket::sendMessage(Arcus::MessagePtr)' marked 'override', but does not override

 void sendMessage(Arcus::MessagePtr message) override;

      ^

C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:30:23: error: 'Arcus::MessagePtr cura::MockSocket::takeNextMessage()' marked 'override', but does not override

 Arcus::MessagePtr takeNextMessage() override;

                   ^

CMakeFiles\ArcusCommunicationTest.dir\build.make:87: recipe for target 'CMakeFiles/ArcusCommunicationTest.dir/tests/arcus/MockSocket.cpp.obj' failed

mingw32-make[2]: *** [CMakeFiles/ArcusCommunicationTest.dir/tests/arcus/MockSocket.cpp.obj] Error 1

CMakeFiles\Makefile2:331: recipe for target 'CMakeFiles/ArcusCommunicationTest.dir/all' failed

mingw32-make[1]: *** [CMakeFiles/ArcusCommunicationTest.dir/all] Error 2

Makefile:161: recipe for target 'all' failed

mingw32-make: *** [all] Error 2

script returned exit code 2

BagelOrb · 2018-09-27T16:37:47Z

That's unrelated to me PR

…

On Thu, 27 Sep 2018, 17:31 alekseisasin, ***@***.***> wrote: [ 81%] Building CXX object CMakeFiles/ArcusCommunicationTest.dir/tests/arcus/MockSocket.cpp.obj In file included from C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.cpp:4:0: C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:23:10: error: 'void cura::MockSocket::connect(const string&, int)' marked 'override', but does not override void connect(const std::string& address, int port) override; ^ C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:24:10: error: 'void cura::MockSocket::listen(const string&, int)' marked 'override', but does not override void listen(const std::string& address, int port) override; ^ C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:25:10: error: 'void cura::MockSocket::close()' marked 'override', but does not override void close() override; ^ C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:26:10: error: 'void cura::MockSocket::reset()' marked 'override', but does not override void reset() override; ^ C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:29:10: error: 'void cura::MockSocket::sendMessage(Arcus::MessagePtr)' marked 'override', but does not override void sendMessage(Arcus::MessagePtr message) override; ^ C:....\ngine_fix_polygon_connector-3V6HUP43CQSHTUOXHZEOXJ7DNWOI4QXO74SGWJXTH7SMVB3F7WOA\tests\arcus\MockSocket.h:30:23: error: 'Arcus::MessagePtr cura::MockSocket::takeNextMessage()' marked 'override', but does not override Arcus::MessagePtr takeNextMessage() override; ^ CMakeFiles\ArcusCommunicationTest.dir\build.make:87: recipe for target 'CMakeFiles/ArcusCommunicationTest.dir/tests/arcus/MockSocket.cpp.obj' failed mingw32-make[2]: *** [CMakeFiles/ArcusCommunicationTest.dir/tests/arcus/MockSocket.cpp.obj] Error 1 CMakeFiles\Makefile2:331: recipe for target 'CMakeFiles/ArcusCommunicationTest.dir/all' failed mingw32-make[1]: *** [CMakeFiles/ArcusCommunicationTest.dir/all] Error 2 Makefile:161: recipe for target 'all' failed mingw32-make: *** [all] Error 2 script returned exit code 2 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#843 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIe9EZ75i-ewIvypHT8BAkdn978EH8D3ks5ufO9SgaJpZM4WgT-L> .

alekseisasin · 2018-09-28T11:19:55Z

Right, we need to update the libarcus on the server.

…r any std::pair

…re reliably when the polygons involved have complex shapes

… poly_idx of the result any more.

…d continue

… line pairs if a good candidate has been found It used to first put all candidate connecting pairs of line segments in a set first and then process all items in the set until a good one is found. Now it puts them in the set within the same loop as it processes the items.

…ygons

…bridge.

…idth, so don't provide that as a parameter

Contributes to issue CURA-5826.

src/utils/PolygonConnector.cpp

src/utils/PolygonConnector.h

src/utils/SparseGrid.h

it wasn't used

Ghostkeeper · 2018-10-25T13:18:00Z

I don't see a definite improvement in how often the cross infill polygons can be connected. It still seems to fail to connect quite often. This is a screenshot of one of the worst layers:

But since that was already the case, I think we'll have to accept that as a limitation of our current algorithm. What does appear to be fixed is the much more pressing issue of polygons connecting across the infill randomly. I am not able to reproduce those any more. Good job.

BagelOrb · 2018-10-25T15:09:40Z

I don't understand what connections you were expecting in that specific case. It looks to me like no connections are possible on that layer. If you use infill walls then the infill can connect to those. Everything is then connected to each other via the infill walls.

…

On Thu, 25 Oct 2018, 15:19 Ghostkeeper, ***@***.***> wrote: Merged #843 <#843> into master. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#843 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIe9EebDWyF4l-SKXoqqytwcH4FDt2gPks5uobpygaJpZM4WgT-L> .

BagelOrb force-pushed the fix_polygon_connector branch from e00a911 to e5b290b Compare September 13, 2018 13:27

BagelOrb force-pushed the fix_polygon_connector branch from e5b290b to 468bb8a Compare September 27, 2018 12:07

Vandrasc mentioned this pull request Oct 5, 2018

[3.5] Messy infill lines Ultimaker/Cura#4483

Closed

BagelOrb mentioned this pull request Oct 11, 2018

[master] - odd skin lines appear when LayerPlan::optimizePaths() is called Ultimaker/Cura#4438

Closed

BagelOrb added 16 commits October 11, 2018 13:49

lil auxiliary AABB function

78045a4

auxilirary LinearAlg2D line segment functions

e127172

auxiliary for SparseGrid: processAll

eb56a9b

auxilirary: hash function for ClosestPolygonPoint and has function fo…

57c7ca1

…r any std::pair

lil fix: early return in PolygonConnector

30fd6a7

fix: PolygonUtils::findSmallestConnection now finds the connection mo…

b9ccff2

…re reliably when the polygons involved have complex shapes

fix: PolygonUtils::walkToNearestSmallestConnection doesn't invalidate…

1dc8900

… poly_idx of the result any more.

big fix: PolygonConnector: find connection using sparse grid

d524546

fix: PolygonConnector favored too large connections

78898f0

lil aux: let all SparseGrid functions return whether processing shoul…

8d29390

…d continue

fix: don't settle easily when trying to connect to small polygon

c5a8e7a

fix: remove too small polygons from infill when connecting infill pol…

1a9457e

…ygons

big fix: PolygonConnector: only return a connection if it can make a …

e0deecc

…bridge.

cleanup: PolygonConnector::getSecondConnection now always uses line_w…

1a80a8e

…idth, so don't provide that as a parameter

fix test cases

3ef1374

BagelOrb force-pushed the fix_polygon_connector branch from ff6bccd to 3ef1374 Compare October 11, 2018 11:49

P3D-1 mentioned this pull request Oct 15, 2018

[3.5.1] [3.5.0] Cura does random connections from supports - support material intersects model Ultimaker/Cura#4575

Closed

Fix spelling

42fc49d

Contributes to issue CURA-5826.

Ghostkeeper suggested changes Oct 16, 2018

View reviewed changes

src/utils/PolygonConnector.cpp Show resolved Hide resolved

src/utils/PolygonConnector.cpp Outdated Show resolved Hide resolved

src/utils/PolygonConnector.h Outdated Show resolved Hide resolved

src/utils/SparseGrid.h Outdated Show resolved Hide resolved

BagelOrb added 3 commits October 16, 2018 17:44

clarify magic numbers

bf5b37e

refactor: remove dangerous constructor which leaves uninitialized

b25bf5a

remove SparseGrid::procesAll function

b3f200e

it wasn't used

Ghostkeeper merged commit b3f200e into master Oct 25, 2018

Ghostkeeper deleted the fix_polygon_connector branch October 25, 2018 13:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix polygon connector #843

Fix polygon connector #843

BagelOrb commented Sep 9, 2018 •

edited

Loading

Ghostkeeper commented Sep 10, 2018

BagelOrb commented Sep 10, 2018 •

edited

Loading

BagelOrb commented Sep 10, 2018 •

edited

Loading

BagelOrb commented Sep 13, 2018

alekseisasin commented Sep 27, 2018

BagelOrb commented Sep 27, 2018

BagelOrb commented Sep 27, 2018

alekseisasin commented Sep 27, 2018

BagelOrb commented Sep 27, 2018 via email

alekseisasin commented Sep 28, 2018

Ghostkeeper commented Oct 25, 2018

BagelOrb commented Oct 25, 2018 via email

Fix polygon connector #843

Fix polygon connector #843

Conversation

BagelOrb commented Sep 9, 2018 • edited Loading

Ghostkeeper commented Sep 10, 2018

BagelOrb commented Sep 10, 2018 • edited Loading

BagelOrb commented Sep 10, 2018 • edited Loading

BagelOrb commented Sep 13, 2018

alekseisasin commented Sep 27, 2018

BagelOrb commented Sep 27, 2018

BagelOrb commented Sep 27, 2018

alekseisasin commented Sep 27, 2018

BagelOrb commented Sep 27, 2018 via email

alekseisasin commented Sep 28, 2018

Ghostkeeper commented Oct 25, 2018

BagelOrb commented Oct 25, 2018 via email

BagelOrb commented Sep 9, 2018 •

edited

Loading

BagelOrb commented Sep 10, 2018 •

edited

Loading

BagelOrb commented Sep 10, 2018 •

edited

Loading