dondi / GRNsight

Web app and service for modeling and visualizing gene regulatory networks.
http://dondi.github.io/GRNsight
BSD 3-Clause "New" or "Revised" License
17 stars 8 forks source link

Weights should not be drawn outside the bounding box when the bounding box/viewport is fixed #444

Closed NAnguiano closed 3 years ago

NAnguiano commented 7 years ago

When a weighted graph is loaded and the "always show weights" option is selected, some of the weights are drawn outside the viewport. While this isn't so much an issue with the ability to scroll around, I figured I'd point it out in case this is something we wanted to change. I assume someone choosing fixed may not want to need to scroll outside the viewport.

screen shot 2017-04-18 at 7 21 54 pm

You can see in this screenshot that is seems that GLN3 doesn't have a weight. The weight is actually drawn right outside the viewport, and can be seen if I scroll down. screen shot 2017-04-18 at 7 22 43 pm

With non-looping edges, if the two nodes are at a great enough distance and the edge is up against the top or bottom of the bounding box, the curving of the edge will cause the weight to be drawn out of bounds. Self-referential edges will always draw out of bounds if right up against the bottom of the viewport. Here you can see these overlapping weights drawn out of bounds on the top: screen shot 2017-04-18 at 7 23 34 pm And here on the bottom: screen shot 2017-04-18 at 7 24 51 pm

I'm not sure if this is something we would want to fix or not, but I hadn't noticed it before and I'm not sure if this behavior is alright or not.

kdahlquist commented 7 years ago

If the viewport/bounding box are fixed, then the weights should not go outside of edges. "Fixed" implies that all drawable objects stay inside the viewable area.

However, I also want to say that I'd like to see #399 and #411 finished off (with the comments that @dondi and I made a couple of weeks ago) before diving into this one.

kdahlquist commented 5 years ago

duplicate with #700; closing this one

dondi commented 4 years ago

Wide range of possibilities to explore in case the “analytic” solution of individually adjusting edge weight label coordinates becomes unwieldy:

igreen1 commented 4 years ago

Commit #cb878d9 fixed this issue for non-self-referential edges. Rather than just moving the textbox for weight, the arcs of the edges of the graph are calculated such that they won't get too close to the edge. This solution was preferred since it also increases readability of the graph. However, if the group wants, @igreen1 can also simply move the textbox within bounds for any given edge !

Commit #639acc4 fixed this issue on self-referential edges (those connecting to themselves) by not allowing labels to leave the viewport. The arcs were not redrawn because it was unnecessarily complicated for this case.

Adding review-requested tag :)

dondi commented 4 years ago

@igreen1 These choices both look like they're worth a shot. Go ahead and issue a PR for it and we’ll see how it holds up to wider review once it’s on beta.

kdahlquist commented 4 years ago

image

An artifact of this fix seems to be that the arc of the edge is altered from a circular/ellipse based curve to a sigmoidal curve. Look at the blue edge between ABF1 and MSN1 in the screenshot above.

dondi commented 4 years ago

Maintaining the arc look is a priority for GRNsight so @igreen1 will refine the calculations further in order to maintain arcs throughout.

igreen1 commented 4 years ago

88edb5e changes the solution to move the weights and preserve the arc calculation. So far, it is set for precision past the radix point of 3 digits. See the screenshot. One of the weights goes out of bounds with precision > 4.

image

@igreen1 will also investigate the arc calculations further to find a way to preserve the arc while not going out of bounds.

kdahlquist commented 4 years ago

I just looked at this. Can we increase the padding a little on this? See how the edge weight at the bottom is not quite inside the bouding box. It's still getting cut off a little. Having a little white space between the edge weight and the bounding box of the viewport would be good.

image

dondi commented 3 years ago

@dondi pending the next beta refresh.

igreen1 commented 3 years ago

Increased offset amount in commit #a9859e09418beca02ed703b08766cb1ff35030da Changed the self-referential edge calculations slightly to reduce unnecessary calculations and variable overhead.

Image below shows the self-referential edge offset

image

kdahlquist commented 3 years ago

confirmed fixed in beta v4.0.26