search

netbox-community / netbox-topology-views

A netbox plugin that draws topology views
Apache License 2.0
744 stars 63 forks source link

Option to disable Physics #489

Open dremerb opened 5 months ago

dremerb commented 5 months ago

I wanted the option to disable physics, because with a bigger network it takes quite some time before things settle. Also organizing is not optimal, when devices do not have coordinates yet and start flying around when another is moved. Unfortunately, simply disabling physics puts everything at (0, 0) by default.

So I got just overengineered some static clustering. The idea is to find a random coordinate as a center point for each rack (using the rack name as a seed yields the same "random" coordinates for every rack) and adding a small random offset by using the (unique) device name as seed. Currently there is two magic numbers (12 in line 218 and 2 in line 225/226). They scale the resulting graph.

image

I recognize the idea is a bit unorthodox, but I also think that having physics enabled by default is not great for larger networks.

Note: If you consider merging this, I think the commit should be squashed, as I initially had #488 as a single branch and split it into two PRs. This makes the history pretty ugly.

dreng commented 5 months ago

Hi,

first of all, thank you for your contribution.

Which accepted FR do you close with this PR actually? If it's #436, that one is under review for a good reason. It is absolutely not clear how to implement the FR. You've made a solution that only fits your personals needs. We should always find solutions that fits everyones needs or discuss until we get to a good compromise.

dremerb commented 4 months ago

This PR does not have a FR, I just wanted the option to disable physics, since in my graph it was a chore to organize the nodes when things were moving constantly. Of course that changes once you turn on "save coordinates", but that still requires every node to be selected/moved once.

436 and #122 both touch the idea of disabling physics, but it is not their main focus as far as I understand.

You've made a solution that only fits your personals needs. We should always find solutions that fits everyones needs or discuss until we get to a good compromise.

Agree with the second part. I built this as a simple fix to everything being located at (0, 0) when I disables physics in the module by setting the key to False for all nodes and edges. Also it fits my personal needs, but since I am working with a decently large setup, I hope it holds some value that can be applied generally.
The proposed code is a "best effort" clustering, instead of running a full ground state simulation on the server. While it does not look as pretty as the result of vis.js' physics simulation, it is obtained a lot faster and does not have a round shape (which is unusual for topology graphs).
A (in my opinion sane) assumption I made was, that users will re-organize the graph manually anyway, therefore I prioritized clustering racks over it looking pretty.

I maybe should have made it clearer, but this more or less is a Feature Request that offers an idea how to solve the issue of not having coordinates when simply disabling physics in vis.js.
If it is easier to discuss first, I am open to close this, create an Issue to discuss the option to disable physics.

mr1716 commented 4 months ago

This would be a great change to do!

dremerb commented 4 months ago

Messed up managing my branches and pushed a merge that does not belong here. Removed those commits with a force push.

mattieserver commented 2 months ago

I tried this out but i am not convinced the seeding works very well (at least for my own testing). I am not sure this is the way we want to implement it, however it gave me and idea on what to try.

I will not merge this for now.