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

Add support for setting indicator color #29

Merged
merged 17 commits into from
Dec 8, 2020
Merged

Add support for setting indicator color #29

merged 17 commits into from
Dec 8, 2020

Conversation

almarklein
Copy link
Collaborator

@almarklein almarklein commented Nov 30, 2020

  • Add color arg, eg VolumeSlicer(... color='red').
  • Draw a rectangle around the image in that color. (But only when other slicers are present.)
  • Other slicers draw indicators in that color.
  • Indicators are now across the image.
  • Decide on way to display the color of the current slicer. -> corners with slightly thicker lines.
  • What should the default color be -> different color for each axis based on a good colormap (e.g. D3).
  • Refactor to use one store less (combine "index" and "pos" into "state).
  • Also include (rate limited) axis ranges, so that the indicators can be drawn correctly when zoomed in.
  • Renamed some fields in the info store to avoid confusion with shape, sampling and origin.
  • Slicer colors are taken from a plotly colormap.
  • The create_overlay_data() method samples default colors from the same colormap.
  • If a colormap is given to create_overlay_data(), the first element is used where mask is 1 (was 0).
  • Update slicer_with_3_views to show slicer view ranges in the 3D figure.
  • Find actual figure size via gd._fullLayout.width / height?

afbeelding

@nicolaskruchten
Copy link

Hmm the border is a bit intense... I wonder if maybe just having the corners highlighted would be enough? Like an indicator line along both sides of every corner for 20 pixels or something?

@almarklein
Copy link
Collaborator Author

Hmm the border is a bit intense...

Yeah, I played a bit with slimmer lines, but certain colors would be barely visible then. I'll try the corners-thing.

@almarklein almarklein mentioned this pull request Dec 1, 2020
Closed
16 tasks
@emmanuelle
Copy link
Contributor

Maybe a matter of color and taste, but I'm fine with lines as they are now. And it helps a lot with the navigation, love this feature!!

@emmanuelle
Copy link
Contributor

What do you think @surchs? Are lines too thick or ok? We need more opinions :-).

@almarklein
Copy link
Collaborator Author

almarklein commented Dec 2, 2020

Added another possible approach to the top post.
edit: And also Nicolas' suggestion.

@surchs
Copy link
Contributor

surchs commented Dec 2, 2020

Nice, this looks very cool and makes it easier to use!
I like the lines as well with the current thickness. Maybe to denote the individual axis graphs, we could just use a thick line on the bottom, rather than a complete border. And maybe this "indicator" line should be optional and default to hidden when no slicer colour is defined (or they all default to turquoise).

@nicolaskruchten
Copy link

I think this is looking pretty great! The corner indicators could be a little smaller perhaps, and I personally preferred the "outer tick" style of indicators for the other two, that way they don't occlude the figure, but that's maybe just a personal style thing.

It would be awesome to get these indicators somehow reflected in the 3d view as well if possible in the example with the 3d plot :)

@almarklein
Copy link
Collaborator Author

@nicolaskruchten
afbeelding

@nicolaskruchten
Copy link

zomg that's cool

@almarklein almarklein marked this pull request as ready for review December 4, 2020 14:46
@almarklein
Copy link
Collaborator Author

Ready for review.

The corner-indicators are now actually square, and have the same size in each view targeting the same volume.

Quite a few changes, I also renamed some variables, and refactored a bit.

@almarklein
Copy link
Collaborator Author

Hold on - After discussion with @nicolaskruchten I'm going to try changing how we define the "indicator ranges".

@almarklein almarklein merged commit 9363a79 into main Dec 8, 2020
@almarklein almarklein deleted the color branch December 8, 2020 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants