Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unset initial_shape_id once selected #342

Merged
merged 6 commits into from
Sep 8, 2020

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Aug 24, 2020

Fixes one of the issues reported at #335 and a related bug:

To test:

  • Find a multi-plane image that has > 500 ROIs (where ROIs are paginated by Z/T plane
  • Browse to a Shape and copy link to it. (workflow from Roi query support #311)
  • Paste link into browser - iviewer should browse to that Z/T plane and select the Shape
  • Try browsing to a different Z/T plane, either with Z/T sliders or via clicking on another Shape
  • Viewer should show the newly-selected Z/T plane and NOT be redirected back to the original Z/T with Shape selected.
  • If browsing via clicking on a Shape, the newly-clicked shape should be selected when the ROIs load for new plane.
  • Also, test the above workflow on an image where ROIs aren't paginated

cc @jburel

@jburel
Copy link
Member

jburel commented Aug 26, 2020

Not related to this PR but noticed while testing see #346 and #347

#346 is also annoying when using an image as suggested in the description e.g. 139315 (user-3)

@jburel
Copy link
Member

jburel commented Aug 26, 2020

  • Select a shape in the right-hand table. The plate is loaded and shape selected

Screenshot 2020-08-26 at 06 51 50

* Copy the shape link and paste in another tab. Shape is not selected

Screenshot 2020-08-26 at 06 52 07

@will-moore
Copy link
Member Author

OK, I found 2 bugs for loading ?shape=328263 above.
When we want to load ROIs (using shape ID) from an image where ROIs are paginated (>500 ROIs on the image), we use iviewer/shape/3328263/page_data/ to look-up which page the ROI is on. In this case we get:

{
"image": {
"id": 139315
},
"roi": {
"id": 319825
},
"roi_index": 263,
"roi_count": 1104
}

Then we load the appropriate page of ROIs to show on the Image and can highlight the appropriate shape.
This works when we have a single-plane image with paginated ROIs (> 500) and also works on a multi-plane image
where we don't have paginated ROIs. Here, although the first plane that loads is not the correct one for the shape,
but triggering a 'shape select' event, we re-load the correct plane (as if you've clicked on the shape in the table).

However, when we have a multi-plane image with paginated ROIs we have a couple of problems:
Loading the ROIs that are on the initial 'default' plane won't necessarily include the target Shape, so when we trigger 'select shape', that shape isn't found (not been loaded) so we don't update to the correct Z/T plane.

Also, if the page_data info above indicated that the ROI would be on page 2 then we use page 2 ?offset=500&limit=500 when loading the ROIs for the default plane - even though there are only a small number of ROIs on that plane!

So, the plane_data needs to include the Z and T indices for the Shape and the roi_count and roi_index needs to refer to the ROIs on that plane! Then iviewer needs to load ROIs for the correct Plane directly.

@will-moore
Copy link
Member Author

will-moore commented Aug 27, 2020

Phew - that was a lot of work to hopefully fix that bug.
Quite a bit has changed, so will need a good test.

@jburel
Copy link
Member

jburel commented Aug 30, 2020

bug described in #342 (comment) is now fixed

@jburel
Copy link
Member

jburel commented Aug 30, 2020

Looks at image with limited number of ROIs, behaviour is as expected

@jburel
Copy link
Member

jburel commented Aug 30, 2020

Outside the scope of this PR, looking at https://merge-ci.openmicroscopy.org/web/webclient/img_detail/105665 (user-3) > 150000 ROIs, there is no way to identify the page to select when we are interested in one area. Ideally user should be able to select a region and load the rois for that region, not the opposite

@will-moore
Copy link
Member Author

I looked into loading ROIs by region in #242. But there are a number of limitations - lot least is the lack of any way to query Shapes by coordinates.

Another approach to browsing ROIs by x/y coordinates is in my parade prototype (see https://youtu.be/FyjGhZxx6es?t=422) but this is more about plotting analysis results and is very much a read-only use-case.

@will-moore will-moore mentioned this pull request Sep 7, 2020
@jburel
Copy link
Member

jburel commented Sep 8, 2020

The problem that this PR tries to fix is now solved.
Merging. More work to be done in follow-up PRs

@jburel jburel merged commit c411712 into ome:master Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants