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

Disable graph and intensity pane for bigimages without stats #3631

Merged
merged 1 commit into from
Mar 19, 2015

Conversation

dominikl
Copy link
Member

Workaround for Ticket 12790

Problem: Graph and Intensity view pane should actually be disabled for big images, but float images aren't recognized as big images (PixelService.requiresPixelPyramid() always returns false for float images -> which on the client side means, it's not a big image). And with #3525 float big images don't have a statsinfo anymore, i. e. global min/max of channels can't be determined, which raised the exception in the ticket above, because this is needed for the graph and intensity view pane. This PR disables the graph and intensity view pane in these cases.
It's more a workaround, because the underlining cause (float images aren't recognized as 'big images') is not tackled here.

@jburel Your thoughts, use this workaround or add some kind of isBigImage() method to Pixelservice, to determine if an image exceeds max plane size, regardless of pixel pyramid is required or not?

Actually, further thinking about it, with #3580 it's possible to also import small images without stats generation with --no_stats_info (even this doesn't make sense, I guess), which also would raise the exception in Ticket 12790, so regardless of the big image problem, this PR would make sense to merge anyway.

Test:

  • Import a big float image, open the image in full viewer, open roi/measurement tool. No crash should happen, there shouldn't be a graph and intensity view pane.
  • Repeat the same for other kinds of non-big images and make sure the Graph and Intensity pane are there in these cases.

Sample image for testing: mrc/AMI-public-data/GroEL/05may19a_00013gr_00008sq_00002hl_00002ef.mrc in team/will/float-data.zip

--no-rebase

@pwalczysko
Copy link
Member

Although the PR IS included, the crash still happens on user-4 image ID 25557

@dominikl
Copy link
Member Author

Sorry @pwalczysko , looks like I pushed a wrong commit before. Now it's the right one and it should work (in the next merge build).

@joshmoore
Copy link
Member

To do this "right", do we need another method like requiresPyramids which is pyramidValid or whatever? Should it throw an exception? Just thinking of what state we've left other users in.

@dominikl
Copy link
Member Author

I don't really know, what to do on the server side, but I guess we shouldn't/can't do anything about that for 5.1.0-m5. The requiresPixelPyramid method is still valid, it has just been slightly "misused" in Insight to also determine if an image is a big image. The only implications I've seen so far was the problem with the graphs/intensity pane which should be fixed with this PR, and that the preview tab is available for big float images (which shouldn't be the case for big images). Can't see any other problems arising from that (yet).

@pwalczysko
Copy link
Member

Tested for

  • float big image ID 25557
  • an imported dv with --no-stats-info
  • images with stats which should have the Graph pane

All works as expected.

@joshmoore
Copy link
Member

Thanks for the explanation, @dominikl. If you guys do need another boolean method to differentiate, say the word.

joshmoore added a commit that referenced this pull request Mar 19, 2015
Disable graph and intensity pane for bigimages without stats
@joshmoore joshmoore merged commit 32a1cfb into ome:develop Mar 19, 2015
@jburel
Copy link
Member

jburel commented Mar 19, 2015

@dominikl: for float we should not use requiredPyramid but we could the isLarge method.
The only limitation is that the isLarge is arbitrary and not configured server side.

@dominikl
Copy link
Member Author

Ah, I didn't know that there is an isLarge method (guess it's ImViewerModel.isLargePlane()). Ok, I'll make use of this method in a follow-up PR, so that big float images behave like all big images (e. g. with respect to the preview panel).

@sbesson sbesson added this to the 5.1.0-m5 milestone Mar 23, 2015
@dominikl dominikl deleted the float_nostats_graphpane branch March 25, 2015 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants