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

Roi table pagination #231

Merged
merged 44 commits into from
Feb 26, 2019
Merged

Roi table pagination #231

merged 44 commits into from
Feb 26, 2019

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Feb 6, 2019

This now paginates the ROI table by Z/T plane for multi-plane images.
This is a much nicer experience than simple pagination of all ROIs (testing with the Multi-T image below). It should allow you to work with ROIs "as normal" for images with less than 500 ROIs per plane. For Big Images (or any image planes with > 500 ROIs per plane) we paginate the ROIs within that plane.

To test, open an image with lots of ROIs (user-3):

  • Multi-Z/T/C image http://web-dev-merge.openmicroscopy.org/webclient/?show=image-161151
    • NB: Not all planes have ROIs on them. Some Shapes have C set, most don't.
  • Browsing, editing & deleting ROIs/Shapes on this image should work "as normal" except that you can't browse the ROI table for shapes on other planes. E.g:
    • If you create more ROIs (drawing or copy/paste) then save and go to another time-point and back to current time-point, you should see the ROIs you created.
    • Delete all the ROIs on a particular Z/T plane - save.
    • Edit existing ROIs and save.
    • Setting T or Z to 'unattached' for a Shape should make it show up in all T / Z planes.
    • Create, edit or delete a shape, then try to change Z/T or 'page' (see below). Should get "Please save first" dialog.
  • If there are more than 500 ROIs to show, then we do pagination. E.g:
    • Enable Z-projection for the image above. ROI table will show all ROIs within the Z-range.
    • Choose a plane, select all ROIs, copy and paste (then drag them and set colour so as to set them apart from others). Repeat until you have > 500 ROIs for that plane. Save. Then go to a different plane and back to this one and you should see it paginated.
    • Can change page by < or > buttons, slider or by putting number in input. No updates possible while the ROIs are loading and invalid numbers should be ignored. Also, no change possible with unsaved ROIs.
    • Big Image: http://web-dev-merge.openmicroscopy.org/webclient/?show=image-160953
  • Behaviour should be completely unchanged on images with < 500 ROIs.

screen shot 2019-02-12 at 03 05 10

@pwalczysko
Copy link
Member

screen shot 2019-02-07 at 17 11 17

I think the numbers here should be editable. I would like to navigate to the same pages, without making too many mistakes (it takes quite a while before the ROIs on that page load, so it is costly to land on a wrong page)

If we requestRois here (when switching images and the ROI tab is active)
then we load ROIs before we know the current Z/T index of the image
and before any cached Z/T settings have been applied.
We don't need to request ROIs here, since the image_info will do
this on success from loading image info if ROIs tab is active
@will-moore
Copy link
Member Author

Those commits should address these issues raised above:

  • ROI drawing tools disabled while ROIs are loading
  • Don't load ROIs while movie is playing - load when it stops (via user or end of T)
  • Also should keep ROIs in sync if user hits > button lots (instead of playing movie)
  • ROI_PAGE_SIZE is configurable. bin/omero config set omero.web.iviewer.roi_page_size

I think we made the decision not to zoom in when an ROI is selected since you may already be at the zoom level you want and if you're moving back and forwards between working with ROIs on the image and in the table, zooming each time you select an ROI is going to be annoying. We even decided not to pan the image if the ROI is within the view-port for the same reason.

@jburel
Copy link
Member

jburel commented Feb 13, 2019

For the zoom level, we might have to revisit that with more feedback. Let's keep it as it is for now

@pwalczysko
Copy link
Member

Does not work in IE, the ROI table never loads on images with many ROIs.

@pwalczysko
Copy link
Member

Win 7 on Firefox and Chrome this works fine. Chrome is slightly faster.

@will-moore
Copy link
Member Author

@pwalczysko I think the failure to show ROIs in the table was a layout issue in IE.
Hope those commits fix it (I was testing in IE11).

@pwalczysko
Copy link
Member

It works now on IE11 for me as well. It is quite slow, but it loads the table and I can select the ROIs and change the page.

@jburel
Copy link
Member

