constellation icon indicating copy to clipboard operation
constellation copied to clipboard

Select in Box works incorrectly for parallel connections

Open serpens24 opened this issue 4 years ago • 13 comments

Prerequisites

  • [X] Put an X between the brackets on this line if you have done all of the following:
    • Running the latest version of Constellation
    • Attached the Support Package via Help > Support Package
    • Checked the FAQs: https://github.com/constellation-app/constellation/wiki/FAQ
    • Checked that your issue isn't already filed: https://github.com/constellation-app/constellation/issues
    • Checked that there is not already a module that provides the described functionality: https://github.com/constellation-app/constellation/wiki/Catalogue-of-Repositories

Description

When selecting connections using select in box, the plugin only checks if the box crosses the middle connection between the two nodes. If it does, all parallel connections are selected. If not, no connections between those nodes are selected.

Steps to Reproduce

  1. Create a graph with 2 nodes.
  2. On the graph, ensure all transactions are individually shown, using the Transactions (no merging) toggle on the left hand button pane.
  3. Open the table view.
  4. Add multiple (3) transactions between these nodes.
  5. Zoom in across the transaction lines so that there is a clear gap on the graph between each transaction.
  6. Left click and drag on the graph to display the selection rectangle but ensure only the central transaction overlaps the rectangle.
  7. View that upon releasing the mouse, all three transaction lines are selected, not just the centre one, additionally all three transactions are highlighted in the table view.
  8. Left click and drag on the graph to display the selection rectangle but ensure only one of the transactions (other than the central one) overlaps the rectangle.
  9. View that upon releasing the mouse, no transaction lines are selected, and no transactions are selected in the table view.

Expected behaviour:

  1. Step 7 (above) should only highlight the central transaction, with the corresponding transaction also highlighted in table view.
  2. Step 9 should highlight the transaction that intersected the selection rectangle and the corresponding transaction should be highlighted in table view.

Actual behaviour:

  1. Step 7 selects all transactions between the two nodes, these transactions are all highlighted in table view as well.
  2. Step 9 does not select any transactions and no transactions are highlighted in table view..

Reproduces how often:

100%

Additional Information

nil

serpens24 avatar Mar 25 '20 11:03 serpens24

CoreInteractiveGraph\src\au\gov\asd\tac\constellation\graph\interaction\plugins\select\BoxSelectionPlugin.java (edit function) is the location of the code that manages the selection of vertexes and/or transactions/edges/links. The basic premise is that checks are made to see if the selection box overlaps any vertexes, if it does, these vertexes are selected and processing ends ... otherwise, checks are made to see if the selection box overlaps any transactions connecting these vertexes.

The underlying issue here is the calculations use the X,Y co-ordinates of the vertexes. These coordinates are located at the centre of each vertex. Refer to image below which highlights the issue.

image

In the above diagram, 4 selection boxes are shown, A, B, C, and D. Of these, two of them do not make any selections (A and B) while the other two do make selections (C and D), although C incorrectly selects all three transactions.

Selection boxes C overlaps the direct line between the centre of the two vertexes (red dots) while selection box D overlaps the centre of Vertex #1.

So in addition to the identified incorrect behaviour called out in this issue (if the issue as described is how this should function .... see my further comments below), I'd also contend that for consistency, Selection box A is also incorrectly not selecting Vertex#0.

Further note, there is a Point sleection plugin which is able to determine exactly which transaction line is clicked on .... so somewhere in the underlying codebase must be code that is able to determine a transaction line at a given graph X/Y coordinate.

Is the requested behaviour what we want

@arcturus2 While putting together the diagram above I mocked it up using Powerpoint .. a somewhat well known and understood product. The selection mechanism in Powerpoint works differently to what we are trying to do in constellation, in that Power point selects elements that are completely bounded by the selection box... as such, using the Powerpoint approach, no vertexes or transaction would have been selected.

serpens24 avatar May 07 '20 02:05 serpens24

@serpens24 the diagram and explanation is really well done, good on you!

I think that being able to make selection work for scenario A would be a good thing.

A fair point regarding Powerpoint but I'd prefer we not go down the path of making selection work if the selection has to cover the entire node/transaction. Constellation's focus is to enable the user to analyse their graph and the quicker then can get to that point the better - so less mouse movements as possible. Powerpoint isn't an analysis tool so it may be acceptable for this audience i presume.

With scenario A, I've seen how frustrating/disappointed it can be for users when they make a selection but don't get the node selected. Honestly, I didn't investigate why this was happening but your diagram and explanation made sense as this was the behaviour I've seen.

