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

Re changes #3155

Merged
merged 4 commits into from Nov 24, 2014
Merged

Re changes #3155

merged 4 commits into from Nov 24, 2014

Conversation

jburel
Copy link
Member

@jburel jburel commented Oct 30, 2014

This PR

  • Prevent user from passing value outside min/max range for double and float
  • Reset window interval is directly modified in DB

To test this PR

  • Modify start/end in DB. Save values outside the pixel type range.

@jburel jburel added the develop label Oct 30, 2014
@ghost
Copy link

ghost commented Oct 31, 2014

@jburel Which table in the DB has these settings?

@ghost
Copy link

ghost commented Oct 31, 2014

I'll need to double-check with you what's supposed to happen here, but the behaviour seems quite wrong, for float/double images (fine for uint8/16). I've imported several float and double images. The min and max appear to be clamped to the global min/max for each channel. This means I can't normalise the rendering settings across multiple images with differing global min/max. Example: Red is max=0.745. I can set this to 0.8, 1.0 or 2.5, or even 40000 but this isn't reflected in the rendering. I think this is far too restrictive, and imposes unreasonable limitations upon what a user would reasonably expect to do. I set these values directly in the UI (they save correctly) and by updating channelbinding in the DB; no effect in either case.

@ghost
Copy link

ghost commented Oct 31, 2014

I should also note that thumbnail generation for all pixel types is broken; not sure if this PR is specifically responsible, but it was working for all pixel types last time I checked a couple of weeks back.

@ghost
Copy link

ghost commented Oct 31, 2014

Test data on trout, user-4 flt/data.

@jburel
Copy link
Member Author

jburel commented Nov 1, 2014

@rleigh-dundee: I was limiting to min/max. I have removed that limitation. To be retested on monday.

@pwalczysko
Copy link
Member

Tested float and double images on trout merge user-4 Project fit.

Changed the inputend and inputstart values in the channelbinding table in the DB. This changed the values shown in Insight as expected.
Did changes in Insight as well, this showed up int the DB as expected.
Tried negative values and values higher than Max, both worked fine.
The rendering of the image changes as expected when these values are imposed.

Two minor issues:

  1. I can set the inputend lower than the inputstart in the DB. I cannot do it in Insight, but when I try to do so, and press Save to All (Save is disabled as expected), the both sliders on the channel in question go to the right hand side (see screenshot)
  2. When I set a value lower than Min or higher than Max, and then try to slide the sliders in Insight, they slide just between Min and Max again, although I just set the value manually. Is this what we want @jburel ?

screen shot 2014-11-03 at 11 57 37

@jburel
Copy link
Member Author

jburel commented Nov 17, 2014

@pwalczysko: sorry missed your comment

  1. I will see how we can prevent that. We mainly prevent it in the UI
  2. Second one look like a follow up problem that @dominikl was looking at. see https://trello.com/c/KwLoyXTn/87-rendering-follow-up

If the value is modified in DB directly, the values are
reset while applying setting.
@jburel
Copy link
Member Author

jburel commented Nov 18, 2014

@pwalczysko: The last commit will swap the start/end value when doing a Save all.

@pwalczysko
Copy link
Member

It works kind of fine.
But the last commit does not swap the values when doing a Save all. Instead, it changes the last edited value to the value before editing.

@pwalczysko
Copy link
Member

I think this is acceptable behaviour though.

@jburel
Copy link
Member Author

jburel commented Nov 19, 2014

this is very much a corner case

@jburel
Copy link
Member Author

jburel commented Nov 24, 2014

@pwalczysko: okay to merge?

@pwalczysko
Copy link
Member

Okay to merge

@jburel
Copy link
Member Author

jburel commented Nov 24, 2014

Thanks all. merging the PR, which is part of m2

jburel added a commit that referenced this pull request Nov 24, 2014
@jburel jburel merged commit 09c1b4e into ome:develop Nov 24, 2014
@sbesson sbesson added this to the 5.1.0-m2 milestone Nov 26, 2014
@jburel
Copy link
Member Author

jburel commented Nov 26, 2014

--no-rebase

@jburel jburel deleted the re-changes branch February 19, 2015 11:48
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

3 participants