Closed moreApi closed 8 months ago
I have now also played around with it a bit - and it's super nice! I do have however some minor complaints about the GUI:
And one more: When saving a transfer function, the file does not have an extension by default. How about .transferfunction
?
- I am not a fan of the borders for Display Range and Color Map. I feel they're both self-explanatory and also don't need a label, or at least not one that wastes so much space.
I am a big fan of them. They make clear what belongs together in a way whitespace could never acheive. I like them so much that I think they are worth the space investment. But most elements are contained in their own Jpanel so for sciView the borders may be adjusted as desired.
- The load and save buttons should just be called load/save , and should be aligned to the right
Since we have two different load/save buttons pairs very close to each other I prefere to have more speaking buttons.
- The control points for the color maps are often badly visible, could you change them to a filled dot, or a crosshair or something?
Since they are basically hand drawn this is not easily possible. We can create an issue so someone with too much time in the future can have a look.
Scaling is broken once the window goes below a certain size - likely something has a minimum size set. This is a problem when embedding this in sciview's inspector.
The problem are the buttons. But I am unable to change it so that the buttons stack, when there is not enoutgh space. Internet says MigLayout does not support this and I also did not got it to work with FlowLayout which should be perfect for it.
Also I dont agree with both codacy issues. plz ignore
I have added a commit 609d359c5f8c2cce7eb36e2a5f0c9824821ab010 that eliminates the final attenuation of color for volume rendering. I do not know where this comes from, but I don't see the point of it and it is, to my knowledge, not consistent with the mathematics of volume rendering.
I made a few little changes to the UI:
What do you think @moreApi and @aryaman-gupta?
What I'd like to do further is move the Load/Save buttons to the right, that also saves a bit of vertical space below the transfer function editor.
- the color of the control point's border is determined by the brightness of it's color, if it's dark, the border is white, otherwise it's black This is good.
Personal taste wise i prefere circles over boxes.
I am ok with the border alternative.
Free floating save and load buttons look desperate for some context and need more explanation for example by adding what they save and load to their label, especially the first pair.
@skalarproduktraum (general question: should we tag each other? I check the PR anyway if it pops up in my githup notifications.)
I played around some more and I think I like this solution best:
With regard to the tagging question, @moreApi, not in general, but as this was a specific question for you two, I decided to tag you ^^
So I think this is what I am halfway happy with now:
It seems though that loading colormaps does not work well, at least there's information missing when I load in one that I've created before.
Do @kephale or @ctrueden maybe have comments on the GUI?
It looks great.
One thing I was wondering about was adjusting the X-range of the histogram (posted here https://github.com/scenerygraphics/sciview/issues/558).
That is already the case. The x-axis is labeld wrong. Also one needs to refresh the histogram for a new scaling to take effect. refreshing is currently done by toggeling the histogram button
Edit: there was more than just the labels wrong. Fix is on the way
I fixed the old histogram code for spim sources and moved histogram generating code to their respective classes using the interface in a sensible way. Now this part should be more clean.
Also I changed the color of the histogram to a more visible if not pretty color. We should prefere function over form. (we could think about keeping the background simple a white if we want to keep gray histogram bars.) The save/load buttons for transferfunctions got a litte "TF". Also not the most pretty solution but if avoids confusing the user with contextless buttons.
So far I am done again.
Nice, thanks @moreApi! Next time, please post a screenshot of the current state as well.
I gotta say I am not a big fan of using "TF" as an abbreviation. What I have done now is implemented the load/save functionality we have under gear menu:
I think this is quite a nice solution, and also opened the possibility to reset the transfer function, because there's more space now. What do you think?
I was also not a fan of the "TF" but it was better than no context. BUT that cogwheel is awsome and solves all of our problems including space 👍🎆
I gotta say I am not a big fan of using "TF" as an abbreviation.
WTF? What TF?
The commit history is a bit screwed, because this is based on some code that got alreadz merged but the reference to those comit is lost, probatly because some rebasing or squashing.