Skip to content

CSSTUDIO-3538 Bugfix: Redraw Linear Meter widget when the range or alarm limits are updated #3649

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.

Sign up for GitHub

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

Merged
abrahamwolk merged 1 commit into from
Nov 17, 2025
Merged

CSSTUDIO-3538 Bugfix: Redraw Linear Meter widget when the range or alarm limits are updated #3649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions ...c/main/java/org/csstudio/display/extra/widgets/linearmeter/LinearMeterRepresentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ protected void registerListeners()
boolean validRange = Double.isFinite(new_value) && Double.isFinite(model_widget.propMaximum().getValue());
meter.setRange(new_value, model_widget.propMaximum().getValue(), validRange);
if (toolkit.isEditMode() && validRange) {
meter.setCurrentValue((new_value + model_widget.propMaximum().getValue()) / 2.0);
meter.setCurrentValue((new_value + model_widget.propMaximum().getValue()) / 2.0, true);
}
});

Expand All @@ -183,7 +183,7 @@ protected void registerListeners()
boolean validRange = Double.isFinite(new_value) && Double.isFinite(model_widget.propMinimum().getValue());
meter.setRange(model_widget.propMinimum().getValue(), new_value, validRange);
if (toolkit.isEditMode() && validRange) {
meter.setCurrentValue((new_value + model_widget.propMinimum().getValue()) / 2.0);
meter.setCurrentValue((new_value + model_widget.propMinimum().getValue()) / 2.0, true);
}
});

Expand Down Expand Up @@ -295,6 +295,7 @@ private void layoutChanged(WidgetProperty<?> property, Object old_value, Object
private void valueChanged(WidgetProperty<?> property, Object old_value, Object new_value) {
if (new_value instanceof VDouble) {
meter.withWriteLock(() -> {
boolean linearMeterScaleHasChanged = false;
VDouble vDouble = ((VDouble) new_value);
double newValue = vDouble.getValue();

Expand Down Expand Up @@ -327,6 +328,7 @@ private void valueChanged(WidgetProperty<?> property, Object old_value, Object n
&& displayRange.getMaximum() - displayRange.getMinimum() > 0.0) {
if (meter.linearMeterScale.getValueRange().getLow() != displayRange.getMinimum() || meter.linearMeterScale.getValueRange().getHigh() != displayRange.getMaximum() || !meter.getValidRange()) {
meter.setRange(displayRange.getMinimum(), displayRange.getMaximum(), true);
linearMeterScaleHasChanged = true;
}
} else {
Range controlRange = display.getControlRange();
Expand All @@ -336,12 +338,15 @@ private void valueChanged(WidgetProperty<?> property, Object old_value, Object n
&& controlRange.getMaximum() - controlRange.getMinimum() > 0.0) {
if (meter.linearMeterScale.getValueRange().getLow() != controlRange.getMinimum() || meter.linearMeterScale.getValueRange().getHigh() != controlRange.getMaximum() || !meter.getValidRange()) {
meter.setRange(controlRange.getMinimum(), controlRange.getMaximum(), true);
linearMeterScaleHasChanged = true;
Copy link
Contributor

@jacomago jacomago Nov 17, 2025

Choose a reason for hiding this comment

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

This makes me feel like this should be a property of the linearmeter, so this should be something like:

meter.setScaleUpdated()

Copy link
Collaborator Author

@abrahamwolk abrahamwolk Nov 17, 2025

Choose a reason for hiding this comment

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

I disagree: I prefer explicit passing of values (in contrast to implicit passing of values through update of state). In particular, linearMeterScaleHasChanged is passed to meter.setCurrentValue() on line 386: https://github.com/ControlSystemStudio/phoebus/pull/3649/files/c37e695e063c8014201fceb374da86198dbf2aa3#diff-99f66c9c7631b64fc8906945f212e452266b923ac7c5a7214eb7f604df210cbfR386

}
} else if (newObservedMinAndMaxValues && !Double.isNaN(observedMin) && !Double.isNaN(observedMax)) {
meter.setRange(observedMin - 1, observedMax + 1, false);
newObservedMinAndMaxValues = false;
linearMeterScaleHasChanged = true;
} else if (meter.linearMeterScale.getValueRange().getLow() != 0.0 || meter.linearMeterScale.getValueRange().getHigh() != 100) {
meter.setRange(0.0, 100.0, false);
linearMeterScaleHasChanged = true;
}
}
}
Expand All @@ -352,10 +357,12 @@ private void valueChanged(WidgetProperty<?> property, Object old_value, Object n
if (warningRange != null) {
if (!Double.isNaN(warningRange.getMinimum()) && meter.getLow() != warningRange.getMinimum()) {
meter.setLow(warningRange.getMinimum());
linearMeterScaleHasChanged = true;
}

if (!Double.isNaN(warningRange.getMaximum()) && meter.getHigh() != warningRange.getMaximum()) {
meter.setHigh(warningRange.getMaximum());
linearMeterScaleHasChanged = true;
}
}
}
Expand All @@ -365,16 +372,18 @@ private void valueChanged(WidgetProperty<?> property, Object old_value, Object n
if (alarmRange != null) {
if (!Double.isNaN(alarmRange.getMinimum()) && meter.getLoLo() != alarmRange.getMinimum()) {
meter.setLoLo(alarmRange.getMinimum());
linearMeterScaleHasChanged = true;
}

if (!Double.isNaN(alarmRange.getMaximum()) && meter.getHiHi() != alarmRange.getMaximum()) {
meter.setHiHi(alarmRange.getMaximum());
linearMeterScaleHasChanged = true;
}
}
}
}
}
meter.setCurrentValue(newValue);
meter.setCurrentValue(newValue, linearMeterScaleHasChanged);
});
}
}
Expand Down
14 changes: 7 additions & 7 deletions ...nearmeter/src/main/java/org/csstudio/display/extra/widgets/linearmeter/RTLinearMeter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ public void setShowWarnings(boolean showWarnings) {
private boolean isValueWaitingToBeDrawn = false;
private double valueWaitingToBeDrawn = Double.NaN;
/** @param newValue Current value */
public void setCurrentValue(double newValue)
public void setCurrentValue(double newValue, boolean forceRedraw)
Copy link
Contributor

@jacomago jacomago Nov 17, 2025

Choose a reason for hiding this comment

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

I'm not a fan of boolean flags, I would write an enum

enum DrawAction {
ForceRedraw
Draw
NoRender
}

or something like this.

Copy link
Collaborator Author

@abrahamwolk abrahamwolk Nov 17, 2025

Choose a reason for hiding this comment

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

An enum could be used, but since the data is still essentially only true and false and the parameter name of meter.setCurrentValue() describes the meaning well, I prefer to use a boolean.

{
withWriteLock(() -> {
valueWaitingToBeDrawn = newValue;
Expand All @@ -746,7 +746,7 @@ public void setCurrentValue(double newValue)
else {
isValueWaitingToBeDrawn = true;

drawNewValue(valueWaitingToBeDrawn);
drawNewValue(valueWaitingToBeDrawn, forceRedraw);
isValueWaitingToBeDrawn = false;
lag = false;
}
Expand Down Expand Up @@ -777,7 +777,7 @@ private Optional<Integer> computeIndicatorPosition(double value) {
});
}

private void drawNewValue(double newValue) {
private void drawNewValue(double newValue, boolean forceRedraw) {
Copy link
Contributor

@jacomago jacomago Nov 17, 2025

Choose a reason for hiding this comment

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

Shouldn't this method be renamed now?

Copy link
Collaborator Author

@abrahamwolk abrahamwolk Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No: the method draws a new value. Before this pull request, it was "optimized too much": drawNewValue() only actually drew a new value if it determined that the indicator had moved at least one pixel, which didn't account for the case where the range or the alarm limits had changed.

withWriteLock(() -> {
WARNING oldWarning = determineWarning();
AtomicReference<Double> newValueAtomicReference = new AtomicReference<>(newValue); // Workaround, since captured variables need to be effectively final in Java.
Expand All @@ -795,15 +795,15 @@ private void drawNewValue(double newValue) {
WARNING newWarning = determineWarning();
logNewWarningIfDifferent(oldWarning, newWarning);

if (oldValue != newValueAtomicReference.get()) {
if (!Double.isNaN(newValueAtomicReference.get())){
if (forceRedraw || oldValue != newValueAtomicReference.get()) {
if (forceRedraw || !Double.isNaN(newValueAtomicReference.get())){
Optional<Integer> maybeNewIndicatorPosition = computeIndicatorPosition(newValue);
boolean indicatorPositionHasChanged = maybeNewIndicatorPosition.isPresent() != maybeOldIndicatorPosition.isPresent() || maybeOldIndicatorPosition.isPresent() && maybeNewIndicatorPosition.isPresent() && maybeOldIndicatorPosition.get() != maybeNewIndicatorPosition.get();
if (indicatorPositionHasChanged || determineWarning() != newWarning) {
if (forceRedraw || indicatorPositionHasChanged || determineWarning() != newWarning) {
redrawIndicator(newValueAtomicReference.get(), newWarning);
}
}
else if (!Double.isNaN(oldValue)) {
else if (forceRedraw || !Double.isNaN(oldValue)) {
redrawIndicator(newValueAtomicReference.get(), newWarning);
}
}
Expand Down