if arrow_shift = :end, move arrows to surface of destination node

[hdavid16]

This PR changes the behavior when arrow_shift = :end. Instead of having the center of the arrow marker on the center of the destination node, the code now calculates the position along the edge such that the tip of the arrow marker will touch the tip of the destination node marker. Note: currently works properly for Circle node marker and Arrow arrow marker since these are of base size 1x1.

radius_px can be extended for other marker types that do not have a 1x1 base size.

close #107
close #101
close #109
close #112
and update docs

To do:

• test with interactive plot (i.e., drag node)

[hdavid16]

Changed the default node and arrow markers so that their size matches their pixels (ie., 1x1 base size; see https://docs.makie.org/v0.19/examples/plotting_functions/scatter/index.html#markersize)

[hdavid16]

• To do: add example in docs

[codecov-commenter]

Codecov Report

Base: 80.45% // Head: 81.29% // Increases project coverage by +0.84% 🎉

Coverage data is based on head (c183fa9) compared to base (6c40e58).
Patch coverage: 88.60% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
+ Coverage   80.45%   81.29%   +0.84%     
==========================================
  Files           4        4              
  Lines         573      647      +74     
==========================================
+ Hits          461      526      +65     
- Misses        112      121       +9     

Impacted Files	Coverage Δ
src/beziercurves.jl	95.88% <84.37%> (-2.69%)	⬇️
src/utils.jl	97.29% <90.00%> (-1.15%)	⬇️
src/recipes.jl	96.75% <91.89%> (-0.78%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[hexaeder]

I am not totally sure about the choice arrow_shift=:end vs arrow_shift=1. I tend to prefere :end because otherwise the arrow_shift is not continous in 0..1 but instead at 1 the arrows jump back.

[hdavid16]

I am not totally sure about the choice arrow_shift=:end vs arrow_shift=1. I tend to prefere :end because otherwise the arrow_shift is not continous in 0..1 but instead at 1 the arrows jump back.

I changed it to :end and updated docstring

[hexaeder]

Hey! I finally tried it locally and it works quite nice, thanks for the contribution! Three minor things i've noticed:

Recalculate on _size change

g = SimpleDiGraph(3)
add_edge!(g, 1, 1); add_edge!(g, 1, 2); add_edge!(g, 2, 1); add_edge!(g, 2, 3); add_edge!(g, 3, 1);

# test update of  node and arrow size
fig, ax, p = graphplot(g; arrow_shift=:end,
                       node_size=[20 for _ in 1:nv(g)],
                       arrow_size=[20 for _ in 1:ne(g)])

p.node_size[][1] = 40
notify(p.node_size)  # does not update arrow shift

p.arrow_size[][3] = 40
notify(p.arrow_size)  # does not update arrow shift

notify(p[:node_pos]) # updates arrow shift

I don't now the chain of observable trigger actions of the top of my head, but change in [arrow|node]_size should probably trigger update of the arrow shift.

Error for 3d plos

using GraphMakie: Spring
fig, ax, p = graphplot(g; arrow_shift=:end, layout=Spring(dim=3))

Errors at reipes.jl:687 because of addition of 2d point to 3d point. I think this should fail early with an error message that 3d plots are not supportet with :end currently.

interpolation fails when zooming out
If the markers get very big with respect to the node distances something goes wrong and throws an error. The easisest to reproduce is to scroll out in a plot very far. I'm not sure where that happens. I think this is due to the desired distance getting negative or so. This should be chacked somewhere. Maybe let reverse_interpolate throw an ArgumentError, catch that in update_arrowshift and set it to 0.5?

[hdavid16]

Thanks for testing these.

• I fixed the lifted function so that it triggers with node size and arrow size changes.
• I now have 3D plots with arrow_shift = :end erroring at the update_arrow_shift function (I added a separate method for 3D plots). When we resolve Rotations in 3D #111, we will be able to update the code so that this arrow shift is supported for 3D plots.
• Interpolation now won't crash if we zoom out. The issue was occurring when the inverse interpolation gave a negative t value (see arrow_shift less than 0 fails (BoundsError) #112). I have now fixed this so that when a negative t value is passed, the first segment will be chosen for the interpolation (rather than the 0th or negative segment which doesn't exist). This is similar to what is done when t > 1 in both interpolate and tangent (we chose the last segment for the interpolation, and end up extrapolating). This fixed this error up to a certain point of zooming out. However, if you zoomed out far enough, the inverse interpolation (root finding with 5th order polynomial) fails because you start to get only imaginary roots or no roots at all). I now catch in update_arrow_shift when t = NaN and reset it to 0.5 with a Warning.