New UI to allow the user to modify the stretch being used.
ClosedPublic

Authored by murveit on Dec 1 2019, 8:12 AM.

Details

Summary

In fitsviewer, the toggle stretch button is removed and replaced with a
stretch bar at the bottom of the window. It is always there and allows
the user to turn on/off stretching, use the automatically generated stretch
or modify the stretch parameters. As before, this is a display-only change
to the image, it does not modify the image itself.

Test Plan

In kstars, open a new fits file with the auto-stretch option either set
or not, and see if it displays accordingly. Then switch the other way
using the UI and play with the sliders to adjust the stretch. Try another
image also. Go back to the first image, it should remember the parameters
that were last used. Should work similarly with images from capture.
Evaluate the UI.

Diff Detail

Repository
R321 KStars
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
murveit created this revision.Dec 1 2019, 8:12 AM
Restricted Application added a project: KDE Edu. · View Herald TranscriptDec 1 2019, 8:12 AM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
murveit requested review of this revision.Dec 1 2019, 8:12 AM

Thank you Hy! A few notes:

  1. Maybe we should have labels for shadows/midtones/highlights? or icons before the sliders?
  2. The Auto & Stretch checkboxes should probably be replaced by toggable icon buttons to make it visually consistent with the rest of the UI.

I'll need to test the actual functionality soon, but great job!

My first observation is that I am very impressed! This is an awesome and sorely needed functionality and you did it so fast!!!

My second observation is that I don't know what these sliders are doing. Yes Jasem is right, labels and/or icons are great. Are these black point, white point, and midtones sliders? Or are they highlights and shadows sliders? Or are they contrast sliders for highlights midtones and shadows?

I'm not sure if this is possible, but one nice thing a number of photo programs do is give feedback in the histogram as you drag sliders like these showing you how it is changing, like showing the histogram updating automatically as you go, and also maybe showing the cutoff points if that is being adjusted.

And a serious issue: Your interface works wonderfully on data with integer numbers of counts, but fails when given doubles or floats. Try opening a stacked image after processing it in pixinsight. If you move the sliders back and forth it alternates between all black and all white instead of slowly changing the image as it does with a raw file.

murveit updated this revision to Diff 70690.Dec 1 2019, 6:11 PM

Fix bug that hid the labels.

Jasem & Rob, thanks for the initial comments.

Ooops, there was a bug that hid the labels--they were there but in visible. Fixed.

I am tempted, though, to remove the highlights section. My assumption is that would be rarely used and eats up real estate for the other sliders.

I wanted to avoid the histogram for now, though we can revisit. It would eat up CPU, and I think since this a visual, non precise thing, we're ok with just sliders.

I'll look into the floating point issue, though probably not today/tomorrow. I can make my own example, but if you have a .fit file that is problematic, please put it somewhere where I can test with it. I did not test with float input. I assume you mean to just save a .fit as float-32bit, whether or not it is stacked.

BTW, I see the problem with the floating point file.
The issue is that I need to normalize the number--the values needed to stretch are 1/64K as big as the 64K-scale-shorts, and so they underflow the 10000 scale I was using.
I will address this in the next couple days.

Very nice work, Hy, many thanks!!
I fully agree to that what Jasem and Robert have said.

Maybe another thing: for me either the "Auto" checkbox or the "Stretch" checkbox is redundant. When "Auto" is selected, "Stretch" gets selected. And vice versa, if "Stretch" is deselected, "Auto" gets deselected as well.

Another question, why does the image get grainy when dragging the sliders? Is that to reduce the memory or processor load required? Or maybe to speed it up?

murveit updated this revision to Diff 70709.Dec 2 2019, 12:13 AM

Fixed issue when stretching float or double fits file.

Thanks again for testing and checking out the interface.

I have fixed the issue with floating input. Please re-test.

To address some of the other comments:

  • Buttons vs CheckBox: I can happily move to buttons. FYI, the reason I did it as I did was this way it is obvious whether stretching is on or off.

With buttons, I'm  not sure whether a highlighted button means on or off. Further, if I change the label to say "Stretch" or "Unstretched", which
would you use when it is stretched? It could mean the current state, or it could mean the state you want after the button is pushed.
Anyway, let me know what you want specifically and I'll implement that.

  • Grainy while using the slider: Yes, I reduce the resolution (see e.g. fitstab.cpp, line 223, view->setSampling(4))

