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

Reverse intensity #219

Merged
merged 13 commits into from Jun 23, 2017

Conversation

Projects
None yet
4 participants
@will-moore
Member

will-moore commented Jun 13, 2017

This adds support for Reverse intensity to figure - see https://trello.com/c/11hiGrmx/13-reverse-intensity-codomain

We also update the luts_10.png with the Janelia LUTs. NB: this is still hard-coded since changing that is more work and will mean that OMERO.figure will rely on openmicroscopy/openmicroscopy#5318 (OMERO 5.3.3 at least).
Can do this in a future PR/release.

This also moves the channel-toggle buttons down beside the sliders, to align OMERO.figure with our other clients. This frees up more space for other buttons beside the image, such as 'flip'. cc @carandraug.

Includes a fix for toggling channels bug: https://trello.com/c/yWFPR9MJ/160-bug-toggle-channels

screen shot 2017-06-13 at 15 21 30

To test:

  • Import images into figure, including some with channels that are saved with reverseIntensity.
  • Should appear same in figure (with reverse intensity enabled where appropriate).
  • Sliders should have gradient for plain colour channels (no LUT)
  • LUT and colour gradient should be reversed with reverse intensity
  • If multiple images are selected, channel should be displayed as 'reverse intensity' only if channel is reversed in ALL images.
  • Color-picker dropdown option 'Reverse Intensity' should be checked if all selected images are reversed.
  • Clicking the 'Reverse Intensity' option should turn it on/off.
  • Layout should look OK as below and other channel controls and rotation/z-projection should be unaffected.
  • Export of figure should respect reverse intensity settings.
@carandraug

This comment has been minimized.

Show comment
Hide comment
@carandraug

carandraug Jun 13, 2017

Contributor

I have been trying this to gain space for the flip button, so thank you.

Maybe rename src/templates/channel_toggle_template.html to src/templates/transformations_template.html since there's no channel toggle left there?

Contributor

carandraug commented Jun 13, 2017

I have been trying this to gain space for the flip button, so thank you.

Maybe rename src/templates/channel_toggle_template.html to src/templates/transformations_template.html since there's no channel toggle left there?

@snoopycrimecop

This comment has been minimized.

Show comment
Hide comment
@snoopycrimecop

snoopycrimecop Jun 13, 2017

Member

Conflicting PR. Removed from build FIGURE-merge#465. See the console output for more details.
Possible conflicts:

  • PR #213 will-moore 'slider range fix'
    • src/js/views/channel_slider_view.js
Member

snoopycrimecop commented Jun 13, 2017

Conflicting PR. Removed from build FIGURE-merge#465. See the console output for more details.
Possible conflicts:

  • PR #213 will-moore 'slider range fix'
    • src/js/views/channel_slider_view.js
@snoopycrimecop

This comment has been minimized.

Show comment
Hide comment
@snoopycrimecop

snoopycrimecop Jun 13, 2017

Member

Conflicting PR. Removed from build FIGURE-merge#466. See the console output for more details.
Possible conflicts:

  • PR #213 will-moore 'slider range fix'
    • src/js/views/channel_slider_view.js
Member

snoopycrimecop commented Jun 13, 2017

Conflicting PR. Removed from build FIGURE-merge#466. See the console output for more details.
Possible conflicts:

  • PR #213 will-moore 'slider range fix'
    • src/js/views/channel_slider_view.js
@snoopycrimecop

This comment has been minimized.

Show comment
Hide comment
@snoopycrimecop

snoopycrimecop Jun 13, 2017

Member

Conflicting PR. Removed from build FIGURE-merge#467. See the console output for more details.
Possible conflicts:

  • PR #213 will-moore 'slider range fix'
    • src/js/views/channel_slider_view.js
Member

snoopycrimecop commented Jun 13, 2017

Conflicting PR. Removed from build FIGURE-merge#467. See the console output for more details.
Possible conflicts:

  • PR #213 will-moore 'slider range fix'
    • src/js/views/channel_slider_view.js
@carandraug

This comment has been minimized.

Show comment
Hide comment
@carandraug

carandraug Jun 14, 2017

Contributor

There's an issue if the image is on a server that does not support
reversing the colours (the demo site uses images from the JCB which
does not, and the current IDR also does not). In such cases, the
reverse option still appears ticked, the slider also reverses the
colours, but the image stays the same. That can be a bit confusing.
Maybe a message could be displayed the same way it does when trying to
pick a LUT?

