Draw Ratsnest with curved lines/arcs to avoid overlapping

[Blubberbub]

I found myself in a situation where the Ratsnest was kind of useless, because i had a lot of connections between pads on the same side of the same IC. This caused all the ratlines to overlap one another.

It would be great if the ratlines could be rendered using a "curved line"/an arc instead of a direct line. This would greatly reduce the chance of any lines overlapping and might make it easier to see, where which line is going.

It probably shouldn't be default, because people are used to the lines being straight and not everyone might like this change, but i think it would make a great addition.

[Blubberbub (blubberbub)]

As a first-time-supporter to this project: How likely is it that a "reasonable good" patch for this feature is going to be merged?

[Maciej Suminski (orsonmmz)]

I would be fine with such patch, given it remains selectable as an option.

[Jeff Young (jeyjey)]

+1

> On 25 Apr 2018, at 07:37, Maciej Suminski <email address hidden> wrote:
>
> I would be fine with such patch, given it remains selectable as an
> option.
>

[Wayne Stambaugh (stambaughw)]

And it doesn't conflict with this:
http://docs.kicad-pcb.org/doxygen/v6_road_map.html#v6_pcb_ratsnest_improvements.

On 4/25/2018 2:37 AM, Maciej Suminski wrote:
> I would be fine with such patch, given it remains selectable as an
> option.
>

[Blubberbub (blubberbub)]

This is the patch i'm trying to test, but i'm still fighting a little bit with the build environment.

(Yes, this is missing the "optional" part)

[Seth Hillbrand (sethh)]

General comments:
- You are implementing this in the legacy canvas. This canvas will be removed in the not-too-distant future. New features should be implemented in the GAL canvas.
- Be sure to keep track of limits or you will end up with arcs >360

[Seth Hillbrand (sethh)]

Third general comment: Neat idea! :)

[Blubberbub (blubberbub)]

Thanks for your comments. These are very much appreciated.

> - You are implementing this in the legacy canvas. This canvas will be removed in the not-too-distant future. New features should be implemented in the GAL canvas.
Yea, i was just happy that the first class i looked into already had the method i wanted to change, so i stayed with that class. For testing if it is feasible its enough, but i intend to also implement it for GAL.

