microsoft / automatic-graph-layout

A set of tools for graph layout and viewing
Other
1.35k stars 302 forks source link

Non-directed or bidirected edges are not rendered properly #303

Open MVoloshin opened 2 years ago

MVoloshin commented 2 years ago

Hello! I'm trying to add a new edge using GViewers's edge insertion tool. The first screenshot shows that happens when Attr->ArrowheadAtSource and Attr->ArrowheadAtTarget are set to ArrowStyle::Normal image And the second one for ArrowStyle::None image

levnach commented 2 years ago

@MVoloshin , can you provide a code snippet where you create an edge and add it to the viewer?

MVoloshin commented 2 years ago

Yes, of course. I'll do that in next few days

MVoloshin commented 2 years ago

@levnach, EdgeDirectionTest.zip

MVoloshin commented 2 years ago

If you build this project, you'll see that:

You can create nodes with right mouse click and connect nodes with edges using the button ("Edge insertion") on GViewer toolbar. P.S. I don't want existing graph elements to be repositioned after adding new nodes or edges, so I have to deal with current layout

MVoloshin commented 2 years ago

Problems with rendering of bidirected and undirected edges still exist, my commit fixed just label dragging (that was another bug)

levnach commented 2 years ago

@MVoloshin , I added your project with some changes to Samples folder in the GraphLayout directory. I made a change where the edge attribute has to be set in advance, before inserting the edge, in the GViewer.LayoutEditor. It is not as flexible as changing it after the edge insertion, but it might be enough for most of the purposes. The first inserted edge in the EdgeRoutingTest will not have arrowheads, and each next inserted edge will have two arrowheads.

MVoloshin commented 2 years ago

@levnach , Hmm... Would it be possible to add the third edge (without arrowheads like first one), for example? P.S. I hope there will be better solution sometime. It's much more obvious to change edge arrow styles by changing corresponding attribute of Edge object (by the same way as we can change edge color by setting it's Color property, even after adding Edge to GViewer)

levnach commented 2 years ago

Of, course it is possible to add the third edge without the arroweheads. Just comment out the lines https://github.com/microsoft/automatic-graph-layout/blob/2c84b4c2a89438f66dd68908841941a186e5be14/GraphLayout/Samples/EdgeDirectionTest/Form1.cs#L62-L66 Here I was just making sure that changing gv.LayoutEditor.EdgeAttribute propagates to geometry and visualization.

MVoloshin commented 2 years ago

@levnach, thank you very much! P.S. I have another question: is it possible to remove IViewerEdge from GViewer without selecting it with mouse? I only have two Node objects and Edge object of edge between them (they are present in the graph). If I try something like graphViewer.RemoveEdge(createGeometryEdge(myEdge), false) I get exception, because Edge object does not have geometry. If I try graphViewer->Graph->RemoveEdge(myEdge), the edge gets detached from vertices but it does not get removed from GViewer anyway. I'd like to know how could I delete such Edge without reloading entire Graph in GViewer,

levnach commented 2 years ago

I modified EdgeDirectionTest to show how to remove an edge. Basically, you need to find an edge you want to delete in IViewer.Entities, and delete it.

MVoloshin commented 2 years ago

@levnach, thank you once again. P.S. I've written an application that allows user to build an undirected graph (using adjacency matrix or GViewer) and then it's possible to run Kruskal algorithm on it (with visualization) CppCLR_WinformsProjekt1.zip

MVoloshin commented 2 years ago

It's in CLI/C++ now, but I think it's trivial to rewrite it in CSharp. That application might beсome a good sample P.S. Left mouse button creates the vertex and the same button on existing vertex removes it. Matrix and GViewer are synchronized

levnach commented 2 years ago

Why don't you create a pull request :-)?

MVoloshin commented 2 years ago

I'm a bit busy with my university tasks for now. But I'm sure I will be able to rewrite this in C# and create a pull request in February

MVoloshin commented 2 years ago

This is still actual. I'm facing difficulties, because undirected edges that are added to graph in "Edge Insertion" mode (with mouse) are still not displayed correctly (in other words, if arrows are set to anything other than default values). They [edges] still sporadically have incomplete lines and sometimes even ignore LayoutEditor->EdgeAttrs at all. And such edges also do not seem to respect graph routing settings, so I have to route them manually inside the code. Other workaround is automatically deleting such edges (on EdgeAdded event) and recreating them with required attributes.

levnach commented 2 years ago

I wonder if it is fixed by https://github.com/microsoft/automatic-graph-layout/commit/da3b5a3cafa10d9271bff1e90a68c913ab084312

MVoloshin commented 2 years ago

@levnach It seems to be partially fixed now. I still have to write "Edge Added" event handler the follwoing way in order to make newly added edge display correctly.

// Событие добавления ребра
    System::Void Form1::graph_viewer_EdgeAdded(System::Object^ sender, System::EventArgs^ e) {
        Edge^ added_edge = (Edge^)sender;
        added_edge->Attr = graph_viewer->LayoutEditor->EdgeAttr->Clone(); // Force Arrows::None
        Microsoft::Msagl::Routing::StraightLineEdges::RouteEdge(added_edge->GeometryEdge, 20); 
        // Edge added with mouse isn't routed straight until we move one of its nodes or call RouteEdge manually
        // (even if we have graph->LayoutAlgorithmSettings->EdgeRoutingSettings->EdgeRoutingMode
        //  = Microsoft::Msagl::Core::Routing::EdgeRoutingMode::StraightLine; already set, it does not matter)
        graph_viewer->Graph->GeometryGraph->UpdateBoundingBox();
        graph_viewer->Refresh();
    }

Is that correct?

levnach commented 2 years ago

I hoped that this event handler would not be needed. Can you give an example where an edge is created incorrectly?

MVoloshin commented 2 years ago

If I attach my entire project here, would it help? Then I will be able to describe my problem exactly

MVoloshin commented 2 years ago

@levnach , https://www.upload.ee/files/14202950/SpanningTreeVis_v2.zip.html P.S. Do you understand Russian? I could do some translation if you wish

MVoloshin commented 2 years ago

If an edge is added to the graph with mouse (and Microsoft::Msagl::Routing::StraightLineEdges::RouteEdge() is not called later in the event handler), the edge wont have a straight line (it will be displayed as a curve despite graph settings) until you move one of its adjacent vertices.

MVoloshin commented 2 years ago

Then if I don't clone EdgeAttrs inside EdgeAdded handler I sometimes get huge black arrows (despite ArrowStyle::None set in LayoutEditor). In other words, try removing RouteEdge() and LayoutEditor->EdgeAttrs->Clone() lines in my code then look what happens

levnach commented 2 years ago

I understand Russian. Not sure it helps:-) I downloaded your project. I can build it but getting an error: Exception thrown at 0x00007FFB19FAC39D (clr.dll) in SpanningTreeVis.exe: 0xC0000005: Access violation reading location 0x0000000000000000. using object_colors = Microsoft::Msagl::Drawing::Color; using node_with_edge = Tuple<Node^, Edge^>;

static const object_colors VISITED_NODE_COLOR = object_colors::Green;

somehow "object_colors" is null

levnach commented 2 years ago

I think a fix would be to call StraightLineEdges::RouteEdge(Edge edge, double padding) when the routing mode in the graph setting is StraightLine. It seems the mode is just ignored.

levnach commented 2 years ago

I have found another bug in this area and committed a fix. It would not create huge arrowheads anymore, hopefully. The routing mode is still ignored.

MVoloshin commented 2 years ago

Did you succeed in fighting 0xC0000005 error? I used to compile this project using latest VS2019. P.S. I can rewrite this in C# and make a pull request once the summer session at university ends

levnach commented 2 years ago

Nope, I have not. Have you tried the fix?

MVoloshin commented 2 years ago

I'll try as soon as possible. So, I don't have to clone EdgeAttrs from LayoutEditor anymore?

levnach commented 2 years ago

I am not sure, and the routing of the edge would ignore the EdgeRoutinMode from the settings.