Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AdjacentTriangles not cleared when the graph is recreated #2

Open
Roland09 opened this issue Dec 27, 2019 · 1 comment
Open

AdjacentTriangles not cleared when the graph is recreated #2

Roland09 opened this issue Dec 27, 2019 · 1 comment

Comments

@Roland09
Copy link

Very awesome implementation, thank you very much for giving it the MIT license.

I made your repository interactive, i. e. allow the user to add points via mouse click. I noticed that you have to clear the AdjacentTriangles map in the Points class

public HashSet<Triangle> AdjacentTriangles { get; } = new HashSet<Triangle>();

whenver you recreate the graph.

Reproduction:

Make

var points = delaunay.GeneratePoints(5000, 800, 400);

a class member.

Then invoke

var delaunayTimer = Stopwatch.StartNew();
var triangulation = delaunay.BowyerWatson(points);
delaunayTimer.Stop();
DrawTriangulation(triangulation);
var voronoiTimer = Stopwatch.StartNew();
var vornoiEdges = voronoi.GenerateEdgesFromDelaunay(triangulation);
voronoiTimer.Stop();
DrawVoronoi(vornoiEdges);
DrawPoints(points);

then add a new point to the points list and re-invoke the graph creation again.

AdjacentTriangles just grows and grows and the graph will get unconnected (old) lines when it is repainted.

Just fyi in case you want to address this.

@RafaelKuebler
Copy link
Owner

Hi @Roland09,
thanks for the interest in the project! I am happy to hear the implementation has been useful for you. I really like the work you have done on your fork.

The issue you mention is indeed unexpected behavior and undesired. However, it stems from a deeper problem which is the storing of the AdjacentTriangles map in the Point class. This was meant as an optimization, but it pollutes what should be a POD type. As making the diagram interactive was not intended in the beginning, the underlying (though undocumented) assumption was that the point list is re-generated before every run and therefore 'clean'. Clearing all AdjacentTriangles at the beginning of each run would introduce some overhead, which in this non-interactive version is overkill?
I am well aware of this drawback but somewhat reluctant to introduce an additional loop over all points (in this specific implementation). Nevertheless, I am very open to suggestions and would appreciate further input, as I agree from an engineering perspective it is kinda broken.

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants