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

Handle missing min/max gracefully for sliders/scrollbars #32

Closed
imagejan opened this issue Oct 3, 2017 · 3 comments
Closed

Handle missing min/max gracefully for sliders/scrollbars #32

imagejan opened this issue Oct 3, 2017 · 3 comments

Comments

@imagejan
Copy link
Member

imagejan commented Oct 3, 2017

Currently, the UI hangs when trying to use a numeric parameter with style=slider, but min or max undefined:

#@ int (style=slider) a

This should be handled gracefully, by checking for softMin != null and softMax != null here:

// add optional widgets, if specified
if (model.isStyle(NumberWidget.SCROLL_BAR_STYLE)) {
int smx = softMax.intValue();
if (smx < Integer.MAX_VALUE) smx++;
scrollBar =
new JScrollBar(Adjustable.HORIZONTAL, softMin.intValue(), 1, softMin
.intValue(), smx);
scrollBar.setUnitIncrement(stepSize.intValue());
setToolTip(scrollBar);
getComponent().add(scrollBar);
scrollBar.addAdjustmentListener(this);
}
else if (model.isStyle(NumberWidget.SLIDER_STYLE)) {
slider =
new JSlider(softMin.intValue(), softMax.intValue(), softMin.intValue());
slider.setMajorTickSpacing((softMax.intValue() - softMin.intValue()) / 4);
slider.setMinorTickSpacing(stepSize.intValue());
slider.setPaintLabels(true);
slider.setPaintTicks(true);
setToolTip(slider);
getComponent().add(slider);
slider.addChangeListener(this);
}

If no slider or scroll bar can be rendered, we should fallback to the standard textfield/spinner combination and log a warning message.

@ctrueden
Copy link
Member

ctrueden commented Nov 3, 2017

I did some digging. The reason it freezes is not because min and max are null—that would throw an NPE after all. It's because min defaults to Integer.MIN_VALUE and max to Integer.MAX_VALUE. The UI builder then proceeds to try to make a slider or scroll bar with a massively huge range, and the CPU goes to 100% indefinitely. The problem is this and that lines of code in the DefaultWidgetModel.

A possible fix would be to change the code that creates the JScrollBar or JSlider to be more conservative about what it feeds to those constructors. This would also guard against the case of explicit min="-1000000000", max="1000000000" sorts of inputs.

@imagejan
Copy link
Member Author

imagejan commented Nov 3, 2017

Thanks for digging, @ctrueden!
I think we can tackle this at the same time as allowing non-integer step sizes in sliders (see #31), by adjusting the step size in case the number of possible steps gets unreasonably high (the acceptable limit might have to be found by testing with different settings).

@ctrueden
Copy link
Member

ctrueden commented Nov 3, 2017

I believe I have fixed the integer cases. Handling non-integers will be more involved, though.

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

No branches or pull requests

2 participants