wo80 / Triangle.NET

C# / .NET version of Jonathan Shewchuk's Triangle mesh generator.
472 stars 85 forks source link

triangle.viewer (solution and examples) #29

Closed adriaanpfeiffer closed 2 years ago

adriaanpfeiffer commented 2 years ago

The triangle.viewer project is in the repository but you removed an accompanying solution in an earlier commit. Did you remove it intentionally because you think it shouldn't be in the repository? I think it can be useful for checking out all the work that's in the triangle.net project. I added the solution to my own branch and reused the examples in the triangle.examples dll to be viewed from the triangle.viewer as I find it easier to work with the examples interactively. Do you think the examples in the triangle.viewer are bloat to the project or does it make sense to you to add them?

wo80 commented 2 years ago

Not too sure about this:

  1. I don't mind putting a solution into the Triangle.Viewer folder, but for me it's fine to just run dotnet build or - if needed - open the csproj file with Visual Studio.
  2. I recently did some changes to the examples in the smoother branch, see IExample.cs and Program.cs. I think I'd like to keep that setup, so your code would have to be adapted.
  3. I'd like to keep the Examples project clean, so instead of introducing code that isn't used there, I'd rather add a callback Action<IMesh> callback to the Run method or return a tuple (bool success, IMesh mesh).

After all, since the examples are in no way interactive and you can get the visual results using the --print command line option, why bother bringing it to the viewer app?

adriaanpfeiffer commented 2 years ago

Not too sure about this:

Okay, no problem. Can imagine that you want to keep the project as clean as possible although I can minimize the changes indeed to a Name property and callback on the Run method.

After all, since the examples are in no way interactive and you can get the visual results using the --print command line option, why bother bringing it to the viewer app?

I like the examples in the viewer because I think i can switch more easily between examples and try stuff out instead of opening up a picture. I keep the changes in my own feature branch and I can easily merge future changes to that one.

wo80 commented 2 years ago

If you think implementing the suggestions in 3) would simplify things for you, that could be easily added. I'd cherry pick the changes I made in the smoother branch and probably go for the "return a tuple" option.

adriaanpfeiffer commented 2 years ago

The reason I didn't return the mesh is that there are some examples that show multiple meshes that illustrate some solution. Such as example 6, 10 or the parallel example. The callback would facilitate to return multiple meshes to be viewed. Otherwise the return type of the Run method would need to be an IEnumerable I guess.

wo80 commented 2 years ago

The reason I didn't return the mesh is that there are some examples that show multiple meshes that illustrate some solution. Such as example 6, 10 or the parallel example.

Another reason why those are not suitable to run in the viewer app.

The callback would facilitate to return multiple meshes to be viewed. Otherwise the return type of the Run method would need to be an IEnumerable I guess.

Either way you'd have to display multiple meshes in the GUI. I looked at your latest commits and I think we agree that using Thread.Sleep(...) to slow down the successive callbacks is far from optimal.

adriaanpfeiffer commented 2 years ago

I looked at your latest commits and I think we agree that using Thread.Sleep(...) to slow down the successive callbacks is far from optimal.

yes, consider it a temporary workaround. I considered splitting up examples and returning lists of meshes. But thought I first discuss the possibility with you to view those meshes in the viewer app. Then I could decide to throw away the branch or maybe improve a bit upon it. For me the most optimal solution would be to create a return method IEnumerable with all the needed information in the ExampleResult.

public class ExampleResult
{
    public bool Success { get; set; } // whether the meshing succeeded or not
    public IMesh Mesh { get; set; }
    public string Message { get; set; } //to get some specific information for each mesh
}
wo80 commented 2 years ago

I've given it some thought and decided not to change the examples in that way. They should be self-contained, self-explaining and as simple as possible to achieve what they do.

If you really want to display more than one resulting mesh, using the callback would be the preferred way. Let me know, whether I should add a Action<IMesh> parameter to the Run method. Otherwise, I'd suggest to just copy the examples into the viewer project and modify by your like.

adriaanpfeiffer commented 2 years ago

I've given it some thought and decided not to change the examples in that way. They should be self-contained, self-explaining and as simple as possible to achieve what they do.

Yes, that is the trade-off. So can imagine that you like to keep them the way that they are.

If you really want to display more than one resulting mesh, using the callback would be the preferred way. Let me know, whether I should add a Action parameter to the Run method. Otherwise, I'd suggest to just copy the examples into the viewer project and modify by your like.

Well, let's keep the code simple for now. Extra parameters add clutter also if they are not used. So let's close this question for now. We can always come back on it.