- Be sure to keep track of limits or you will end up with arcs >360
This should not happen: I found out, that if you want an arc that is distance * 10% high you need a radius of 1.3 times the distance between the two points. (That's where the 1.2 constant comes from - 1/(8*a) + a/2 will give you the factor for arbitrary relative heights a).
So the circle should always be big enough.

As someone on [IRC already found out, though](https://i.imgur.com/qzFvSch.png), there is a slight problem in the patch i uploaded. :D
Instead of drawing the arc i intended to draw it draws the arc for the wrong direction. Putting the Circle Center on the other side(changing + to - in the cx/cy calculation) or swapping the order of the points might be enough to fix that.

[Blubberbub (blubberbub)]

It wasn't the order of the points, but i was missing "s.x + " and "s.y + " in the cx/cy calculation. Ooops.

This patch now uses Bezier curves and should work for GAL and Legacy rendering.

[Blubberbub (blubberbub)]

For the "making it optional" i would like to add a toggle button to the left of the canvas similar to the buttons that control the pad and via style. But I haven't really found out how to do that, so i need a hint on how to achieve that. (Or it might be a trivial task for someone else more familiar with the code base?)

[Seth Hillbrand (sethh)]

Your revised patch works better.

For the toolbar, you can look at PCB_EDIT_FRAME::ReCreateOptToolbar() in tool_pcb_editor.cpp. You'll want to create an icon that can be set/unset when clicked. Also, don't forget that system options should be stored in the preferences (pcbnew_config.cpp)

[Blubberbub (blubberbub)]

I switched back to using arcs for legacy rendering, because bezier turned out to be horrible slow.
Also i added a button that allows to toggle between curved and straight lines. (Programmer-Art included)

Adding the button to also work in the modern rendering turned out to be a major hassle, because all the different files one has to change to make it work.

Sadly i don't have any example files for comparison, so if one can point me in a direction where i can download an example file i'm happy to create comparison images.

[Nick Østergaard (nickoe)]

Maybe thy this. Just use the global deletions menu to delete all tracks.

https://github.com/c4puter/motherboard/blob/master/motherboard-stable.kicad_pcb

[Blubberbub (blubberbub)]

You can see a comparison and my programmer-art toggle-icon here: https://imgur.com/a/7AVqrQ7

[Jeff Young (jeyjey)]

Not bad for programmer-art. ;)

I do think it looks too much like a box, though. I’d delete the outer 4 ratsnest lines and just curve the diagonal one a bit more.

However…

I think the curved traces are clearly uniformly better. Do we really need a setting for this?

Cheers,
Jeff.

> On 2 May 2018, at 13:55, Blubberbub <email address hidden> wrote:
>
> You can see a comparison and my programmer-art toggle-icon here:
> https://imgur.com/a/7AVqrQ7
>
> --
> You received this bug notification because you are a member of KiCad Bug
> Squad, which is subscribed to KiCad.
> https://bugs.launchpad.net/bugs/1766597
>
> Title:
> Draw Ratsnest with curved lines/arcs to avoid overlapping
>
> Status in KiCad:
> New
>
> Bug description:
> I found myself in a situation where the Ratsnest was kind of useless,
> because i had a lot of connections between pads on the same side of
> the same IC. This caused all the ratlines to overlap one another.
>
> It would be great if the ratlines could be rendered using a "curved
> line"/an arc instead of a direct line. This would greatly reduce the
> chance of any lines overlapping and might make it easier to see, where
> which line is going.
>
> It probably shouldn't be default, because people are used to the lines
> being straight and not everyone might like this change, but i think it
> would make a great addition.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/kicad/+bug/1766597/+subscriptions

[Blubberbub (blubberbub)]

I'm glad you like it, Jeff.

> I think the curved traces are clearly uniformly better. Do we really need a setting for this?

I made it the default, but i would keep the option, because i think a lot of people don't like a "forced change in their workflow" like this.

One issue remains: when moving something i kept the straight lines for now. After you stopped moving, both the straight and the curved lines are drawn until you deselect the object. The line was probably drawn 2 times before, but one couldn't see because they overlapped each other.

[Jeff Young (jeyjey)]

I’d argue that it only changes graphics, not workflow.

Wayne’s pet peeve is nag dialogs. Mine is settings. Each one sounds like a good idea at the time, but years down the road you find yourself sitting on a huge pile of them.

Cheers,
Jeff.

> On 2 May 2018, at 14:40, Blubberbub <email address hidden> wrote:
>
> I'm glad you like it, Jeff.
>
>> I think the curved traces are clearly uniformly better. Do we really
> need a setting for this?
>
> I made it the default, but i would keep the option, because i think a
> lot of people don't like a "forced change in their workflow" like this.
>
> One issue remains: when moving something i kept the straight lines for
> now. After you stopped moving, both the straight and the curved lines
> are drawn until you deselect the object. The line was probably drawn 2
> times before, but one couldn't see because they overlapped each other.
>
> --
> You received this bug notification because you are a member of KiCad Bug
> Squad, which is subscribed to KiCad.
> https://bugs.launchpad.net/bugs/1766597
>
> Title:
> Draw Ratsnest with curved lines/arcs to avoid overlapping
>
> Status in KiCad:
> New
>
> Bug description:
> I found myself in a situation where the Ratsnest was kind of useless,
> because i had a lot of connections between pads on the same side of
> the same IC. This caused all the ratlines to overlap one another.
>
> It would be great if the ratlines could be rendered using a "curved
> line"/an arc instead of a direct line. This would greatly reduce the
> chance of any lines overlapping and might make it easier to see, where
> which line is going.
>
> It probably shouldn't be default, because people are used to the lines
> being straight and not everyone might like this change, but i think it
> would make a great addition.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/kicad/+bug/1766597/+subscriptions

[Blubberbub (blubberbub)]

I don't know how such things are decided in this project. Is there someone that just makes a decision? Will there be a poll?

Should i also draw the dynamic ratsnest using curved lines?

[Nick Østergaard (nickoe)]

Ad #16, I personally don't really think forcing arbitrarily curved rastnests are better. I don't agree with #15, although I think the icon commented look ok as proposed by Blubberbub, but no strong opinion here.

I would definitely like this as an option somehow, maybe even make the toggle between the one and the other style hotkeyable. It can be useful to get the rastnests drawn differently by a toggle.

[Jeff Young (jeyjey)]

I believe our decision making looks something like (in priority order):

1) Wayne makes a call
2) A consensus appears to develop
3) Make a proposal: if met with agreement of deafening silence go ahead with it