My vote is to fix the transactions being selected to only highlight the intended one and a stretch goal or another ticket to make the scenario A work too.

arcturus2 avatar May 07 '20 07:05 arcturus2

My vote is to fix the transactions being selected to only highlight the intended one and a stretch goal or another ticket to make the scenario A work too.

@arcturus2 - I'm happy to see if this is possible. I don't have a good understanding of how the rendering works, but the key will probably be whether the displayed lines etc are stored somewhere with their start/end coordinates .....(rather than using the X/Y coordinates of the raw vertexes) ...
I might throw the debugger over the PointSelectionPlugin and see if anything springs up ... but happy to get any adivce that might be out there ?

serpens24 avatar May 07 '20 23:05 serpens24

I haven't had the pleasure of working on the renderer myself so I don't have first hand tips. Some things I know though are:

  • The cursor changing from the arrow to cross hairs is using the HitTester code. This may not be relevant to the select in box
  • If your debugging requires you to look at the JOGL code then you can break point there code by linking the source jars. You probably knew this already.

arcturus2 avatar May 08 '20 02:05 arcturus2

HitTester.display(...) seems to be the location that determines if the hovered position is a Vertex, Transaction or nothing. It does so by grabbing an item out of the renderer buffer. A simple if statement determines what it is. This may not be the right implementation direction to go - as it would require checking of colours in a selection box. I'm not sure of the feasibility, but someone more experienced may be able to do some calculations with the distance from the node and the nradius attribute. Which effectively gives you the size of the element. Transactions may be able to use the same concept - no small feat!

aldebaran30701 avatar May 15 '20 01:05 aldebaran30701

Okay so I have managed to get stretch goal A working @arcturus2 now nodes are selected when the box touches any part of the node.

Moving onto the transactions now, I'm doing this second because my first impression is that this is a larger and more difficult change.

You can see the changes so far in https://github.com/constellation-app/constellation/tree/bugfix/Issue365-SelectBox

Nova-2119 avatar Sep 03 '20 02:09 Nova-2119

@aldebaran30701 You were pretty much bang on, I was able to take the r attribute and use that to find the extremities of the node. Then i just check if the box is outside the bounds of the opposite edge of the vertex. So I check if the right side of the box if to the left of the left side of the vertex. If any of these checks are true then the vertex isn't in the box, if all are false then vertex is in the box. There's some funky conversion stuff as well, but the way Constellation displays nodes means we don't have to do anything more than convert the original coordinates which was already done for us! (If only I had realized that initially...)

Sadly I don't think transactions will be able to use the same logic. Ive only just started looking, but I don't believe they have a "location" or any sort of equivalent like nodes do. I'll update here as I figure stuff out.

Nova-2119 avatar Sep 03 '20 02:09 Nova-2119

This issue is stale because it has been open for 6 months with no activity. Consider reviewing and taking an action on this issue.

github-actions[bot] avatar Mar 03 '21 00:03 github-actions[bot]

Pull Request https://github.com/constellation-app/constellation/pull/992 fixes the issue described above when selecting vertices.

I have ran out of time to fix the issue when selecting transactions (this is harder) but here is what I have so far: The solution will rely on recreating the logic for splitting transactions used when rendering them to the graph (unlike verticies they have no location data we can access). The logic for this seems to rest in Line.gs:76-78. You cant test this by removing the "/ 32" from lies 77 & 78 and observing the change in distance between edges/transactions. I was unable to confirm what value(s) gData[0].q generally takes. Whoever picks this ticket up should figure that out first. After that it should be fairly straightforward to replicate this logic in the BoxSelectionPlugin to figure out what transactions are where and hence what should be selected.

Stretch goal: If you can figure out a way to decentralise the logic for offsetting transactions so that Line.gs and BoxSelectionPugin can refer to the same logic that would be better.

Nova-2119 avatar Mar 18 '21 00:03 Nova-2119

This issue is stale because it has been open for 6 months with no activity. Consider reviewing and taking an action on this issue.

github-actions[bot] avatar Feb 08 '22 01:02 github-actions[bot]

This issue is stale because it has been open for 6 months with no activity. Consider reviewing and taking an action on this issue.

github-actions[bot] avatar Aug 31 '22 00:08 github-actions[bot]

This issue is stale because it has been open for 6 months with no activity. Consider reviewing and taking an action on this issue.

github-actions[bot] avatar Mar 08 '23 00:03 github-actions[bot]

This issue is stale because it has been open for 6 months with no activity. Consider reviewing and taking an action on this issue.

github-actions[bot] avatar Sep 19 '23 00:09 github-actions[bot]