while dragging to speed up the computation. Otherwise it can get laggy. I could make the sampling a function of the
image size and # of channels, but perhaps that's fine tuning that should be done in a follow-up submission.

  • Auto vs Stretch: I see that can be confusing. They really are two different things, and you should be able to have one

checked without the other. E.g. If you have the auto-stretch setting on, then when you load an image, you should see both
checked.If you move the sliders, then Auto should go unchecked, as you've manually overridden the automatically generated
parameters. If you then uncheck Stretch, then both would be unchecked and no stretching done. If you re-check stretch,
then auto should stay unchecked, as it will remember the previous manual setting. Let me know if it doesn't work that way
for you. It is true, though, that if Stretch is unchecked, and you check Auto, both checks will appear, as setting Auto, to me,
implies that you want to Stretch. Again, if this is confusing, let me know if you see a better way.

Please let me know if you see other issues or ways to improve.
Hy

That does look much better. I am uncertain as to whether highlights is needed or not, we should try it on some bright planet or moon images before getting rid of it.

One way to alleviate some confusion between auto and stretch would be to have the "stretch" button enable/disable all the other controls, then one can get a visual cue that the stretch is gone other than just a checkbox. The controls can still remember their values, but you can't change them until you enable stretch.

Another thing that could help would be to have the controls be at an absolute position and hitting auto will visually make them go to that spot in the slider. Also moving them to a spot will make the slider stay at that spot. I understand the utility of having the slider work the way you have done it for more dynamic control, but it is a little disconcerting at first to move the slider, have it change the image as you go, and then it snaps back to center when you let go.

oh related to what I just said, turning on "auto" does not change the values on the sliders. is it supposed to?

oh one more question. What will be done with the fits filters under the view menu?

Rob, thanks for all the suggestions and questions. Let me respond before I make any code changes.

  • One way to alleviate some confusion between auto and stretch would be to have the "stretch" button enable/disable all the other controls, then one can get a visual cue that the stretch is gone other than just a checkbox. The controls can still remember their values, but you can't change them until you enable stretch.

I'll try and code that up. Should be easy assuming all the input elements have a disable method.

  • Another thing that could help would be to have the controls be at an absolute position and hitting auto will visually make them go to that spot in the slider. Also moving them to a spot will make the slider stay at that spot. I understand the utility of having the slider work the way you have done it for more dynamic control, but it is a little disconcerting at first to move the slider, have it change the image as you go, and then it snaps back to center when you let go.

Sorry I don't really understand your suggestion fully.  I do think, though, that if we went to absolute sliders, we'd need a way to change to the scale, like in PI's histogram transform, and the user interface will get more complex as it is there. Honestly I think that UI is hard to understand. I sort of like the simplicity of this, and would like to push back a little on changing this as I think people will get used to it, but would give in, of course, if you all felt strongly about it. It's possible that we'd have more room for absolute sliders if we got rid of highlights, but I started with an absolute slider, and wound up often playing with very small slider moves near the edge of the slider, which wasn't good.

  • oh related to what I just said, turning on "auto" does not change the values on the sliders. is it supposed to?

When you hit auto, it auto-computes the stretch parameters given the algorithm pointed to in the code (see sections 8.5.6 and 8.5.7 in http://pixinsight.com/doc/docs/XISF-1.0-spec/XISF-1.0-spec.html). When you load an image with stretch enabled, it does the same thing, so you may be recomputing the same same parameters. If you enable stretch with auto, then manually change the settings by moving . slider, then hit check auto again, you should see the numbers change. Note, the slider handles are alway set to the middle of the slider, so that will not move.

  • What will be done with the fits filters under the view menu?

I really am looking for you guys for guidance. I never used those filters, so from my point-of-view, I'd get rid of them. I hesitated to do that, since I'm not as experienced with this software. BTW, I'd add the histogram controls to your suggestion.

murveit updated this revision to Diff 70715.Dec 2 2019, 4:45 AM

Disable most of the stretch UI when stretching is disabled.

I implemented Rob's suggestion to disable the sliders and their labels and the auto checkbox when stretching is disabled.
I think this was a good idea.

FWIW, it does have a downside too--previously one could just move sliders without enabling anything to get started manually stretching.
Now you have to turn on stretching explicitly.

I'm not sure you understand my comment about how auto didn't change the sliders. What I did is I hit auto first, noted the numbers for the sliders, then I played with one of the sliders, noted the new number it displayed, then I hit auto again. The number displayed on the slider was not the one that was shown when I hit auto the first time, but it instead showed the last number I had gotten when playing with the slider. Auto clearly changed the image, but it didn't change the info on the slider.

So my suggestion for your downside: I believe you have stretch and auto enabled by default correct? So then all should be enabled, it would only be disabled if they uncheck stretch so it shows a linear image.

you answered my question about the absolute sliders adequately. I can see how the absolute slider would not be easy to use because of the fine adjustments required. It would still be nice to have some visual information about where in the range the current value happens to be, but that doesn't have to be in the form of an absolute slider.

murveit updated this revision to Diff 70719.Dec 2 2019, 6:59 AM

Fix bug related to updated slider text value when auto is checked.

Rob: Thanks for describing the bug related to the slider value display. I was able to reproduce and fix. You should not see that behavior anymore.

At this point I know Rob would like to see more visual feedback somehow related to the slider value.
I'm hoping we can delay that, as I don't have a clear idea of what to do, and I think this interface as it stands can work well.
So, if you're willing, could we try the current way, and then I'd be happy to add something else if we think of an improvement?

Jasem: I know you said you'd prefer push-buttons to check-boxes.
Happy to do that, if you still feel that way. Please let me know what text for the buttons.
One possibility is:

  • When stretch is enabled: "Stretching" and the button would be highlighted.
  • When stretch is not enabled: Not Stretching" and button is not highlighted.

Honestly, though, I think the current stretch checkbox is clearer.

Please let me know if there any other issues I haven't addressed.
Hy

I think its much improved!!

Great progress Hy. There isn't going to be any text, it's just buttons. If you used PixInsight before, many buttons on the toolbar are similar. Also, in KStars toolbar as well. Users would learn the context of the buttons, maybe a tooltip would help, but we should reserve as much space as possible, and use small icons to trigger this. You can search from the breeze icons which ones best suit these particular tasks.

Here are some screenshots of similar interfaces from PixInsight and Luminar.

murveit updated this revision to Diff 70815.Dec 3 2019, 9:31 AM

Moved from QCheckBox to QPushButton for the stretch and auto buttons.
Auto button is only enabled when it makes sense.
Sorry, the blank auto-button is a little wider when disabled,
couldn't figure out how to avoid that.

I just tested it. the left one works great! The auto button has an issue though. When auto is selected, its blank, and then when you move a slider, the button appears. Is it supposed to be no button at all when auto is selected?

Rob, That's working as intended. I clear the button and disabled it when selecting it didn't make sense. It only appears when there's something to do.
I tried it the way you suggest and it was confusing--the user could hit the button and nothing really would happen (e.g. I made an info pop-up, but in the end, I though the better thing to do was this).

This comment was removed by lancaster.

Doesn't it usually become a different color when disabled like a gray color? That is usually the better way.

The difference is subtle and IMHO it would not be obvious to many users.
I think this is much clearer.
You can try it by disabling the clear--comment out line 184 in fitstab.cpp

Nice work Hy! This is impressive. Just tested it! Couple of comments:

  1. Left button has tooltip "Automatically find...etc". It shouldn't have this tooltip when it is not active.
  2. Add small arrow key to hide/show these controls? Maybe some users don't want to see it there all the time?
murveit updated this revision to Diff 71062.Dec 7 2019, 7:52 PM

Adjusted stretch button tooltips depending on their state.

Thanks for the review and testing Jasem.
I addressed your tooltip-concern in my latest update.
I don't quite understand how to respond to your other concern (re hiding the contols).

Here's my confusion: It's true that I'm taking a bit of space at the bottom of the display (as little as I could, but some space for sure).
If I added a button to hide the controls and put it on the bottom near the other controls, it would wind up taking the same vertical space,
so I don't see a benefit there.
I could put the hide button on the top bar, away from the other stretch buttons, where the previous stretch button was.
There it seems redundant to the new bottom left button, and the physical distance could be confusing, but I suppose I could do that.
It could be exactly the same as the new bottom-left-button, but then there would be no way of hiding the controls when stretch is
active. In the end I worry that this hiding makes things harder to understand.
What do you recommend?

The hiding is not strictly necessary and can come later.. it's just to make the interface as clean as possible. Thanks, will merge this now!

mutlaqja accepted this revision.Dec 7 2019, 9:00 PM
This revision is now accepted and ready to land.Dec 7 2019, 9:00 PM
lancaster accepted this revision.Dec 8 2019, 12:32 AM
This revision was automatically updated to reflect the committed changes.