So far (1) and (2) haven't happened. You can try and get a (1), or propose a (3) of your own.

[Wayne Stambaugh (stambaughw)]

We are in feature freeze so this isn't going to happen until after the v5 release. I haven't had time to review or test the patch yet and I probably wont until after v5 is out.

[Wayne Stambaugh (stambaughw)]

Version 5.1 is in feature freeze. Moving to 6.0.0 milestone.

[Jeff Young (jeyjey)]

@Blubberbub, master is now open for 6.0. Can you rebase your patch so I can apply it?

Thanks,
Jeff.

[Blubberbub (blubberbub)]

I rebased the patch on `master` and fixed the conflicts. Only very minor changes were necessary to make it work (the way bitmaps are handled changed slightly).

In my very small test it appeared to work like it is supposed to.

So if you can give it a test and merge it i would be very happy (Keep in mind, that this is my first contribution to this project, so i'm not very familiar with the code base).

[Seth Hillbrand (sethh)]

One small request: Can you change the license of the artwork to CC-BY-SA-4.0? In Inkscape, this is under Document Properties.

[Jeff Young (jeyjey)]

@Blubberbub, also the way your patch is formatted (just a diff?) will commit the changes under my name instead of yours. Can you use git format-patch to make the patch? That way you'll get credit for it.

[Blubberbub (blubberbub)]

Added the licence to the .svg file and used `git format-patch` to format the patch this time. Tried to match the commit message format.

Also fixed a small typo in a comment.

[Jeff Young (jeyjey)]

Perfect. I'll merge it to master.

Thanks for your contribution to Kicad!

[Tomasz Wlostowski (twlostow)]

May I kindly ask that every option we add is available in the Preferences dialog and not only through an icon in the toolbars?

@blubberbub: could you provide a fix?

Tom

[Blubberbub (blubberbub)]

Hi Tom,

i couldn't find any of the other options on the toolbar in the preferences dialog (I mostly used the existing toolbar entries as a template for implementing the new one). Am i missing something here?

Also please consider, that i'm not familiar with that code at all, so adding this (probably) very simple option to the settings will very likely take me more than a few hours for a change that could probably be done in less than half an hour by someone who actually knows what they are doing and would require less review and iterations.

If nobody else is going to do it and you consider it a necessity i would give it a try. I don't expect to have much free time to spare in the near future, though. :/

[Wayne Stambaugh (stambaughw)]

This patch really should have been cleaned up before it was pushed into master. There are a lot of coding policy violations. We really need to start doing a better job of enforcing this. I'm seeing way too many coding policy issues as I traverse through the KiCad source.

@Tom Please submit a new bug report for the missing configuration option rather than reopen this bug report. The patch actually satisfied the requirements of the bug report.

[Tomasz Wlostowski (twlostow)]

@Blubberbub My comment wasn't directed exclusively at you, sorry if it sounded thay way. What I meant is that there are many functions of KiCad accessible only by toolbars or hotkeys that have no menu entries or checkboxes in the Preferences dialog. This is confusing and contributes to the perceived complexity/weirdness of Kicad GUI.

If you want to add this option, the starting point are these wo files:
- panel_pcbnew_settings_base.fbp where you can add a checkbox using wxFormBuilder,
- panel_pcbnew_settings.cpp: sync the ratsnest option with the dialog state in TransferData(From|To)Window().

PS. I like the new curved ratsnest lines, makes some connections easier to follow, especially in BGAs or other components with arrays of pads. Good job!

Tom