blitzarx1 / egui_graphs

Interactive graph visualization widget for rust powered by egui and petgraph
https://docs.rs/crate/egui_graphs
MIT License
389 stars 29 forks source link

export EdgeShapeBuilder and TipProps #204

Closed nacl42 closed 1 week ago

nacl42 commented 3 weeks ago

fixes #203

blitzarx1 commented 1 week ago

As I said in the #203 I do not see the reason to export these structs.

nacl42 commented 1 week ago

I am not sure if I describe my problem properly, so I will try again.

The situation is as follows: I am building a graph with a custom implementation of the DisplayEdge trait, i.e. I would like to have my own customized edge drawing. For this, I need to implement the two functions shapes, update and is_inside. In your original source code, i.e. in src/draw/displays_default/edge.rs, the shapes for the DefaultEdgeShape are constructed using an EdgeShapeBuilder. Therefore I tried to duplicate your code as a starting point for my own experimental CustomEdgeShape. But then I hit the restriction that EdgeShapeBuilder is private outside the egui_graphs crate. Similar problem with TipProps.

So from my understanding, your API exposes enough for the user of the library, but not for someone who would like to re-implement the edge drawing or the node drawing.

I hope I was able to convey my problem. If you have another solution or if I have overlooked something obvious, I would be happy about any hint.

blitzarx1 commented 1 week ago

Thanks for the thorough description here :)

Again the reasons you provided to make EdgeShapeBuilder and TipProps structs a part of public API of the lib are not enough.

I can explain why. EdgeShapeBulder and TipProps are part of the internal implementation of the DefaultEdgeShape and therefore should stay inside the lib as the internal stuff. In other words if user wants to create his own implementation of the DisplayEdge trait it should use his own helpers.

I can see the continuation of this discussion in the context of providing more public helpers for common shapes, which are used inside the lib which can become parts of the users implementations of ...Display traits. But this topic is outside of this PR.

I think this is inappropriate request to make publicly available internal helpers just because you chose to make your own implementation by copying the internal implementation of the lib (in this case you could also copy the EdgeShapeBulder and TipProps source code as well).

So from my understanding, your API exposes enough for the user of the library, but not for someone who would like to re-implement the edge drawing or the node drawing.

You could check animated_nodes example to understand how to implement custom node drawing without using internal lib helpers.

But anyway I partly understand your concern and today or tomorrow I will add an example with custom DisplayEdge implementation from which you could pick up some ideas to implement your own from scratch.

blitzarx1 commented 1 week ago

@nacl42 hope this will help https://github.com/blitzarx1/egui_graphs/tree/master/examples/rainbow_edges

nacl42 commented 1 week ago

@blitzarx1 Thank you so much for the rainbow example. It is both illustrative and beautiful. This solves my problem in a much better way!

blitzarx1 commented 1 week ago

Nice! Glad to hear it!