screenshot from 2017-06-14 15-40-12

Also, most programs call this invert instead of revert. ImageJ, Icy,
and Imaris, all programs for microscope image analysis only have
'Invert'. Both GIMP and Photoshop also use 'Invert'. GraphicsMagick
and ImageMagick use 'negate'. So maybe rename "Reverse Intensity"
to "Invert"?

For what is worth, the colours on the slider do not show up at all on
Firefox ESR 45.9 which is the latest on Debian stable but not for much
longer (the last ESR was released on March and a new Debian stable is
planned for release this Friday).

screenshot from 2017-06-14 15-36-53

Contributor

carandraug commented Jun 14, 2017

There's an issue if the image is on a server that does not support
reversing the colours (the demo site uses images from the JCB which
does not, and the current IDR also does not). In such cases, the
reverse option still appears ticked, the slider also reverses the
colours, but the image stays the same. That can be a bit confusing.
Maybe a message could be displayed the same way it does when trying to
pick a LUT?

screenshot from 2017-06-14 15-40-12

Also, most programs call this invert instead of revert. ImageJ, Icy,
and Imaris, all programs for microscope image analysis only have
'Invert'. Both GIMP and Photoshop also use 'Invert'. GraphicsMagick
and ImageMagick use 'negate'. So maybe rename "Reverse Intensity"
to "Invert"?

For what is worth, the colours on the slider do not show up at all on
Firefox ESR 45.9 which is the latest on Debian stable but not for much
longer (the last ESR was released on March and a new Debian stable is
planned for release this Friday).

screenshot from 2017-06-14 15-36-53

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Jun 14, 2017

Member

cc @waxenegger for name suggestion.
If we go for this option we will need to synchronize all the apps and the main web client
So probably for 5.4 then do all the apps

FYI @carandraug: omero does not run on debian 9 https://www.openmicroscopy.org/community/viewtopic.php?f=5&t=8295

Member

jburel commented Jun 14, 2017

cc @waxenegger for name suggestion.
If we go for this option we will need to synchronize all the apps and the main web client
So probably for 5.4 then do all the apps

FYI @carandraug: omero does not run on debian 9 https://www.openmicroscopy.org/community/viewtopic.php?f=5&t=8295

@will-moore

This comment has been minimized.

Show comment
Hide comment
@will-moore

will-moore Jun 14, 2017

Member

@carandraug We really only support install of latest OMERO.figure on 5.3.x (the message you saw above was added before we made that decision) and the loading of images from other servers is still mostly a demo feature. It would be quite a bit more work to handle the UI for e.g. selecting multiple images, some have reverseInstensity: True, some False and some not-supported, but I'll bear it in mind if we want to improve the handling of external images in future.

Member

will-moore commented Jun 14, 2017

@carandraug We really only support install of latest OMERO.figure on 5.3.x (the message you saw above was added before we made that decision) and the loading of images from other servers is still mostly a demo feature. It would be quite a bit more work to handle the UI for e.g. selecting multiple images, some have reverseInstensity: True, some False and some not-supported, but I'll bear it in mind if we want to improve the handling of external images in future.

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel
Member

jburel commented Jun 15, 2017

@will-moore will-moore referenced this pull request Jun 20, 2017

Merged

Fix demo build #217

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Jun 21, 2017

Member

It is not really obvious that we need to click on "Reverse Intensity" so it can take effect
It does not appear as a button or checkbox but more like text

Member

jburel commented Jun 21, 2017

It is not really obvious that we need to click on "Reverse Intensity" so it can take effect
It does not appear as a button or checkbox but more like text

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Jun 21, 2017

Member

The option is now tagging into account in the script

Member

jburel commented Jun 21, 2017

The option is now tagging into account in the script

@will-moore

This comment has been minimized.

Show comment
Hide comment
@will-moore

will-moore Jun 23, 2017

Member

With the last commit, the options look like this:

screen shot 2017-06-23 at 10 15 37

Member

will-moore commented Jun 23, 2017

With the last commit, the options look like this:

screen shot 2017-06-23 at 10 15 37

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Jun 23, 2017

Member

That's better with the checkbox

Member

jburel commented Jun 23, 2017

That's better with the checkbox

@jburel jburel merged commit a1a2a2e into ome:master Jun 23, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jburel jburel added this to the 3.1.0 milestone Jun 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment