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

Luts #160

Merged
merged 16 commits into from Aug 30, 2016

Conversation

Projects
None yet
5 participants
@will-moore
Member

will-moore commented Aug 3, 2016

This adds LUTs support to OMERO.figure.

To test:

  • click on channel color chooser drop-down and select "Lookup Tables".
  • This will open a dialog showing available LUTs
  • Click a LUT to preview it, then OK
  • Channel button AND slider should have LUT background.
  • This should also work to set the channel LUT when multiple images are selected, even if they have different LUTs/colors for that channel before setting LUT.
  • Export to PDF/TIFF should export images with LUTs applied as in the figure.

screen shot 2016-08-03 at 23 16 22

@gusferguson

This comment has been minimized.

Show comment
Hide comment
@gusferguson

gusferguson Aug 4, 2016

@will-moore

Tested with https://cowfish.openmicroscopy.org/webmerge/webclient/ user-3 read-only 1 Figure: Test 16-08-04 08:48:09

Selection of colours, LUT and clour picker behave as expected.

Question: @will-moore @jburel

  • using the left panel in the test figure I configured the top channel as blue, the second channel as sepia LUT and the third channel as green
  • when I select channel 1 and 3 with 2 deselected, the rendering is as expected - screenshot 1
  • but as soon as I select channel 2 (the sepia LUT) it seems to obscure channel 1 but channel 3 is visible - screenshot 2
  • if I make channel 2 a normal colour and channel 3 a LUT (grey in this screenshot) both channels 1 and 2 are not visible at all - screenshot 3
  • if I put the LUT on channel 1 then I can see all 3 channels - screenshot 4

It looks to me as if selecting an LUT for a channel causes all channels “below” it to be obscured - the graphics app metaphor of this would be - normal colour channels have a “transparent background” but the LUTs don’t.

I am getting the same effect with LUTs with the same image in the main Web app - although on some other images there seems to be a small degree of "show through" below LUTs.

Is this expected?

omero figure 0

omero figure 3

omero figure 4

omero figure 5

gusferguson commented Aug 4, 2016

@will-moore

Tested with https://cowfish.openmicroscopy.org/webmerge/webclient/ user-3 read-only 1 Figure: Test 16-08-04 08:48:09

Selection of colours, LUT and clour picker behave as expected.

Question: @will-moore @jburel

  • using the left panel in the test figure I configured the top channel as blue, the second channel as sepia LUT and the third channel as green
  • when I select channel 1 and 3 with 2 deselected, the rendering is as expected - screenshot 1
  • but as soon as I select channel 2 (the sepia LUT) it seems to obscure channel 1 but channel 3 is visible - screenshot 2
  • if I make channel 2 a normal colour and channel 3 a LUT (grey in this screenshot) both channels 1 and 2 are not visible at all - screenshot 3
  • if I put the LUT on channel 1 then I can see all 3 channels - screenshot 4

It looks to me as if selecting an LUT for a channel causes all channels “below” it to be obscured - the graphics app metaphor of this would be - normal colour channels have a “transparent background” but the LUTs don’t.

I am getting the same effect with LUTs with the same image in the main Web app - although on some other images there seems to be a small degree of "show through" below LUTs.

Is this expected?

omero figure 0

omero figure 3

omero figure 4

omero figure 5

@gusferguson

This comment has been minimized.

Show comment
Hide comment
@gusferguson

gusferguson Aug 4, 2016

@will-moore @jburel - LUTs in Insight also giving this obscuring effect.

gusferguson commented Aug 4, 2016

@will-moore @jburel - LUTs in Insight also giving this obscuring effect.

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel
Member

jburel commented Aug 4, 2016

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Aug 4, 2016

Member

Alpha is not taken into account when applying LUT (rendering engine), we may need to review that part

Member

jburel commented Aug 4, 2016

Alpha is not taken into account when applying LUT (rendering engine), we may need to review that part

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Aug 4, 2016

Member

@gusferguson: openmicroscopy/openmicroscopy@d57c42e fixes the "override" issue mentioned above

Member

jburel commented Aug 4, 2016

@gusferguson: openmicroscopy/openmicroscopy@d57c42e fixes the "override" issue mentioned above

@will-moore

This comment has been minimized.

Show comment
Hide comment
@will-moore

will-moore Aug 8, 2016

Member

To test today's commits:

  • Names of LUTs in picker dialog should not have '.lut', underscores should be replaced by space and name should start with capital letter.
  • If you Add a new image into OMERO.figure, any LUTs saved on that image from Insight/webclient are shown immediately.
  • Layout of channel sliders is fixed (as it was before this PR, instead of start/end labels squashed onto different rows).
Member

will-moore commented Aug 8, 2016

To test today's commits:

  • Names of LUTs in picker dialog should not have '.lut', underscores should be replaced by space and name should start with capital letter.
  • If you Add a new image into OMERO.figure, any LUTs saved on that image from Insight/webclient are shown immediately.
  • Layout of channel sliders is fixed (as it was before this PR, instead of start/end labels squashed onto different rows).
@snoopycrimecop

This comment has been minimized.

Show comment
Hide comment
@snoopycrimecop

snoopycrimecop Aug 8, 2016

Member

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

  • PR #152 will-moore 'open with images'
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js
Member

snoopycrimecop commented Aug 8, 2016

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

  • PR #152 will-moore 'open with images'
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js
@dominikl

This comment has been minimized.

Show comment
Hide comment
@dominikl

dominikl Aug 9, 2016

Member

The channel color buttons look a bit strange (black vertical line in the center).
screen shot 2016-08-09 at 07 57 49

But I don't have a proper local setup (no lookup tables available), might be due to that.

Member

dominikl commented Aug 9, 2016

The channel color buttons look a bit strange (black vertical line in the center).
screen shot 2016-08-09 at 07 57 49

But I don't have a proper local setup (no lookup tables available), might be due to that.

@dominikl

This comment has been minimized.

Show comment
Hide comment
@dominikl

dominikl Aug 9, 2016

Member

Ignore the last comment, I guess that's just because I can't get my local server properly setup for lookup tables.

Member

dominikl commented Aug 9, 2016

Ignore the last comment, I guess that's just because I can't get my local server properly setup for lookup tables.

@snoopycrimecop

This comment has been minimized.

Show comment
Hide comment
@snoopycrimecop

snoopycrimecop Aug 9, 2016

Member

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

  • PR #152 will-moore 'open with images'
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js
Member

snoopycrimecop commented Aug 9, 2016

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

  • PR #152 will-moore 'open with images'
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js

@will-moore will-moore referenced this pull request Aug 10, 2016

Merged

open with images #152

@snoopycrimecop

This comment has been minimized.

Show comment
Hide comment
@snoopycrimecop

snoopycrimecop Aug 10, 2016

Member

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

  • PR #152 will-moore 'open with images'
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js
Member

snoopycrimecop commented Aug 10, 2016

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

  • PR #152 will-moore 'open with images'
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js
@snoopycrimecop

This comment has been minimized.

Show comment
Hide comment
@snoopycrimecop

snoopycrimecop Aug 11, 2016

Member

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

  • PR #152 will-moore 'open with images'
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js
Member

snoopycrimecop commented Aug 11, 2016

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

  • PR #152 will-moore 'open with images'
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js

@snoopycrimecop snoopycrimecop referenced this pull request Aug 11, 2016

Closed

Rois from omero #165

6 of 6 tasks complete
@snoopycrimecop

This comment has been minimized.

Show comment
Hide comment
@snoopycrimecop

snoopycrimecop Aug 17, 2016

Member

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

  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
Member

snoopycrimecop commented Aug 17, 2016

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

  • PR #158 dominikl 'Label markdown'
    • static/figure/figure.js
  • Upstream changes
    • static/figure/figure.js
    • static/figure/js/views/modal_views.js
@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Aug 18, 2016

Member

Having the list of luts hard-coded in 2 different files is far from ideal.

Member

jburel commented Aug 18, 2016

Having the list of luts hard-coded in 2 different files is far from ideal.

@will-moore

This comment has been minimized.

Show comment
Hide comment
@will-moore

will-moore Aug 18, 2016

Member

@jburel: The list of LUTs is only hard-coded in one place. Remember that every commit for figure includes the concatenated JavaScript so every change is duplicated.

Member

will-moore commented Aug 18, 2016

@jburel: The list of LUTs is only hard-coded in one place. Remember that every commit for figure includes the concatenated JavaScript so every change is duplicated.

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Aug 18, 2016

Member

true, forget my comment

Member

jburel commented Aug 18, 2016

true, forget my comment

@snoopycrimecop snoopycrimecop referenced this pull request Aug 22, 2016

Merged

Inputs for sliders #166

@gusferguson

This comment has been minimized.

Show comment
Hide comment
@gusferguson

gusferguson Aug 23, 2016

@will-moore

Tested using https://cowfish.openmicroscopy.org/webmerge/figure/new/ Safari user-3

LUTs behaving as expected.

Good to merge.

gusferguson commented Aug 23, 2016

@will-moore

Tested using https://cowfish.openmicroscopy.org/webmerge/figure/new/ Safari user-3

LUTs behaving as expected.

Good to merge.

@jburel

This comment has been minimized.

Show comment
Hide comment
@jburel

jburel Aug 30, 2016

Member

Thanks merging so we have everything in place

Member

jburel commented Aug 30, 2016

Thanks merging so we have everything in place

@jburel jburel merged commit f9c68c7 into ome:master Aug 30, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment