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

Colorbar feature #605

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open

Colorbar feature #605

wants to merge 43 commits into from

Conversation

Tom-TBT
Copy link
Contributor

@Tom-TBT Tom-TBT commented Dec 9, 2024

This PR implements the optional display of a colorbar next to a panel. The colorbar shows the mapping between pixel values and LUT ("normal" color included). This is work from @MinaEnayat and me.
There are still adjustments to be made, but this is functional and mature enough that we thought it was ready to receive comments/ideas.

image

Implemented features:

  • colorbar display showing the LUT of the first active channel (opinion on what other behavior when more than 1 channel?)
  • The colorbar can be placed on one of the four sides of the panel
  • Parameters for:
    • thickness of the colorbar
    • spacing between colorbar and panel
    • number of ticks+labels
    • ticks+labels+spine color
    • tick length
    • label margin (distance between label and dash)
    • label font size
  • Crop page around panels handles horizontal labels (measure the CSS property of generated labels). Could be adapted for the handling of right/left normal labels
  • Export to pdf / tiff

A spine with the ticks and labels gives a visual clue when the LUT is the same color as the page background, but only on one side to avoid confusion with the LUT when the LUT is the same color as the spine:
image

Suggestions for tests:

  • Intensity range:
    • 0-255 (full range)
    • 40-190 (different start-end value)
    • 16-bit range
  • reversed channel flips the colorbar (not the labels)
  • uneven panel resizing (width/height without keeping the ratio).
  • Parameter testing thickness, spacing, label font, label margin, and tick length:
    • python export handles them all
    • "crop page around panel" handles them all
  • all tests above for the python export to tiff & pdf

Known issues / work left:

  • CSS display of ticks's dashes is uneven -> cannot be solved afaik (not the case with the exported figure)
  • Labels in python left justified. Make them the same way in JS
  • Labels in python do not always match the labels in JS (rounding difference)
  • Handle left/right normal labels for "crop page around panels"
  • labels are rounded according to the ticks spread: -log10(end-start / num_ticks)
  • Rearrange Colorbar form (adding/removing options ?)

MinaEnayat and others added 30 commits November 18, 2024 16:56
this.render();
});
this.loadLuts();
this.render();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change now calls this.render() before the luts have loaded. Haven't tested to see if this actually causes a bug, but I think this will be fixed with:

this.loadLuts().then(() => this.render());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That may indeed, I added your fix. Thanks

@will-moore
Copy link
Member

I wonder if you want to allow users to choose the channel for the colorbar (if you have more than one active)?
In this case, I'm most interested in the Green channel (but I still want the Red channel to be active):

Screenshot 2025-01-10 at 14 31 28

It's even possible to imagine someone wanting 2 or more colorbars at once, for different channels, but I guess that's a bit edge case (you would just do a split-view figure).
Actually, you can fake it by duplicating the panel and overlaying both panels, with the underneath panel having just one channel active! :)

Screenshot 2025-01-10 at 14 36 31

@Tom-TBT
Copy link
Contributor Author

Tom-TBT commented Jan 14, 2025

I wonder if you want to allow users to choose the channel for the colorbar (if you have more than one active)?

It's even possible to imagine someone wanting 2 or more colorbars at once

I assumed that colorbars only made sense with a single channel (especially if the channel uses a LUT).
With your example, red and green overlapping produce a new color (orange) in neither of the two colorbars.

I guess your trick is a good enough workaround if someone has the use case, but I wouldn't make it the normal "use case" to show one colorbar per active channel.

Instead of adding a parameter for the channel to show a colorbar for, I would add a doc/tooltip somewhere indicating that the first active channel is used for the colorbar.

@will-moore
Copy link
Member

I had to try quite hard to "break" this, but I managed to paste some invalid text into the "Spacing" field, which gave me figure JSON "spacing":null and kinda broke the layout as the spacing is used to layout a bunch of stuff. This is really not too important to fix, as it's very edge case. But it did also expose that the placeholder attribute for the "Spacing" field is still Height.

Screenshot 2025-01-30 at 15 36 00

Actually, I'm not sure that any of the number field placeholder attributes are really useful as strings since they're mostly not readable and a placeholder should really be a suggestion of a default value? (I see that I've done it the same way with the Scalebar fields too, but I think this is also incorrect. See https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/placeholder).

The form layout needs tweaking a bit to get all the input fields to the same size and to align them to the left. I think you need class="input-group-sm" on all the divs that wrap the inputs, and break the form into 3 separate "row" elements (currently there are just 2 and the 2nd one wraps).

As discussed last week, I think we are running out of space in the Labels right-hand tab, so that any Labels are often hidden and you need to scroll to see them. I'm wondering if we should add a 4th tab to the right panel Info, Preview, Labels, ??? but I'm not sure what to call it or what to put there (maybe Scalebar and Colorbar could go there). Maybe Inset and Border (and ROIs ???) can go too and just leave "Labels" in the Labels tab!?
Ideas welcome!

@will-moore
Copy link
Member

The LUT png is resized big enough to provide for bars of 50px thick, but if you go higher you get other ones showing. E.g. 100 wide:

Screenshot 2025-01-30 at 16 16 50

You could simply stretch the lut png a bit more, or limit the thickness. Probably both. NB: even with a thickness of 50 you get the next LUT creeping in as there's anti-aliasing at the join.
For most of the other parameters I think it's OK to allow users to choose crazy numbers if they want. With 1 "Tick" you get a NaN so maybe 2 is the minimum? Negative numbers don't make sense for most inputs, so maybe use min="0" or a higher min value if appropriate.

@will-moore
Copy link
Member

The PDF and TIFF export look great! Just maybe needs a thinker border line on the TIFF colorbar? Screenshot shows TIFF, PNG and figure:

Screenshot 2025-01-30 at 16 43 49

@will-moore
Copy link
Member

will-moore commented Jan 30, 2025

Minor issue with longer ticks in PDF and TIFF. These ticks are 20 long and go through the numbers.

With PDF and TIFF export, multi-page figures cause an issue. No ticks, numbers etc on the 2nd page:

Screenshot 2025-01-30 at 17 19 11

@will-moore
Copy link
Member

A couple more thoughts...

I tried batch editing 2 panels at once. In the previous screenshot, I selected the top 2 panels (blue and green) and wanted to make them consistent by choosing the same number of Ticks and Font size but not changing other properties such as position.
However, when I make a change to any of the properties, all of the colorbar properties are applied to both panels. I was kinda hoping that I could make changes to each property individually, as you can with channels.

Then I checked the behaviour for Scalebar and found that this doesn't work Scalebar - If you make a change to one property, it tries to apply ALL of them (same as Colorbar). In some ways, Scalebar is worse because for values that are not the same between selected panels E.g. Length - it shows NO value when those panels are selected together. The idea here is that the UI lets you know when these values are different between selected panels (which Colorbar doesn't do). BUT, when you edit a different property (E.g. Height) the Length:null gets saved to both panels!

So, it's probably best just to leave it as is. In the scenario above, I was actually happy to have all the properties consistent between the 2 colorbars, except for position. So I just had to re-select the first panel and reset it's colorbar position, which wasn't a big deal.

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.

3 participants