jburel commented Feb 18, 2019

I think iviewer on IE is generally slower than on other browser

@pwalczysko
Copy link
Member

Loading of ROIs gets sometimes "stuck". this is not browser specific. I still have to find out a foolproof workflow.
Some combination of opening the svs image in multiview and then combinig it with the multi-t image from the same dataset, then clicking on the single windows whilst being on the ROI tab, will get into endless "Loading ROIs" state.

@will-moore
Copy link
Member Author

@pwalczysko Just make sure you have the developer tools open so you can see any error there (under Console tab). Also the Network tab will tell you whether the last call failed or returned nothing.

@pwalczysko
Copy link
Member

pwalczysko commented Feb 18, 2019

@will-moore I got stuck just now in FF Win 7, whilst trying to retrieve
http://web-dev-merge.openmicroscopy.org/static/omero_iviewer/css/all.min.css?_iviewer-0.6.0

Under console tab, there is a message

Use of mozImageSmoothingEnabled is deprecated. Please use the unprefixed imageSmoothingEnabled property instead.

IN Safari, lets have two images open. On one, I am getting the ROIs table as expected. On the other image, it does only "Loading ROIs" message.
In the Network tab, the one image and the other have the same response - both apparently loaded successfully, only in reality, only one really did.
screen shot 2019-02-18 at 14 21 17
screen shot 2019-02-18 at 14 21 09

@will-moore
Copy link
Member Author

will-moore commented Feb 19, 2019

@pwalczysko I think those FF messages are not related to this issue (you can use the XHR filter in the Network tab to exclude css, images etc), but I managed to reproduce what you're seeing in Safari and added a fix.
To test, simply have any 2 image viewers open (without ROIs loaded), then click on the ROIs tab for one of them and then select the other image. ROIs didn't load for the newly-selected image, but now they should.

@pwalczysko
Copy link
Member

pwalczysko commented Feb 19, 2019

@will-moore Great, thanks for finding out the workflow. Yes, now, in Safari and FF on Windows 7, I have no problems.

The only issue I see is on IE, where , IE being much slower, when I go through the workflow described by you #231 (comment) - then there is a long delay in switching between the windows. Up to the point that a message came "openmicroscopy is not responding due to a long-running script". But, in the end, the switch happens, and the ROI table loads.

(Edit: yes, confirmed once more - even on IE, after a long wait, the behaviour is as expected, verified three times)

@will-moore
Copy link
Member Author

That last commit fixes a 'silent bug' that causes additional loading of ROIs and might explain some of the long delays seen above. In the case of opening an image when the ROI tab is already open, the previous fix 42f5d68 ensures that we load ROIs immediately (as we did before this PR). However, we don't yet have the image data (including roi_count) in hand at this point so the initial ROI loading doesn't use pagination (just loads first 500 ROIs). Once the image data has loaded, init of Z/T sliders (& setting of Z & T values) causes the ROIs to load again, this time using pagination for the current plane. This doesn't happen until after the first, slow loading of 500 ROIs is done.
Now, with 1e152ff we don't start loading ROIs until the image_data is loaded, so we avoid the wasted (and slow) call to load 500 ROIs.
Unfortunately, in cases where we don't need to paginate by plane, we are now waiting a bit longer before we start loading ROIs, but this can't be helped.

@will-moore will-moore mentioned this pull request Feb 21, 2019
@pwalczysko
Copy link
Member

I tested on Win 7 IE. The performance is still very slow. It is very hard to say if it is faster than before, the ROIs on svs with multi-view load within 43 secs - but that is such long a time that it is hard to judge it as a speed-up.

I do not see any worsening of the ROI loading speed on FF Mac with ROIs around 200 (non-paginated)

@jburel
Copy link
Member

jburel commented Feb 26, 2019

Merging this PR for the rc. In the long run, the table is likely to be removed. See discussion on the investigation PR #242

@jburel jburel merged commit 3dd0dcd into ome:master Feb 26, 2019
@jburel jburel added this to the 0.7.0 milestone Mar 14, 2019
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.

3 participants