Add ellipse selection widget to the new Instrument View#40841
Add ellipse selection widget to the new Instrument View#40841jclarkeSTFC wants to merge 8 commits intomantidproject:mainfrom
Conversation
jclarkeSTFC
added
the
Epic
github-actions
bot
added
the
Has Conflicts
|
👋 Hi, @jclarkeSTFC, Conflicts have been detected against the base branch. Please rebase your branch against the base branch. |
jclarkeSTFC
removed
the
Has Conflicts
andy-bridger
left a comment
andy-bridger
left a comment
There was a problem hiding this comment.
Looks good, I've had a good try at breaking it and nothing catastrophic has fallen out - comments are mainly aesthetic
qt/python/instrumentview/instrumentview/test/test_shape_widgets.py
Outdated
Show resolved
Hide resolved
| self._ellipse_mesh = self._create_ellipse_mesh(cx, cy, rx, ry, z, angle=self._angle) | ||
| self._ellipse_actor = self._plotter.add_mesh( | ||
| self._ellipse_mesh, | ||
| color="yellow", |
There was a problem hiding this comment.
Is it intentional that all the widgets are different colours (circle - red, box -white, ellipse - yellow)? An argument could be made for them all being the same colour so they all look a bit more unified?
There was a problem hiding this comment.
Ellipse is now white to match the rectangular bounding box
Co-authored-by: andy-bridger <71634246+andy-bridger@users.noreply.github.com>
This helps with selecting the rectangle grab point when it's close to the rotation handle.
andy-bridger
left a comment
andy-bridger
left a comment
There was a problem hiding this comment.
Thanks for making those changes, sorry to be annoying with some final comments - especially because I wouldn't be committed to what I have requested as the best option (I get quite a noticeable grid illusion https://en.wikipedia.org/wiki/Grid_illusion from the handles on the black background which is quite distracting - I've played with a few options to make it better but haven't had much luck)
So feel free to either make the changes and I'll approve or say you disagree and I'll also approve!
| render_lines_as_tubes=False, | ||
| pickable=False, | ||
| show_edges=False, | ||
| name=f"ellipse_widget_{id(self)}", |
There was a problem hiding this comment.
I personally think having point_size = 0 looks nicer and neater than seeing the individual points along the ellipse (matches the rectangle better as well)
|
|
||
| x, y, _z = self.display_to_world_coords(width / 2 + 0.15 * width, height / 2, 0) | ||
| cylinder_repr.SetRadius(np.sqrt((x - cx) ** 2 + (y - cy) ** 2)) | ||
|
|
There was a problem hiding this comment.
I'd also make this white:
| cylinder_repr.SetEdgeColor(1, 1, 1) |
2 participants
This adds a new elliptical widget for selecting detectors in the new Instrument View. You can resize both axes of the ellipse, and rotate it.
Closes #40757 .
To test:
Check all three widgets still work for selection, circle, rectangle, ellipse. Resize and rotate the ellipse, try dragging the edges over each other, ellipse should reverse.
I recommend you zoom in a bit before drawing shapes, otherwise you'll end up picking loads of detectors.
Reviewer
Your comments will be used as part of the gatekeeper process. Comment clearly on what you have checked and tested during your review. Provide an audit trail for any changes requested.
As per the review guidelines:
mantid-developersormantid-contributorsteams, add a review commentrerun cito authorize/rerun the CIGatekeeper
As per the gatekeeping guidelines: