From 916203ea35e51d9560ee41087bb159107713cd24 Mon Sep 17 00:00:00 2001 From: Emilio Heredia Date: Thu, 19 Mar 2026 15:45:54 -0600 Subject: [PATCH 1/7] refactor(rtplot): fix tick layout for narrow axes; add RTTank dual-scale and outline LogTicks was producing overlapping labels on narrow widgets (such as Tank). Rework the tick-thinning logic so sub-decade ticks (2..9) are suppressed when available pixel height is too small, falling back to power-of-ten labels only. LinearTicks receives a matching minor adjustment for very small ranges. AxisPart gains a minor helper used by the new tank layout. YAxisImpl gains a force_text_up flag (default false). When true the tick labels always read bottom-to-top regardless of the is_right setting. This is needed by RTTank's right-side scale, which is a true YAxisImpl instance set to is_right=true; without force_text_up its labels read top-to-bottom (upside-down from the operator's perspective). RTTank is substantially extended: - Dual independent YAxisImpl instances (left + right) replacing the previous single-axis design. Both share the same range, log mode, format and tick settings. Inspired by CS-Studio BOY's tank that could render markers on both sides of the bar. - Alarm/warning limit lines (LOLO, LO, HI, HIHI) drawn as horizontal lines over the tank body. Lines sourced from PV metadata are solid; manually configured lines are dashed (2 px dash / 4 px gap) so the operator can distinguish the source at a glance. - Tank body outline via drawRoundRect in the foreground color. Edges that lack a neighbouring scale receive a 1 px inset so the outline stroke is not clipped by the canvas boundary. - setLimitsFromPV(), setAlarmColors(), setRightScaleVisible() API. - Constructor init-order fix: right_scale.setOnRight(true) moved after update_throttle creation to avoid a NPE via requestUpdate(). --- .../org/csstudio/javafx/rtplot/RTTank.java | 267 +++++++++- .../javafx/rtplot/internal/AxisPart.java | 17 + .../javafx/rtplot/internal/LinearTicks.java | 59 ++- .../javafx/rtplot/internal/LogTicks.java | 463 +++++++++++++----- .../javafx/rtplot/internal/YAxisImpl.java | 249 ++++++++-- .../csstudio/javafx/rtplot/LogTicksTest.java | 72 ++- .../csstudio/javafx/rtplot/RTTankTest.java | 90 ++++ 7 files changed, 1036 insertions(+), 181 deletions(-) create mode 100644 app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java index ddb81de2e8..2f17a0dbb5 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java @@ -7,22 +7,26 @@ ******************************************************************************/ package org.csstudio.javafx.rtplot; +import java.awt.BasicStroke; import java.awt.Color; import java.awt.GradientPaint; import java.awt.Graphics2D; import java.awt.Rectangle; import java.awt.RenderingHints; import java.awt.image.BufferedImage; +import java.text.NumberFormat; import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import org.csstudio.javafx.rtplot.internal.LinearTicks; import org.csstudio.javafx.rtplot.internal.PlotPart; import org.csstudio.javafx.rtplot.internal.PlotPartListener; import org.csstudio.javafx.rtplot.internal.YAxisImpl; import org.csstudio.javafx.rtplot.internal.util.GraphicsUtils; import org.phoebus.ui.javafx.BufferUtil; import org.phoebus.ui.javafx.UpdateThrottle; +import org.phoebus.ui.vtype.FormatOption; import javafx.application.Platform; import javafx.beans.value.ChangeListener; @@ -33,8 +37,20 @@ import javafx.scene.text.Font; /** Tank with scale + * + *

Renders a vertical tank with fill level, optional left and right + * scales, a foreground outline, and optional alarm/warning limit lines. + * The dual-scale layout is modelled after CS-Studio BOY's tank widget + * which supported markers on both sides of the bar. + * + *

When alarm limits come from a live PV they are drawn as solid + * lines; when they are manually configured the lines are drawn dashed + * so the operator can tell at a glance that they are not tied to the + * control system's alarm state. * * @author Kay Kasemir + * @author Heredie Delvalle — CLS, dual scale, alarm limits, + * format/precision, outline, log scale fixes */ @SuppressWarnings("nls") public class RTTank extends Canvas @@ -45,14 +61,35 @@ public class RTTank extends Canvas /** Background color */ private volatile Color background = Color.WHITE; + /** Foreground color (used for scale ticks and the tank outline) */ + private volatile Color foreground = Color.BLACK; + /** Fill color */ private volatile Color empty = Color.LIGHT_GRAY.brighter().brighter(); private volatile Color empty_shadow = Color.LIGHT_GRAY; - /** Fill color */ + /** Fill color. + * {@code fill_highlight} is a lighter variant used for a gradient. + */ private volatile Color fill = Color.BLUE; private volatile Color fill_highlight = new Color(72, 72, 255); + /** Configurable alarm/warning line colors (default: named MAJOR=red, MINOR=orange) */ + private volatile Color limit_major_color = Color.RED; + private volatile Color limit_minor_color = new Color(255, 128, 0); + + /** Alarm and warning limits drawn as horizontal lines (NaN = not set/hidden) */ + private volatile double limit_lolo = Double.NaN; + private volatile double limit_lo = Double.NaN; + private volatile double limit_hi = Double.NaN; + private volatile double limit_hihi = Double.NaN; + + /** Are the alarm limits sourced from PV metadata? When false, use dashed lines. */ + private volatile boolean limits_from_pv = true; + + /** Is the right-side scale displayed? */ + private volatile boolean right_scale_visible = false; + /** Current value, i.e. fill level */ private volatile double value = 5.0; @@ -101,8 +138,15 @@ public void refreshPlotPart(final PlotPart plotPart) } }; + /** Left-side scale axis */ final private YAxisImpl scale = new YAxisImpl<>("", plot_part_listener); + /** Right-side (opposite) scale axis. + * Always has {@code is_right=true} and {@code force_text_up=true} + * so that axis labels read in the same direction as the left scale. + */ + final private YAxisImpl right_scale = new YAxisImpl<>("", plot_part_listener); + final private PlotPart plot_area = new PlotPart("main", plot_part_listener); @@ -132,6 +176,13 @@ public RTTank() } } }); + + // Configure right-side scale — must happen after update_throttle is + // initialised because setOnRight() triggers requestUpdate() via the + // PlotPartListener. setForceTextUp(true) keeps the label direction + // consistent with the left scale. + right_scale.setOnRight(true); + right_scale.setForceTextUp(true); } @@ -149,6 +200,8 @@ public void setFont(final Font font) { scale.setLabelFont(font); scale.setScaleFont(font); + right_scale.setLabelFont(font); + right_scale.setScaleFont(font); } /** @param color Background color */ @@ -157,10 +210,12 @@ public void setBackground(final javafx.scene.paint.Color color) background = GraphicsUtils.convert(Objects.requireNonNull(color)); } - /** @param color Foreground color */ + /** @param color Foreground color (scale ticks and tank outline) */ public void setForeground(final javafx.scene.paint.Color color) { + foreground = GraphicsUtils.convert(Objects.requireNonNull(color)); scale.setColor(color); + right_scale.setColor(color); } /** @param color Color for empty region */ @@ -188,7 +243,7 @@ public void setFillColor(final javafx.scene.paint.Color color) ); } - /** @param visible Whether the scale must be displayed or not. */ + /** @param visible Whether the left-side scale must be displayed. */ public void setScaleVisible (boolean visible) { if (visible != scale_visible) @@ -202,6 +257,104 @@ public void setScaleVisible (boolean visible) public void setLogScale(final boolean logscale) { scale.setLogarithmic(logscale); + right_scale.setLogarithmic(logscale); + requestUpdate(); + } + + /** @param show Show minor tick marks on the scale? */ + public void setShowMinorTicks(final boolean show) + { + scale.setShowMinorTicks(show); + right_scale.setShowMinorTicks(show); + requestUpdate(); + } + + /** Configure the number format used for scale tick labels. + * @param format Display format; {@code null} or {@link FormatOption#DEFAULT} restores automatic formatting. + * @param precision Number of decimal places; clamped to [0, 15]. + */ + public void setLabelFormat(final FormatOption format, final int precision) + { + final int prec = Math.max(0, Math.min(15, precision)); + final NumberFormat fmt; + if (format == null || format == FormatOption.DEFAULT) + fmt = null; + else switch (format) + { + case DECIMAL: + fmt = LinearTicks.createDecimalFormat(prec); + break; + case ENGINEERING: + case EXPONENTIAL: + default: + fmt = LinearTicks.createExponentialFormat(prec); + break; + } + scale.setLabelFormat(fmt); + right_scale.setLabelFormat(fmt); + requestUpdate(); + } + + /** Set alarm and warning limit values to display as horizontal lines on the tank. + * Pass {@link Double#NaN} for any limit that should not be shown. + * @param lolo LOLO (major alarm) lower limit + * @param lo LO (minor warning) lower limit + * @param hi HI (minor warning) upper limit + * @param hihi HIHI (major alarm) upper limit + */ + public void setLimits(final double lolo, final double lo, + final double hi, final double hihi) + { + limit_lolo = lolo; + limit_lo = lo; + limit_hi = hi; + limit_hihi = hihi; + requestUpdate(); + } + + /** Set the colors used to draw alarm limit lines. + * @param minor color for LO / HI (minor warning) lines + * @param major color for LOLO / HIHI (major alarm) lines + */ + public void setAlarmColors(final javafx.scene.paint.Color minor, + final javafx.scene.paint.Color major) + { + limit_minor_color = GraphicsUtils.convert(Objects.requireNonNull(minor)); + limit_major_color = GraphicsUtils.convert(Objects.requireNonNull(major)); + requestUpdate(); + } + + /** Indicate whether the current alarm limits come from PV metadata. + * When {@code false} the limit lines are drawn dashed to signal that + * they are manually configured and do not reflect live alarm state. + * @param from_pv {@code true} for solid (PV-sourced), {@code false} for dashed + */ + public void setLimitsFromPV(final boolean from_pv) + { + limits_from_pv = from_pv; + requestUpdate(); + } + + /** Show or hide a second scale on the opposite (right) side of the tank. + * Both scales share the same range, log mode, format and tick settings. + * @param visible {@code true} to show, {@code false} to hide (default) + */ + public void setRightScaleVisible(final boolean visible) + { + if (right_scale_visible == visible) + return; + right_scale_visible = visible; + need_layout.set(true); + requestUpdate(); + } + + /** @param perpendicular Draw tick labels perpendicular to the axis direction? + * When {@code false}, labels are rotated parallel to the axis. + */ + public void setPerpendicularTickLabels(final boolean perpendicular) + { + scale.setPerpendicularTickLabels(perpendicular); + right_scale.setPerpendicularTickLabels(perpendicular); requestUpdate(); } @@ -211,7 +364,11 @@ public void setLogScale(final boolean logscale) */ public void setRange(final double low, final double high) { + // Guard against NaN, Infinite, or inverted/flat range + if (!Double.isFinite(low) || !Double.isFinite(high) || low >= high) + return; scale.setValueRange(low, high); + right_scale.setValueRange(low, high); } /** @param value Set value */ @@ -224,22 +381,79 @@ public void setValue(final double value) requestUpdate(); } - /** Compute layout of plot components */ + /** Map a value to a Y pixel within the plot bounds (low value at bottom). + * Returns -1 when the mapping is undefined (e.g. log scale with non-positive inputs). + */ + private int valueToY(final Rectangle pb, final double min, final double max, + final double v, final boolean logscale) + { + final double frac; + if (logscale) + { + if (min <= 0 || max <= 0 || v <= 0) + return -1; + frac = Math.log(v / min) / Math.log(max / min); + } + else + frac = (v - min) / (max - min); + return (int) (pb.y + pb.height * (1.0 - frac)); + } + + /** Draw a single horizontal limit line across the tank area at the given value. */ + private void drawLimitLineAt(final Graphics2D gc, final Rectangle pb, + final double min, final double max, + final double limit, final Color color) + { + if (!Double.isFinite(limit) || limit <= min || limit >= max) + return; + final int y = valueToY(pb, min, max, limit, scale.isLogarithmic()); + if (y < pb.y || y > pb.y + pb.height) + return; + gc.setColor(color); + gc.drawLine(pb.x, y, pb.x + pb.width, y); + } + + /** Compute layout of plot components. + * Supports independent left and right scales; the plot area sits + * between them. A 1-pixel inset is added on any edge that has no + * scale so the outline {@code drawRoundRect} is not clipped. + */ private void computeLayout(final Graphics2D gc, final Rectangle bounds) { - final Rectangle scale_region = new Rectangle(bounds); - scale_region.width = scale.getDesiredPixelSize(scale_region, gc); + int left_width = 0; + int right_width = 0; + int[] ends = { 0, 0 }; // [bottom gap, top gap] + + if (scale_visible) + { + left_width = scale.getDesiredPixelSize(bounds, gc); + ends = scale.getPixelGaps(gc); + } + if (right_scale_visible) + { + right_width = right_scale.getDesiredPixelSize(bounds, gc); + final int[] r_ends = right_scale.getPixelGaps(gc); + ends[0] = Math.max(ends[0], r_ends[0]); + ends[1] = Math.max(ends[1], r_ends[1]); + } - final int[] ends = scale.getPixelGaps(gc); - scale_region.y += ends[1]; - scale_region.height -= ends[0] + ends[1]; + // Small inset so the tank outline is not clipped on edges without a scale + final int inset_left = (left_width == 0) ? 1 : 0; + final int inset_right = (right_width == 0) ? 1 : 0; + final int inset_top = (ends[1] == 0) ? 1 : 0; + final int inset_bottom = (ends[0] == 0) ? 1 : 0; - scale.setBounds(scale_region); + final int top = bounds.y + ends[1] + inset_top; + final int height = bounds.height - ends[0] - ends[1] - inset_top - inset_bottom; if (scale_visible) - plot_area.setBounds(bounds.x + scale_region.width, bounds.y+ends[1], bounds.width-scale_region.width, bounds.height-ends[0]-ends[1]); - else - plot_area.setBounds(bounds.x, bounds.y, bounds.width, bounds.height); + scale.setBounds(new Rectangle(bounds.x, top, left_width, height)); + if (right_scale_visible) + right_scale.setBounds(new Rectangle(bounds.x + bounds.width - right_width, + top, right_width, height)); + + plot_area.setBounds(bounds.x + left_width + inset_left, top, + bounds.width - left_width - right_width - inset_left - inset_right, height); } /** Draw all components into image buffer */ @@ -270,6 +484,8 @@ protected Image updateImageBuffer() if (scale_visible) scale.paint(gc, plot_bounds); + if (right_scale_visible) + right_scale.paint(gc, plot_bounds); plot_area.paint(gc); @@ -305,6 +521,31 @@ else if (scale.isLogarithmic()) // by mellguth2, https://github.com/ControlSyste gc.fillRoundRect(plot_bounds.x, plot_bounds.y+plot_bounds.height-level, plot_bounds.width, level, arc, arc); else gc.fillRoundRect(plot_bounds.x, plot_bounds.y, plot_bounds.width, level, arc, arc); + + // Dark outline around the tank body + gc.setColor(foreground); + gc.drawRoundRect(plot_bounds.x, plot_bounds.y, plot_bounds.width, plot_bounds.height, arc, arc); + + // Draw alarm / warning limit lines over the tank body + final double lim_lolo = limit_lolo; + final double lim_lo = limit_lo; + final double lim_hi = limit_hi; + final double lim_hihi = limit_hihi; + if (normal && (!Double.isNaN(lim_lolo) || !Double.isNaN(lim_lo) || + !Double.isNaN(lim_hi) || !Double.isNaN(lim_hihi))) + { + if (limits_from_pv) + gc.setStroke(new BasicStroke(2f)); + else + gc.setStroke(new BasicStroke(2f, BasicStroke.CAP_BUTT, + BasicStroke.JOIN_MITER, 10f, new float[]{6f, 4f}, 0f)); + drawLimitLineAt(gc, plot_bounds, min, max, lim_lolo, limit_major_color); + drawLimitLineAt(gc, plot_bounds, min, max, lim_lo, limit_minor_color); + drawLimitLineAt(gc, plot_bounds, min, max, lim_hi, limit_minor_color); + drawLimitLineAt(gc, plot_bounds, min, max, lim_hihi, limit_major_color); + gc.setStroke(new BasicStroke(1f)); + } + gc.setColor(background); gc.dispose(); diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/AxisPart.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/AxisPart.java index c94493aa8a..94cbd56586 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/AxisPart.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/AxisPart.java @@ -55,6 +55,8 @@ public abstract class AxisPart> extends PlotPart impleme protected volatile boolean show_grid = false; + /** Show minor tick marks? */ + protected volatile boolean show_minor_ticks = true; protected volatile Color grid_color; private AtomicBoolean visible = new AtomicBoolean(true); @@ -166,6 +168,21 @@ public void setGridColor(final Color grid_color) this.grid_color = grid_color; } + /** @return Whether minor tick marks are shown */ + public boolean isShowMinorTicks() + { + return show_minor_ticks; + } + + /** @param show {@code true} to show minor tick marks */ + public void setShowMinorTicks(final boolean show) + { + if (show_minor_ticks == show) + return; + show_minor_ticks = show; + requestRefresh(); + } + /** {@inheritDoc} */ @Override public boolean isVisible() diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/LinearTicks.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/LinearTicks.java index fe9d020a8e..db7c6c6c61 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/LinearTicks.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/LinearTicks.java @@ -53,6 +53,61 @@ public class LinearTicks extends Ticks /** Threshold for order-of-magnitude to use exponential notation */ private long exponential_threshold = 4; + /** User-specified override format, or {@code null} for automatic selection. + * When set, it is applied to all non-empty major tick labels after + * {@code compute()} finishes its internal layout. + */ + private volatile NumberFormat label_fmt_override = null; + + /** When {@code true}, the axis draws tick labels perpendicular to the axis direction. + * When {@code false} (default for RTPlot charts), they are rotated parallel to the axis. + */ + private volatile boolean perpendicular_tick_labels = false; + + /** Set a user-specified format for all major tick labels. + * @param fmt Format to apply, or {@code null} to restore automatic formatting. + */ + public void setLabelFormat(final NumberFormat fmt) + { + label_fmt_override = fmt; + } + + /** @return The current label format override, or {@code null} if automatic. */ + public NumberFormat getLabelFormatOverride() + { + return label_fmt_override; + } + + /** Configure whether tick labels are drawn as horizontal (readable) text. + * @param horizontal {@code true} to draw labels horizontally (left-to-right); + * {@code false} to draw them rotated 90° (RTPlot default). + */ + public void setPerpendicularTickLabels(final boolean perpendicular) + { + perpendicular_tick_labels = perpendicular; + } + + /** @return {@code true} if tick labels are drawn perpendicular to the axis. */ + public boolean isPerpendicularTickLabels() + { + return perpendicular_tick_labels; + } + + /** Re-apply {@code fmt} to every non-empty major tick label in {@code ticks}. + * @param ticks List to mutate in-place + * @param fmt Format to use + */ + protected static void relabelTicks(final List> ticks, + final NumberFormat fmt) + { + for (int i = 0; i < ticks.size(); i++) + { + final MajorTick t = ticks.get(i); + if (!t.getLabel().isEmpty()) + ticks.set(i, new MajorTick<>(t.getValue(), fmt.format(t.getValue()))); + } + } + /** @param order_of_magnitude determines when to use exponential notation */ public void setExponentialThreshold(long order_of_magnitude) { @@ -289,7 +344,7 @@ public static double selectNiceStep(final double min_distance) * @param precision * @return NumberFormat */ - protected static NumberFormat createDecimalFormat(final int precision) + public static NumberFormat createDecimalFormat(final int precision) { final NumberFormat fmt = NumberFormat.getNumberInstance(LOCALE); fmt.setGroupingUsed(false); @@ -304,7 +359,7 @@ protected static NumberFormat createDecimalFormat(final int precision) * @param mantissa_precision * @return NumberFormat */ - protected static NumberFormat createExponentialFormat(final Integer mantissa_precision) + public static NumberFormat createExponentialFormat(final Integer mantissa_precision) { return exponential_formats.computeIfAbsent(mantissa_precision, prec -> { diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/LogTicks.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/LogTicks.java index d1a6c4aa39..3a76def321 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/LogTicks.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/LogTicks.java @@ -11,50 +11,78 @@ import java.awt.FontMetrics; import java.awt.Graphics2D; +import java.text.NumberFormat; import java.util.ArrayList; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Set; import java.util.TreeSet; import java.util.logging.Level; -import java.util.stream.IntStream; import javafx.util.Pair; import org.csstudio.javafx.rtplot.internal.util.Log10; -/** Helper for creating tick marks. - *

- * Computes tick positions, formats tick labels. - * Doesn't perform the actual drawing. +/** Helper for creating tick marks on a logarithmic scale. + * + *

Computes tick positions and formats tick labels. + * Does not perform actual drawing. + * + *

Three layout strategies are applied in order of preference: + *

    + *
  1. If there are more decades than fit on screen, the decade list is + * thinned geometrically. All decades still get a prominent tick line; + * only the thinned subset receives a text label.
    2. + *
  2. If all decades fit, sub-decade values (2×10^n … 9×10^n) are added + * as minor ticks — or promoted to labeled major ticks when the axis + * has enough room for all of them.
    3. + *
  3. When the range spans less than one decade a linear tick spacing is + * used as a fallback.
    4. + *
+ * * @author Kay Kasemir */ @SuppressWarnings("nls") public class LogTicks extends LinearTicks { + /** Set to {@code true} by {@link #buildThinnedDecadeTicks} to signal that + * the labeled subset has already been chosen symmetrically by + * {@link #thinDecades}. When {@code true}, the axis painter must not + * apply a second visibility-culling pass, which would destroy the + * intentional spacing. + */ + private boolean thinned = false; + + /** @return {@code true} when the tick-label set was pre-thinned by + * {@link #thinDecades} and must not be culled again by the caller. */ + public boolean isThinned() { return thinned; } + public LogTicks() { - num_fmt = createExponentialFormat(2); + num_fmt = createExponentialFormat(2); detailed_num_fmt = createExponentialFormat(3); } + /** {@inheritDoc} + * + *

Clamps non-positive or non-finite bounds to valid values for a + * logarithmic scale and ensures {@code low < high}. + */ @Override - public Pair adjustRange(Double low, Double high) { - // Only support 'normal' order, low < high: - if (!Double.isFinite(low) || low <= 0.0) { + public Pair adjustRange(Double low, Double high) + { + if (!Double.isFinite(low) || low <= 0.0) low = Math.ulp(0.0); - } - if (!Double.isFinite(high)) { + if (!Double.isFinite(high)) high = Double.MAX_VALUE; + if (Math.abs(high - low) < 3 * Math.ulp(low)) + high = low + 3 * Math.ulp(low); + if (high < low) + { + final double swap = low; + low = high; + high = swap; } - if (Math.abs(high - low) < 3*Math.ulp(low)) { - high = low + 3*Math.ulp(low); - } - if (high < low) { - double new_high = low; - low = high; - high = new_high; - } - return new Pair<>(low, high); } @@ -62,127 +90,314 @@ public Pair adjustRange(Double low, Double high) { @Override public void compute(Double low, Double high, final Graphics2D gc, final int screen_width) { - Pair adjustedRange = adjustRange(low, high); - double newLow = adjustedRange.getKey(); - double newHigh = adjustedRange.getValue(); - - if (newLow != low || newHigh != high) { - logger.log(Level.WARNING, "Invalid value range for a logarithmic scale {0,number,#.###############E0} ... {1,number,#.###############E0}. Adjusting the range to {2,number,#.###############E0} ... {3,number,#.###############E0}.", - new Object[] {low, high, newLow, newHigh }); - high = newHigh; - low = newLow; - } + final Pair adjusted = adjustRange(low, high); + final double adjLow = adjusted.getKey(); + final double adjHigh = adjusted.getValue(); + if (adjLow != low || adjHigh != high) + logger.log(Level.WARNING, + "Invalid value range for a logarithmic scale {0,number,#.###############E0} ... " + + "{1,number,#.###############E0}. Adjusting the range to " + + "{2,number,#.###############E0} ... {3,number,#.###############E0}.", + new Object[] { low, high, adjLow, adjHigh }); + low = adjLow; + high = adjHigh; logger.log(Level.FINE, "Compute log ticks, width {0}, for {1} - {2}", new Object[] { screen_width, low, high }); - double low_exp_exact = Log10.log10(low); - double high_exp_exact = Log10.log10(high); - - // Test format - int precision = 2; - num_fmt = createExponentialFormat(precision); - - // Determine minimum label distance on the screen, using some - // percentage of the available screen space. - // Guess the label width, using the two extremes. - final String low_label = format(low); - final String high_label = format(high); - final FontMetrics metrics = gc.getFontMetrics(); - final int label_width = Math.max(metrics.stringWidth(low_label), metrics.stringWidth(high_label)); - final int num_that_fits = Math.max(1, screen_width/label_width*FILL_PERCENTAGE/100); + final double lowExp = Log10.log10(low); + final double highExp = Log10.log10(high); + + num_fmt = createExponentialFormat(0); + zero_threshold = 0.0; + thinned = false; + + final int lowExpInt = (int) Math.floor(lowExp); + final int highExpInt = (int) Math.ceil(highExp); + final int numFits = numLabelsThatFit(gc.getFontMetrics(), screen_width, lowExpInt, highExpInt); + final List decadeExps = collectDecadeExponents(lowExp, highExp, low, high); + final List subDecadeValues = collectSubDecadeValues(lowExp, highExp, low, high); + final List> major_ticks = new ArrayList<>(); final List> minor_ticks = new ArrayList<>(); - List exponentsOfPowersOfTenInInterval = new ArrayList<>(); - List minorTickValuesInInterval = new LinkedList<>(); - for(int exponent : IntStream.rangeClosed((int) Math.floor(low_exp_exact), (int) Math.ceil(high_exp_exact)).toArray()) { - double powerOfTen = Log10.pow10(exponent); - if (powerOfTen >= low && powerOfTen <= high) { - exponentsOfPowersOfTenInInterval.add(exponent); - } + if (decadeExps.size() >= 2 && decadeExps.size() > numFits) + buildThinnedDecadeTicks(decadeExps, subDecadeValues, numFits, major_ticks, minor_ticks); + else if (decadeExps.size() >= 2) + buildLabeledDecadeTicks(decadeExps, subDecadeValues, numFits, major_ticks, minor_ticks); + else + buildLinearFallbackTicks(low, high, numFits, major_ticks); - for (int i = 1; i < 10; i++) { - double minorTickValue = i * powerOfTen; - if (minorTickValue >= low && minorTickValue <= high) { - minorTickValuesInInterval.add(minorTickValue); - } - } + if (major_ticks.size() < 2) + { // Fallback: ensure range endpoints are always visible. + // Do NOT reset num_fmt to precision 17 (as LinearTicks does) + // because log-scale boundaries are clean powers of ten. + major_ticks.add(0, new MajorTick<>(low, format(low))); + major_ticks.add( new MajorTick<>(high, format(high))); } - zero_threshold = 0.0; - if (exponentsOfPowersOfTenInInterval.size() >= 2 && exponentsOfPowersOfTenInInterval.size() > num_that_fits) { - precision = 0; - num_fmt = createExponentialFormat(precision); - while (exponentsOfPowersOfTenInInterval.size() > num_that_fits) { - List newExponentsOfPowersOfTenInInterval = new LinkedList<>(); - for (int i=0; i < exponentsOfPowersOfTenInInterval.size()/2; i++) { - newExponentsOfPowersOfTenInInterval.add(i, exponentsOfPowersOfTenInInterval.get(2*i)); - } - if (exponentsOfPowersOfTenInInterval.size() % 2 == 1) { - newExponentsOfPowersOfTenInInterval.add(exponentsOfPowersOfTenInInterval.get(exponentsOfPowersOfTenInInterval.size() - 1)); - } - exponentsOfPowersOfTenInInterval = newExponentsOfPowersOfTenInInterval; - } - for (int exponent : exponentsOfPowersOfTenInInterval) { - double majorTickValueInInterval = Log10.pow10(exponent); - major_ticks.add(new MajorTick<>(majorTickValueInInterval, format(majorTickValueInInterval))); - } + // Apply user-specified label format override if set. + final NumberFormat override = getLabelFormatOverride(); + if (override != null) + relabelTicks(major_ticks, override); + + this.major_ticks = major_ticks; + this.minor_ticks = minor_ticks; + } + + // ----------------------------------------------------------------------- + // Private helpers + // ----------------------------------------------------------------------- + + /** @return Number of tick labels that fit in {@code screenPixels}. + * + *

Labels on a log-scale axis are drawn rotated 90° so their physical + * extent along the axis is their string width, not {@link FontMetrics#getHeight()}. + * Using height (≈14 px) instead of a typical string width (≈30 px for + * {@code "1E10"}) overestimates by 2–3×, causing thinDecades to keep too + * many labels and producing visual overlap. + * + *

This method measures the widest of the bottom and top decade labels + * and uses that as the slot size. + * + * @param metrics Font metrics for the current scale font + * @param screenPixels Axis length in pixels + * @param lowExpInt Floor of {@code log10(low)} + * @param highExpInt Ceil of {@code log10(high)} + */ + private int numLabelsThatFit(final FontMetrics metrics, final int screenPixels, + final int lowExpInt, final int highExpInt) + { + final int labelSize; + if (isPerpendicularTickLabels()) + // Horizontal labels: each occupies one font-height along the axis. + labelSize = Math.max(1, metrics.getHeight()); + else + { + // Vertical (rotated) labels: each occupies its string width along the axis. + final NumberFormat fmt = createExponentialFormat(0); + final int w1 = metrics.stringWidth(fmt.format(Log10.pow10(lowExpInt))); + final int w2 = metrics.stringWidth(fmt.format(Log10.pow10(highExpInt))); + labelSize = Math.max(1, Math.max(w1, w2)); } - else if (exponentsOfPowersOfTenInInterval.size() >= 2 && exponentsOfPowersOfTenInInterval.size() <= num_that_fits) { - precision = 0; - num_fmt = createExponentialFormat(precision); - for (int exponent : exponentsOfPowersOfTenInInterval) { - double majorTickValueInInterval = Log10.pow10(exponent); - major_ticks.add(new MajorTick<>(majorTickValueInInterval, format(majorTickValueInInterval))); - } + return Math.max(2, screenPixels * FILL_PERCENTAGE / 100 / labelSize); + } + + /** Collect the integer exponents for all powers of 10 within [low, high]. + * + * @param lowExp {@code log10(low)} + * @param highExp {@code log10(high)} + * @param low Range lower bound + * @param high Range upper bound + * @return Sorted list of exponents {@code n} such that {@code 10^n} is in [low, high] + */ + private List collectDecadeExponents(final double lowExp, final double highExp, + final double low, final double high) + { + final List result = new ArrayList<>(); + for (int exp = (int) Math.floor(lowExp); exp <= (int) Math.ceil(highExp); exp++) + { + final double val = Log10.pow10(exp); + if (val >= low && val <= high) + result.add(exp); + } + return result; + } - for (double minorTickValueInInterval : minorTickValuesInInterval) { - minor_ticks.add(new MinorTick<>(minorTickValueInInterval)); + /** Collect sub-decade values {@code i × 10^n} for {@code i = 1…9} within [low, high]. + * + *

The {@code i = 1} values are exact decades and are included so that + * callers receive the complete set of candidate tick positions. Callers + * that must not double-count decades can use {@link #isNotExactDecade}. + * + * @param lowExp {@code log10(low)} + * @param highExp {@code log10(high)} + * @param low Range lower bound + * @param high Range upper bound + * @return Values in [low, high] of the form {@code i × 10^n}, {@code i = 1…9} + */ + private List collectSubDecadeValues(final double lowExp, final double highExp, + final double low, final double high) + { + final List result = new LinkedList<>(); + for (int exp = (int) Math.floor(lowExp); exp <= (int) Math.ceil(highExp); exp++) + { + final double base = Log10.pow10(exp); + for (int i = 1; i < 10; i++) + { + final double val = i * base; + if (val >= low && val <= high) + result.add(val); } } - else linearScale: { - // Compute scale with linearly spaced values: - int logOfSignificantDecimal = (int) Math.floor(Math.log10(high - low)); - double significantDecimal = Log10.pow10(logOfSignificantDecimal); - Set tickValues = new TreeSet<>(); - double stepSize = significantDecimal; - int steps = 0; - do { - double tickValue = low - (low % significantDecimal) - stepSize; - if (!Double.isFinite(tickValue) || tickValue + stepSize == tickValue) { - // The precision of the double is insufficient for the calculation. - break linearScale; - } - while (tickValue <= high) { - if (tickValue >= low && tickValue <= high) { - tickValues.add(tickValue); - } - tickValue += stepSize; - } - steps++; - stepSize /= 2; - } while (tickValues.size() < num_that_fits / 2); - - for (double tickValue : tickValues) { - precision = (int) Math.floor(Math.log10(tickValue)) - logOfSignificantDecimal + steps; - num_fmt = createExponentialFormat(precision-1); - detailed_num_fmt = createExponentialFormat(precision+1); - major_ticks.add(new MajorTick<>(tickValue, format(tickValue))); + return result; + } + + /** Select up to {@code maxCount} evenly-spaced entries from {@code decades}, + * always including the first and last entry. + * + *

The previous halving approach could not preserve the true midpoint of + * a range: e.g., for exponents [0…10] with maxCount=3 it returned {0,8,10} + * (or after the visibility pass: 0, 4, 10) instead of the symmetrical + * {0, 5, 10}. Evenly-spaced index selection avoids that bias. + * + * @param decades Full sorted list of decade exponents + * @param maxCount Maximum number of entries to select + * @return Selected list with at most {@code maxCount} entries + */ + private List thinDecades(final List decades, final int maxCount) + { + final int n = decades.size(); + if (n <= maxCount) + return new ArrayList<>(decades); + + final List result = new ArrayList<>(maxCount); + for (int i = 0; i < maxCount; i++) + { + // Map slot i uniformly across the index range [0, n-1]. + final int idx = (int) Math.round((double) i * (n - 1) / (maxCount - 1)); + result.add(decades.get(idx)); + } + return result; + } + + /** @return {@code true} when {@code val} is not an exact power of ten. */ + private static boolean isNotExactDecade(final double val) + { + final double log = Log10.log10(val); + return Math.abs(log - Math.round(log)) > 1e-9; + } + + /** Build ticks for the case where there are more decades than fit on screen. + * + *

The labeled set is thinned to {@code numFits}, but every decade still + * receives a prominent (long) tick line. Decades that lost their label + * are emitted as major ticks with an empty label string so they remain + * visually distinct from the shorter minor marks. + * + * @param decadeExps All decade exponents in range + * @param subDecadeValues Sub-decade values (2×10^n … 9×10^n) in range + * @param numFits Number of labels that fit + * @param major_ticks Output: major tick list to populate + * @param minor_ticks Output: minor tick list to populate + */ + private void buildThinnedDecadeTicks( + final List decadeExps, final List subDecadeValues, + final int numFits, + final List> major_ticks, final List> minor_ticks) + { + num_fmt = createExponentialFormat(0); + thinned = true; // caller must not apply a second visibility pass + + final List labeled = thinDecades(decadeExps, numFits); + final Set labeledSet = new HashSet<>(labeled); + + for (int exp : decadeExps) + { + final double val = Log10.pow10(exp); + final String label = labeledSet.contains(exp) ? format(val) : ""; + major_ticks.add(new MajorTick<>(val, label)); + } + + // Sub-decade values (2×10^n … 9×10^n) become minor ticks. + // Exact decades are already major ticks above; skip them here. + for (double val : subDecadeValues) + if (isNotExactDecade(val)) + minor_ticks.add(new MinorTick<>(val)); + } + + /** Build ticks for the case where all decades fit within the available space. + * + *

When all sub-decade values also fit, they are promoted to labeled + * major ticks so a tall tank shows every significant value. Otherwise + * they remain as short minor marks. + * + * @param decadeExps All decade exponents in range + * @param subDecadeValues Sub-decade values (i×10^n, i=1…9) in range + * @param numFits Number of labels that fit + * @param major_ticks Output: major tick list to populate + * @param minor_ticks Output: minor tick list to populate + */ + private void buildLabeledDecadeTicks( + final List decadeExps, final List subDecadeValues, + final int numFits, + final List> major_ticks, final List> minor_ticks) + { + num_fmt = createExponentialFormat(0); + + for (int exp : decadeExps) + { + final double val = Log10.pow10(exp); + major_ticks.add(new MajorTick<>(val, format(val))); + } + + final boolean allFit = decadeExps.size() + subDecadeValues.size() <= numFits; + if (allFit) + { + // Promote sub-decade values to labeled major ticks. + // Filter exact decades (already added above) to avoid duplicates. + for (double val : subDecadeValues) + if (isNotExactDecade(val)) + major_ticks.add(new MajorTick<>(val, format(val))); + major_ticks.sort((a, b) -> Double.compare(a.getValue(), b.getValue())); + } + else + { + // Not enough room to label everything; keep sub-decade values as + // anonymous minor marks. Include i=1 values so the minor line + // appears at every decade position before the labeled major tick. + for (double val : subDecadeValues) + minor_ticks.add(new MinorTick<>(val)); + } + } + + /** Build ticks using a linear spacing for sub-decade ranges. + * + *

Used as a fallback when the range spans less than one decade and + * the logarithmic decade strategy would produce fewer than two ticks. + * The step size is halved iteratively until enough ticks are present. + * + *

On arithmetic overflow or insufficient double precision, the method + * returns without adding any ticks; the caller will then fall back to + * showing just the range endpoints. + * + * @param low Range lower bound + * @param high Range upper bound + * @param numFits Target number of ticks + * @param major_ticks Output: major tick list to populate + */ + private void buildLinearFallbackTicks( + final double low, final double high, final int numFits, + final List> major_ticks) + { + final int logStep = (int) Math.floor(Math.log10(high - low)); + final double step0 = Log10.pow10(logStep); + final Set values = new TreeSet<>(); + double step = step0; + int refinements = 0; + + do + { + double v = low - (low % step0) - step; + if (!Double.isFinite(v) || v + step == v) + return; // precision exhausted; let caller add endpoints + while (v <= high) + { + if (v >= low) + values.add(v); + v += step; } + refinements++; + step /= 2; } + while (values.size() < numFits / 2); - if (major_ticks.size() < 2) - { // If the best-laid plans of mice and men fail - // and we end up with just one or no tick, - // add the low and high markers - precision = 17; - num_fmt = createExponentialFormat(precision-1); - detailed_num_fmt = createExponentialFormat(precision+1); - major_ticks.add(0, new MajorTick<>(low, format(low))); - major_ticks.add(new MajorTick<>(high, format(high))); + for (double v : values) + { + final int prec = (int) Math.floor(Math.log10(v)) - logStep + refinements; + num_fmt = createExponentialFormat(prec - 1); + detailed_num_fmt = createExponentialFormat(prec + 1); + major_ticks.add(new MajorTick<>(v, format(v))); } - this.major_ticks = major_ticks; - this.minor_ticks = minor_ticks; } } diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/YAxisImpl.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/YAxisImpl.java index 37b1b1ac36..109ad205c5 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/YAxisImpl.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/internal/YAxisImpl.java @@ -60,6 +60,18 @@ public class YAxisImpl> extends NumericAxis impl /** Show on right side? */ private volatile boolean is_right = false; + /** When {@code true}, rotated tick labels always use the 'up' direction + * (bottom-to-top) regardless of {@link #is_right}. This keeps the text + * orientation of a right-side scale identical to a left-side scale. + */ + private boolean force_text_up = false; + + /** When {@code true}, tick labels are drawn perpendicular to the axis (readable + * across the axis). When {@code false} (RTPlot default), they are rotated parallel + * to the axis. + */ + private boolean perpendicular_tick_labels = false; + /** Traces on this axis. * *

{@link CopyOnWriteArrayList} adds thread safety. @@ -140,6 +152,15 @@ public void setOnRight(final boolean right) requestRefresh(); } + /** Force rotated tick labels to always read bottom-to-top ('up'), + * matching the orientation of a left-side axis. + * @param force {@code true} to override the default text direction + */ + public void setForceTextUp(final boolean force) + { + force_text_up = force; + } + /** Add trace to axis * @param trace {@link Trace} * @throws IllegalArgumentException if trace already on axis @@ -199,13 +220,28 @@ public int getDesiredPixelSize(final Rectangle region, final Graphics2D gc) gc.setFont(scale_font); metrics = gc.getFontMetrics(); - final int scale_size = metrics.getHeight(); + final int scale_size; + if (perpendicular_tick_labels) + { + // Horizontal labels: axis width must accommodate the widest label string. + int max_w = metrics.getHeight(); // minimum width = one font-height + for (final MajorTick tick : ticks.getMajorTicks()) + { + final String lbl = tick.getLabel(); + if (!lbl.isEmpty()) + max_w = Math.max(max_w, metrics.stringWidth(lbl)); + } + scale_size = max_w; + } + else + scale_size = metrics.getHeight(); // Width of labels, width of (rotated) axis text, tick markers. return lines * x_sep + scale_size + TICK_LENGTH; } - private int computeLabelLayout(Graphics2D gc){ + private int computeLabelLayout(final Graphics2D gc) + { FontMetrics metrics = gc.getFontMetrics(); final int x_sep = metrics.getHeight(); @@ -269,6 +305,29 @@ private int computeLabelLayout(Graphics2D gc){ } /** {@inheritDoc} */ + /** Set a user-specified format for all major tick labels on this axis. + * @param fmt Format to apply, or {@code null} to restore automatic formatting. + */ + public void setLabelFormat(final java.text.NumberFormat fmt) + { + if (ticks instanceof LinearTicks) + ((LinearTicks) ticks).setLabelFormat(fmt); + requestLayout(); + } + + /** Configure whether tick labels are drawn as horizontal (readable) text. + * @param horizontal {@code true} → left-to-right text; {@code false} → rotated 90°. + */ + public void setPerpendicularTickLabels(final boolean perpendicular) + { + if (perpendicular_tick_labels == perpendicular) + return; + perpendicular_tick_labels = perpendicular; + if (ticks instanceof LinearTicks) + ((LinearTicks) ticks).setPerpendicularTickLabels(perpendicular); + requestLayout(); + } + @Override public int[] getPixelGaps(final Graphics2D gc) { @@ -282,7 +341,13 @@ public int[] getPixelGaps(final Graphics2D gc) if (major_ticks.isEmpty()) return super.getPixelGaps(gc); - // Measure first and last tick + if (perpendicular_tick_labels) + { + // Horizontal labels: top/bottom gap = half a line height + final int half = metrics.getHeight() / 2; + return new int[] { half, half }; + } + // Vertical (rotated) labels: gap = half the string width final int low = metrics.stringWidth(major_ticks.get(0).getLabel()); final int high = metrics.stringWidth(major_ticks.get(major_ticks.size()-1).getLabel()); @@ -323,17 +388,24 @@ public void paint(final Graphics2D gc, final Rectangle plot_bounds) gc.drawLine(line_x, region.y, line_x, region.y + region.height-1); computeTicks(gc); - // Major tick marks - Rectangle avoid = null; - for (MajorTick tick : ticks.getMajorTicks()) + final List> majorTicks = ticks.getMajorTicks(); + // Skip the visibility pass when LogTicks already thinned the labeled set: + // a second greedy pass would destroy the intentional symmetric spacing. + final boolean skipVisibility = (ticks instanceof LogTicks) && ((LogTicks) ticks).isThinned(); + final boolean[] showLabel = skipVisibility + ? allLabeled(majorTicks) + : computeTickLabelVisibility(majorTicks, gc.getFontMetrics()); + + // Major tick marks and labels + for (int mi = 0; mi < majorTicks.size(); mi++) { + final MajorTick tick = majorTicks.get(mi); final int y = getScreenCoord(tick.getValue()); gc.setStroke(TICK_STROKE); gc.drawLine(line_x, y, tick_x, y); - // Grid line if (show_grid) - { // Dashed line + { // Dashed grid line gc.setColor(grid_color); gc.setStroke(new BasicStroke(1, BasicStroke.CAP_SQUARE, BasicStroke.JOIN_MITER, 1, new float[] { 5 }, 0)); gc.drawLine(plot_bounds.x, y, plot_bounds.x + plot_bounds.width-1, y); @@ -341,16 +413,17 @@ public void paint(final Graphics2D gc, final Rectangle plot_bounds) } gc.setStroke(old_width); - // Tick Label - avoid = drawTickLabel(gc, y, tick.getLabel(), false, avoid); + if (showLabel[mi]) + drawTickLabel(gc, y, tick.getLabel(), false); } // Minor tick marks - for (MinorTick tick : ticks.getMinorTicks()) - { - final int y = getScreenCoord(tick.getValue()); - gc.drawLine(minor_x, y, line_x, y); - } + if (isShowMinorTicks()) + for (MinorTick tick : ticks.getMinorTicks()) + { + final int y = getScreenCoord(tick.getValue()); + gc.drawLine(minor_x, y, line_x, y); + } gc.setColor(old_fg); gc.setBackground(old_bg); @@ -374,54 +447,158 @@ protected void paintLabels(final Graphics2D gc) { // Draw labels at pre-computed locations if (i > 0) GraphicsUtils.drawVerticalText(gc, label_x.get(i-1), label_y.get(i-1) - label_y_separation, - label_provider.getSeparator(), !is_right); + label_provider.getSeparator(), force_text_up || !is_right); gc.setColor(GraphicsUtils.convert(label_provider.getColor())); GraphicsUtils.drawVerticalText(gc, - label_x.get(i), label_y.get(i), label_provider.getLabel(), !is_right); + label_x.get(i), label_y.get(i), label_provider.getLabel(), force_text_up || !is_right); gc.setColor(old_fg); ++i; } } - /** @param gc - * @param screen_y Screen location of label along the axis - * @param mark Label text - * @param floating Add 'floating' box? - * @param avoid Outline of previous label to avoid - * @return Outline of this label or the last one if skipping this label + /** Return a mask where every tick that has a non-empty label is {@code true}. + * + *

Used when {@link LogTicks#isThinned()} is {@code true} — the + * labeled subset was already chosen by {@link LogTicks#thinDecades} so a + * second culling pass must not run. + */ + private static boolean[] allLabeled(final List> majorTicks) + { + final boolean[] show = new boolean[majorTicks.size()]; + for (int i = 0; i < show.length; i++) + show[i] = !majorTicks.get(i).getLabel().isEmpty(); + return show; + } + + /** Pre-compute which major tick labels should be painted. + * + *

Two-pass algorithm: the first and last labeled ticks are always shown. + * An intermediate tick is shown only when there is enough pixel gap to + * both the previously shown tick and the final tick, preventing + * crowding without the erratic endpoint disappearance that a purely + * forward-scanning overlap check can produce. + * + *

Gap is measured in font-height units so it matches how a human + * perceives the spacing of 90°-rotated label text. + * + * @param majorTicks Ordered list of major ticks + * @param sm Font metrics for the current scale font + * @return Boolean mask (same length as {@code majorTicks}); + * {@code true} means the label should be painted */ - private Rectangle drawTickLabel(final Graphics2D gc, final int screen_y, final String mark, final boolean floating, final Rectangle avoid) + private boolean[] computeTickLabelVisibility( + final List> majorTicks, final FontMetrics sm) + { + final int n = majorTicks.size(); + final boolean[] show = new boolean[n]; + + // Locate first and last ticks that carry a non-empty label + int firstIdx = -1, lastIdx = -1; + for (int i = 0; i < n; i++) + if (!majorTicks.get(i).getLabel().isEmpty()) + { + if (firstIdx < 0) firstIdx = i; + lastIdx = i; + } + if (firstIdx < 0) + return show; // nothing to show + + show[firstIdx] = true; + if (lastIdx > firstIdx) + show[lastIdx] = true; + + // One font-height of breathing room prevents labels from running together + // (e.g. "1E11E4"). For rotated labels the physical extent along the axis + // equals stringWidth; for horizontal labels it equals getHeight(). + final int charGap = sm.getHeight(); + final int lastExtent = perpendicular_tick_labels + ? sm.getHeight() + : sm.stringWidth(majorTicks.get(lastIdx).getLabel()); + final int yLast = getScreenCoord(majorTicks.get(lastIdx).getValue()); + int prevIdx = firstIdx; + int yPrev = getScreenCoord(majorTicks.get(firstIdx).getValue()); + + for (int i = firstIdx + 1; i < lastIdx; i++) + { + final String lbl = majorTicks.get(i).getLabel(); + if (lbl.isEmpty()) + continue; + final int y = getScreenCoord(majorTicks.get(i).getValue()); + final int extent = perpendicular_tick_labels ? sm.getHeight() : sm.stringWidth(lbl); + final int prevExtent = perpendicular_tick_labels ? sm.getHeight() : sm.stringWidth(majorTicks.get(prevIdx).getLabel()); + final int minFromPrev = extent / 2 + prevExtent / 2 + charGap; + final int minFromLast = extent / 2 + lastExtent / 2 + charGap; + if (Math.abs(y - yPrev) >= minFromPrev && Math.abs(yLast - y) >= minFromLast) + { + show[i] = true; + yPrev = y; + prevIdx = i; + } + } + return show; + } + + /** Draw a single tick label at the given screen position. + * + * @param gc Graphics context + * @param screen_y Pixel position along the axis + * @param mark Label text to draw + * @param floating When {@code true}, surround the label with a floating box + */ + private void drawTickLabel(final Graphics2D gc, final int screen_y, final String mark, final boolean floating) { final Rectangle region = getBounds(); gc.setFont(scale_font); final FontMetrics metrics = gc.getFontMetrics(); + + if (perpendicular_tick_labels) + { + final int mark_width = metrics.stringWidth(mark); + final int mark_height = metrics.getHeight(); + final int x = is_right ? region.x + TICK_LENGTH + : region.x + region.width - TICK_LENGTH - mark_width; + // Vertically centre the baseline on screen_y + final int y_baseline = screen_y + metrics.getAscent() - mark_height / 2; + + if (floating) + { + final Rectangle outline = new Rectangle(x - BORDER, screen_y - mark_height/2 - BORDER, + mark_width + 2*BORDER, mark_height + 2*BORDER); + if (is_right) + gc.drawLine(x - TICK_LENGTH, screen_y, x, screen_y); + else + gc.drawLine(x + mark_width, screen_y, x + mark_width + TICK_LENGTH, screen_y); + gc.clearRect(outline.x, outline.y, outline.width, outline.height); + gc.drawRect(outline.x, outline.y, outline.width, outline.height); + } + gc.drawString(mark, x, y_baseline); + return; + } + + // Rotated (default) path final int mark_height = metrics.stringWidth(mark); final int mark_width = metrics.getHeight(); final int x = is_right ? region.x + TICK_LENGTH : region.x + region.width - TICK_LENGTH - mark_width; - int y = screen_y - mark_height/2; - // Correct location of top label to remain within region + int y = screen_y - mark_height / 2; + // Clamp only to keep label from going above the top of the image. + // Do NOT clamp to region boundaries — that would push endpoint labels + // inward, making them physically closer to neighbours than the + // visibility pre-check expected, causing visual overlap. if (y < 0) y = 0; - final Rectangle outline = new Rectangle(x-BORDER, y-BORDER, mark_width+2*BORDER, mark_height+2*BORDER); if (floating) { + final Rectangle outline = new Rectangle(x - BORDER, y - BORDER, mark_width + 2*BORDER, mark_height + 2*BORDER); if (is_right) gc.drawLine(x - TICK_LENGTH, screen_y, x, screen_y); else gc.drawLine(x + mark_width, screen_y, x + mark_width + TICK_LENGTH, screen_y); - - // Box around label gc.clearRect(outline.x, outline.y, outline.width, outline.height); gc.drawRect(outline.x, outline.y, outline.width, outline.height); } - if (avoid != null && outline.intersects(avoid)) - return avoid; - // Debug: Outline of text - // gc.drawRect(x, y, mark_width, mark_height); // Debug outline of tick label - GraphicsUtils.drawVerticalText(gc, x, y, mark, !is_right); - return outline; + GraphicsUtils.drawVerticalText(gc, x, y, mark, force_text_up || !is_right); } /** {@inheritDoc} */ @@ -434,6 +611,6 @@ public void drawTickLabel(final Graphics2D gc, final Double tick) final int y0 = getScreenCoord(tick); final String mark = ticks.formatDetailed(tick); - drawTickLabel(gc, y0, mark, true, null); + drawTickLabel(gc, y0, mark, true); } } diff --git a/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/LogTicksTest.java b/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/LogTicksTest.java index 1aa8cf7969..dc5867e9de 100644 --- a/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/LogTicksTest.java +++ b/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/LogTicksTest.java @@ -10,10 +10,21 @@ import org.csstudio.javafx.rtplot.internal.LogTicks; import org.junit.jupiter.api.Test; +import java.awt.FontMetrics; + import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; -/** JUnit test +/** JUnit test for {@link LogTicks}. + * + *

Each test case prints the tick text produced by {@link TicksTestBase#ticks2text} + * so failures are easy to diagnose. In the output, labels enclosed in single + * quotes ({@code 'label'}) are major (decade) ticks; bare values are minor ticks. + * + *

All decade labels use compact exponential notation (e.g. {@code 1E0}, + * {@code 1E3}) for visual consistency. A future {@code propFormat} / + * {@code propPrecision} property will let users override this per-widget. + * * @author Kay Kasemir */ @SuppressWarnings("nls") @@ -24,7 +35,7 @@ public void testLogTicks() { final LogTicks ticks = new LogTicks(); - // 'Normal' log scale with majors at 1E0, 1E1, 1E2, .. + // Four-decade range: all decades labeled, sub-decade values shown as minor ticks. double start = 1.0, end = 10000.0; ticks.compute(start, end, gc, buf.getWidth()); System.out.println("Ticks for " + start + " .. " + end + ":"); @@ -32,21 +43,70 @@ public void testLogTicks() System.out.println(text); assertThat(text, equalTo("'1E0' 1E0 2E0 3E0 4E0 5E0 6E0 7E0 8E0 9E0 '1E1' 1E1 2E1 3E1 4E1 5E1 6E1 7E1 8E1 9E1 '1E2' 1E2 2E2 3E2 4E2 5E2 6E2 7E2 8E2 9E2 '1E3' 1E3 2E3 3E3 4E3 5E3 6E3 7E3 8E3 9E3 '1E4' 1E4 ")); - // Wider log scale with majors at 1E0, 1E2, 1E4, .. + // Nine-decade range: all decades still fit in the 400 px test buffer + // (font height ≈ 14 px → ~20 labels fit; 9 decades ≤ 20). start = 1.0; end = 1e8; ticks.compute(start, end, gc, buf.getWidth()); System.out.println("Ticks for " + start + " .. " + end + ":"); text = ticks2text(ticks); System.out.println(text); - assertThat(text, equalTo("'1E0' '1E2' '1E4' '1E6' '1E8' ")); + assertThat(text, equalTo("'1E0' 1E0 2E0 3E0 4E0 5E0 6E0 7E0 8E0 9E0 '1E1' 1E1 2E1 3E1 4E1 5E1 6E1 7E1 8E1 9E1 '1E2' 1E2 2E2 3E2 4E2 5E2 6E2 7E2 8E2 9E2 '1E3' 1E3 2E3 3E3 4E3 5E3 6E3 7E3 8E3 9E3 '1E4' 1E4 2E4 3E4 4E4 5E4 6E4 7E4 8E4 9E4 '1E5' 1E5 2E5 3E5 4E5 5E5 6E5 7E5 8E5 9E5 '1E6' 1E6 2E6 3E6 4E6 5E6 6E6 7E6 8E6 9E6 '1E7' 1E7 2E7 3E7 4E7 5E7 6E7 7E7 8E7 9E7 '1E8' 1E8 ")); - // Log scale with same exponents for all ticks + // Sub-decade range: the log tick strategy yields fewer than two decade + // ticks, so a linear spacing is used as a fallback. start = 1001.0; end = 1234.0; ticks.compute(start, end, gc, buf.getWidth()); System.out.println("Ticks for " + start + " .. " + end + ":"); text = ticks2text(ticks); System.out.println(text); - assertThat(text, equalTo("'1.05E3' '1.10E3' '1.15E3' '1.20E3' ")); + assertThat(text, equalTo("'1.025E3' '1.050E3' '1.075E3' '1.100E3' '1.125E3' '1.150E3' '1.175E3' '1.200E3' '1.225E3' ")); + + // Regression: even number of decades must always show the top tick label. + // 1E-3 to 1E4 spans 8 exponents {-3,-2,-1,0,1,2,3,4} — an even count. + start = 1e-3; end = 1e4; + ticks.compute(start, end, gc, buf.getWidth()); + System.out.println("Ticks for " + start + " .. " + end + ":"); + text = ticks2text(ticks); + System.out.println(text); + assertThat("Top tick must be present for even-decade range", + text.contains("'1E4'"), equalTo(true)); + assertThat("Bottom tick must be present for even-decade range", + text.contains("'1E-3'"), equalTo(true)); + } + + @Test + public void testMidpointSymmetry() + { + // Regression for the halving-loop bias: with an 11-decade range (E0..E10) + // and space for only 3 labels, the old halving algorithm produced the + // labeled set {0, 4, 10} — the middle label at 40% not 50%. + // The evenly-spaced selection must produce {0, 5, 10} — symmetric. + // + // We use a narrow axis so that numLabelsThatFit == 3. + // numLabelsThatFit uses measured string widths; "1E0" and "1E10" are + // the bottom/top labels for this range. At 3 labels the thinned set + // must contain the true midpoint E5. + final FontMetrics fm = gc.getFontMetrics(); + // Make the axis just wide enough for 3 labels (with FILL_PERCENTAGE=70): + // width * 70/100 / labelSize == 3 → width = 3 * labelSize * 100/70 + 1 + // Use the ticks object to format sample labels identically to production. + final LogTicks sample = new LogTicks(); + sample.compute(1.0, 1e10, gc, 10000); // compute into a very wide axis so all decades are labeled + // The widest label in range 1..1E10 is "1E10" (5 chars with sign). + final int labelSize = Math.max( + fm.stringWidth(sample.format(1.0)), + fm.stringWidth(sample.format(1e10))); + final int width = 3 * labelSize * 100 / 70 + 1; + + final LogTicks ticks = new LogTicks(); + ticks.compute(1.0, 1e10, gc, width); + System.out.println("Ticks for 1.0 .. 1E10 (width=" + width + ", labelSize=" + labelSize + "):"); + final String text = ticks2text(ticks); + System.out.println(text); + assertThat("Bottom label must be present", text.contains("'1E0'"), equalTo(true)); + assertThat("Middle label must be at exponent 5 (true midpoint)", + text.contains("'1E5'"), equalTo(true)); + assertThat("Top label must be present", text.contains("'1E10'"), equalTo(true)); } } diff --git a/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java b/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java new file mode 100644 index 0000000000..8b44379542 --- /dev/null +++ b/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java @@ -0,0 +1,90 @@ +/******************************************************************************* + * Copyright (c) 2025 Oak Ridge National Laboratory. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + *******************************************************************************/ +package org.csstudio.javafx.rtplot; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.MatcherAssert.assertThat; + +/** JUnit tests for {@link RTTank}. + * + *

These are basic construction and API smoke tests. + * Full visual testing requires a running JavaFX toolkit (see TankDemo). + * + * @author Heredie Delvalle + */ +@SuppressWarnings("nls") +public class RTTankTest +{ + /** RTTank must be constructable without a JavaFX toolkit for + * headless CI — Canvas extends Node but the constructor should + * not require a Stage. + */ + @Test + public void testConstruction() + { + // This will throw if the right_scale.setOnRight(true) ordering + // is broken (NPE on update_throttle). + final RTTank tank = new RTTank(); + assertThat(tank, not(nullValue())); + } + + /** setRange should reject invalid ranges */ + @Test + public void testSetRangeRejectsInvalid() + { + final RTTank tank = new RTTank(); + // Should silently ignore these — no exception + tank.setRange(Double.NaN, 100); + tank.setRange(0, Double.NaN); + tank.setRange(100, 100); // flat + tank.setRange(100, 0); // inverted + tank.setRange(Double.POSITIVE_INFINITY, 100); + } + + /** setValue should handle NaN and Infinity */ + @Test + public void testSetValueEdgeCases() + { + final RTTank tank = new RTTank(); + tank.setRange(0, 100); + // Should not throw + tank.setValue(Double.NaN); + tank.setValue(Double.POSITIVE_INFINITY); + tank.setValue(Double.NEGATIVE_INFINITY); + tank.setValue(50); + } + + /** setLimits should accept any combination of NaN values */ + @Test + public void testSetLimits() + { + final RTTank tank = new RTTank(); + // All NaN — no lines drawn + tank.setLimits(Double.NaN, Double.NaN, Double.NaN, Double.NaN); + // Partial + tank.setLimits(10, Double.NaN, 90, Double.NaN); + // All set + tank.setLimits(10, 20, 80, 90); + } + + /** Verify the dual-scale toggle does not crash */ + @Test + public void testDualScale() + { + final RTTank tank = new RTTank(); + tank.setScaleVisible(true); + tank.setRightScaleVisible(true); + // Both hidden + tank.setScaleVisible(false); + tank.setRightScaleVisible(false); + } +} From 4f28cbd9a554a35c34677a7586d46b6bb400dc72 Mon Sep 17 00:00:00 2001 From: Emilio Heredia Date: Thu, 19 Mar 2026 15:52:11 -0600 Subject: [PATCH 2/7] feat(display): add ScaledPVWidget base class; extend TankWidget with alarm limits, dual scale, and format controls ScaledPVWidget (new abstract class, extends PVWidget): Consolidates properties common to scaled PV monitor widgets, following the pattern of CS-Studio BOY's AbstractScaledWidget hierarchy. Properties added on top of PVWidget: limits_from_pv - use PV display range for min/max (existing) minimum / maximum - manual display range (existing, moved here) border_alarm_sensitive - alarm border (existing, repositioned for grouping) alarm_limits_from_pv - use PV alarm metadata for LOLO/LO/HI/HIHI show_alarm_limits - draw limit lines on the widget body level_lolo/lo/hi/hihi - manual alarm thresholds (NaN = inactive) minor_alarm_color / major_alarm_color - themed alarm line colors Implementation notes: * All new properties use types that stock Phoebus already knows (boolean, double, color) so old versions silently skip unknown XML elements; no ordinal-based enums are used (BooleanWidgetProperty throws on ordinals >= 2, which broke round-trips in an earlier enum-based approach). * New properties default to values that reproduce existing behavior, so existing .bob files open with no visual difference. TankWidget (now extends ScaledPVWidget): New own properties: opposite_scale_visible - second scale on the right / bottom side show_minor_ticks - toggle minor tick marks perpendicular_tick_labels - rotate tick labels 90 degrees format - numeric format (DEFAULT, DECIMAL, ...) precision - decimal places (0-15) log_scale - logarithmic axis horizontal - horizontal tank orientation (existing) TankRepresentation: Wires all new model properties to RTTank. valueChanged() reads alarm metadata from the PV (VType -> Display -> warning/alarmRange) when alarm_limits_from_pv is true, falling back to the manual levels otherwise. Passes a limitsFromPV flag to RTTank so it can render PV-sourced limits solid and manual limits dashed. --- .../display/builder/model/Messages.java | 6 + .../builder/model/widgets/ScaledPVWidget.java | 187 ++++++++++++++++++ .../builder/model/widgets/TankWidget.java | 92 +++++++-- .../display/builder/model/messages.properties | 6 + .../javafx/widgets/TankRepresentation.java | 82 +++++++- 5 files changed, 347 insertions(+), 26 deletions(-) create mode 100644 app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/Messages.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/Messages.java index f0c46cd261..1f56ddea77 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/Messages.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/Messages.java @@ -169,6 +169,7 @@ public class Messages WidgetCategory_Plots, WidgetCategory_Structure, WidgetProperties_Actions, + WidgetProperties_AlarmLimitsFromPV, WidgetProperties_AngleSize, WidgetProperties_AngleStart, WidgetProperties_ArrayIndex, @@ -262,6 +263,7 @@ public class Messages WidgetProperties_Locale, WidgetProperties_LogScale, WidgetProperties_Macros, + WidgetProperties_MajorAlarmColor, WidgetProperties_MajorTickSpace, WidgetProperties_MajorTickStepHint, WidgetProperties_MajorTickVisible, @@ -271,6 +273,7 @@ public class Messages WidgetProperties_MinMaxTolerance, WidgetProperties_MinorTickSpace, WidgetProperties_MinorTickVisible, + WidgetProperties_MinorAlarmColor, WidgetProperties_MinuteColor, WidgetProperties_MinuteTickMarkColor, WidgetProperties_MinuteTickMarkVisible, @@ -305,6 +308,7 @@ public class Messages WidgetProperties_ScaleFactor, WidgetProperties_ScaleFormat, WidgetProperties_ScaleDirection, + WidgetProperties_OppositeScaleVisible, WidgetProperties_ScaleVisible, WidgetProperties_SecondColor, WidgetProperties_SecondVisible, @@ -316,10 +320,12 @@ public class Messages WidgetProperties_ShowHiHi, WidgetProperties_ShowIndex, WidgetProperties_ShowLED, + WidgetProperties_ShowAlarmLimits, WidgetProperties_ShowLimits, WidgetProperties_ShowLow, WidgetProperties_ShowLoLo, WidgetProperties_ShowMinorTicks, + WidgetProperties_PerpendicularTickLabels, WidgetProperties_ShowOK, WidgetProperties_ShowScale, WidgetProperties_ShowUnits, diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java new file mode 100644 index 0000000000..c04473d4ff --- /dev/null +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java @@ -0,0 +1,187 @@ +/******************************************************************************* + * Copyright (c) 2015-2025 Oak Ridge National Laboratory. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + *******************************************************************************/ +package org.csstudio.display.builder.model.widgets; + +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newBooleanPropertyDescriptor; +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newColorPropertyDescriptor; +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newDoublePropertyDescriptor; +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propLimitsFromPV; +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propMaximum; +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propMinimum; + +import java.util.List; + +import org.csstudio.display.builder.model.Messages; +import org.csstudio.display.builder.model.WidgetProperty; +import org.csstudio.display.builder.model.WidgetPropertyCategory; +import org.csstudio.display.builder.model.WidgetPropertyDescriptor; +import org.csstudio.display.builder.model.persist.NamedWidgetColors; +import org.csstudio.display.builder.model.persist.WidgetColorService; +import org.csstudio.display.builder.model.properties.WidgetColor; + +/** Base class for PV widgets that display a numeric value on a scale + * (Tank, Thermometer, ProgressBar and similar). + * + *

Consolidates properties that were historically duplicated across + * individual scaled widgets: display range, limits-from-PV toggle, + * alarm-level thresholds and their colours. This follows the pattern + * established by CS-Studio BOY's {@code LinearMeterWidget} which exposed + * LOLO/LO/HI/HIHI levels with both PV-sourced and manual modes. + * + *

Provides: + *

+ * + *

Backward compatibility note: every new property uses a type that + * stock Phoebus already knows (boolean, double, color). Old versions + * will silently skip the unknown XML elements; saving in old Phoebus + * will simply drop them. No ordinal-based enums are used, specifically + * to avoid the round-trip failure that {@code BooleanWidgetProperty} + * causes when it encounters an ordinal ≥ 2. + * + * @author Kay Kasemir + * @author Heredie Delvalle — CLS, alarm limits, scale refactoring + */ +@SuppressWarnings("nls") +public abstract class ScaledPVWidget extends PVWidget +{ + // ---- Property descriptors ------------------------------------------- + + /** 'alarm_limits_from_pv' — use PV alarm metadata for LOLO/LO/HI/HIHI? */ + public static final WidgetPropertyDescriptor propAlarmLimitsFromPV = + newBooleanPropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "alarm_limits_from_pv", + Messages.WidgetProperties_AlarmLimitsFromPV); + + /** 'show_alarm_limits' — draw LOLO/LO/HI/HIHI alarm limit markers on the scale */ + public static final WidgetPropertyDescriptor propShowAlarmLimits = + newBooleanPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "show_alarm_limits", + Messages.WidgetProperties_ShowAlarmLimits); + + /** 'level_lolo' — LOLO (major alarm) lower threshold; {@code NaN} = inactive */ + public static final WidgetPropertyDescriptor propLevelLoLo = + newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_lolo", + Messages.WidgetProperties_LevelLoLo); + + /** 'level_lo' — LO (minor warning) lower threshold; {@code NaN} = inactive */ + public static final WidgetPropertyDescriptor propLevelLow = + newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_lo", + Messages.WidgetProperties_LevelLow); + + /** 'level_hi' — HI (minor warning) upper threshold; {@code NaN} = inactive */ + public static final WidgetPropertyDescriptor propLevelHigh = + newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_hi", + Messages.WidgetProperties_LevelHigh); + + /** 'level_hihi' — HIHI (major alarm) upper threshold; {@code NaN} = inactive */ + public static final WidgetPropertyDescriptor propLevelHiHi = + newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_hihi", + Messages.WidgetProperties_LevelHiHi); + + /** 'minor_alarm_color' — color for LO / HI lines; defaults to named MINOR alarm color */ + public static final WidgetPropertyDescriptor propMinorAlarmColor = + newColorPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "minor_alarm_color", + Messages.WidgetProperties_MinorAlarmColor); + + /** 'major_alarm_color' — color for LOLO / HIHI lines; defaults to named MAJOR alarm color */ + public static final WidgetPropertyDescriptor propMajorAlarmColor = + newColorPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "major_alarm_color", + Messages.WidgetProperties_MajorAlarmColor); + + // ---- Instance fields ------------------------------------------------ + + private volatile WidgetProperty limits_from_pv; + private volatile WidgetProperty alarm_limits_from_pv; + private volatile WidgetProperty minimum; + private volatile WidgetProperty maximum; + private volatile WidgetProperty show_alarm_limits; + private volatile WidgetProperty level_lolo; + private volatile WidgetProperty level_low; + private volatile WidgetProperty level_high; + private volatile WidgetProperty level_hihi; + private volatile WidgetProperty minor_alarm_color; + private volatile WidgetProperty major_alarm_color; + + protected ScaledPVWidget(final String type, final int default_width, final int default_height) + { + super(type, default_width, default_height); + } + + /** Reorder properties so that alarm-related items sit together. + * {@code border_alarm_sensitive} (from PVWidget) is moved down to + * appear next to {@code alarm_limits_from_pv}. + */ + @Override + protected void defineProperties(final List> properties) + { + super.defineProperties(properties); + + // Move border_alarm_sensitive (added by PVWidget) down so it sits + // next to the alarm-related limit properties. + final WidgetProperty alarm_border_prop = propBorderAlarmSensitive(); + properties.remove(alarm_border_prop); + + properties.add(limits_from_pv = propLimitsFromPV.createProperty(this, true)); + properties.add(minimum = propMinimum.createProperty(this, 0.0)); + properties.add(maximum = propMaximum.createProperty(this, 100.0)); + properties.add(alarm_border_prop); + properties.add(alarm_limits_from_pv = propAlarmLimitsFromPV.createProperty(this, true)); + properties.add(show_alarm_limits = propShowAlarmLimits.createProperty(this, false)); + properties.add(level_lolo = propLevelLoLo.createProperty(this, Double.NaN)); + properties.add(level_low = propLevelLow.createProperty(this, Double.NaN)); + properties.add(level_high = propLevelHigh.createProperty(this, Double.NaN)); + properties.add(level_hihi = propLevelHiHi.createProperty(this, Double.NaN)); + properties.add(minor_alarm_color = propMinorAlarmColor.createProperty(this, + WidgetColorService.getColor(NamedWidgetColors.ALARM_MINOR))); + properties.add(major_alarm_color = propMajorAlarmColor.createProperty(this, + WidgetColorService.getColor(NamedWidgetColors.ALARM_MAJOR))); + } + + /** @return 'limits_from_pv' property (min/max display range) */ + public WidgetProperty propLimitsFromPV() { return limits_from_pv; } + + /** @return 'alarm_limits_from_pv' property (LOLO/LO/HI/HIHI alarm levels) */ + public WidgetProperty propAlarmLimitsFromPV() { return alarm_limits_from_pv; } + + /** @return 'minimum' property */ + public WidgetProperty propMinimum() { return minimum; } + + /** @return 'maximum' property */ + public WidgetProperty propMaximum() { return maximum; } + + /** @return 'show_alarm_limits' property */ + public WidgetProperty propShowAlarmLimits() { return show_alarm_limits; } + + /** @return 'level_lolo' property */ + public WidgetProperty propLevelLoLo() { return level_lolo; } + + /** @return 'level_lo' property */ + public WidgetProperty propLevelLow() { return level_low; } + + /** @return 'level_hi' property */ + public WidgetProperty propLevelHigh() { return level_high; } + + /** @return 'level_hihi' property */ + public WidgetProperty propLevelHiHi() { return level_hihi; } + + /** @return 'minor_alarm_color' property */ + public WidgetProperty propMinorAlarmColor() { return minor_alarm_color; } + + /** @return 'major_alarm_color' property */ + public WidgetProperty propMajorAlarmColor() { return major_alarm_color; } +} diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java index edd621c8c2..2fb9343a1b 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java @@ -9,15 +9,16 @@ import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newBooleanPropertyDescriptor; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newColorPropertyDescriptor; +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newIntegerPropertyDescriptor; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propBackgroundColor; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propFillColor; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propFont; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propForegroundColor; -import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propLimitsFromPV; -import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propMaximum; -import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propMinimum; -import static org.csstudio.display.builder.model.widgets.plots.PlotWidgetProperties.propLogscale; +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propFormat; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propHorizontal; +import static org.csstudio.display.builder.model.widgets.plots.PlotWidgetProperties.propLogscale; + +import org.phoebus.ui.vtype.FormatOption; import java.util.Arrays; import java.util.List; @@ -42,10 +43,24 @@ import org.w3c.dom.Element; /** Widget that displays a tank with variable fill level + * + *

Extends {@link ScaledPVWidget} to inherit common scale/limit + * properties (min, max, alarm thresholds, limit colours). This avoids + * the property duplication that existed across Tank, ProgressBar and + * Thermometer. + * + *

Additional display properties include a configurable scale format, + * dual-scale support (left + right / top + bottom when horizontal), + * minor ticks, perpendicular label orientation, and log scaling. + * The dual-scale feature is modelled after CS-Studio BOY's tank which + * supported markers on both sides of the tank body. + * * @author Kay Kasemir + * @author Heredie Delvalle — CLS, ScaledPVWidget refactoring, + * dual scale, alarm limits, format/precision controls */ @SuppressWarnings("nls") -public class TankWidget extends PVWidget +public class TankWidget extends ScaledPVWidget { /** Widget descriptor */ public static final WidgetDescriptor WIDGET_DESCRIPTOR = @@ -69,6 +84,27 @@ public Widget createWidget() public static final WidgetPropertyDescriptor propScaleVisible = newBooleanPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "scale_visible", Messages.WidgetProperties_ScaleVisible); + /** 'show_minor_ticks' */ + public static final WidgetPropertyDescriptor propShowMinorTicks = + newBooleanPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "show_minor_ticks", Messages.WidgetProperties_ShowMinorTicks); + + /** 'perpendicular_tick_labels' — draw scale labels perpendicular + * to the axis direction (horizontal text beside vertical scale) + */ + public static final WidgetPropertyDescriptor propPerpendicularTickLabels = + newBooleanPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "perpendicular_tick_labels", Messages.WidgetProperties_PerpendicularTickLabels); + + /** 'opposite_scale_visible' — show a second scale on the opposite + * side of the tank (right for vertical, bottom for horizontal). + * Inspired by CS-Studio BOY which could show markers on both sides. + */ + public static final WidgetPropertyDescriptor propOppositeScaleVisible = + newBooleanPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "opposite_scale_visible", Messages.WidgetProperties_OppositeScaleVisible); + + /** 'precision' — number of decimal places (0..15) */ + public static final WidgetPropertyDescriptor propScalePrecision = + newIntegerPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "precision", Messages.WidgetProperties_Precision, 0, 15); + /** Widget configurator to read legacy *.opi files*/ private static class CustomConfigurator extends WidgetConfigurator { @@ -131,9 +167,11 @@ public WidgetConfigurator getConfigurator(final Version persisted_version) private volatile WidgetProperty fill_color; private volatile WidgetProperty empty_color; private volatile WidgetProperty scale_visible; - private volatile WidgetProperty limits_from_pv; - private volatile WidgetProperty minimum; - private volatile WidgetProperty maximum; + private volatile WidgetProperty show_minor_ticks; + private volatile WidgetProperty perpendicular_tick_labels; + private volatile WidgetProperty opposite_scale_visible; + private volatile WidgetProperty format; + private volatile WidgetProperty precision; private volatile WidgetProperty log_scale; private volatile WidgetProperty horizontal; @@ -154,9 +192,11 @@ protected void defineProperties(final List> properties) properties.add(fill_color = propFillColor.createProperty(this, new WidgetColor(0, 0, 255))); properties.add(empty_color = propEmptyColor.createProperty(this, new WidgetColor(192, 192, 192))); properties.add(scale_visible = propScaleVisible.createProperty(this, true)); - properties.add(limits_from_pv = propLimitsFromPV.createProperty(this, true)); - properties.add(minimum = propMinimum.createProperty(this, 0.0)); - properties.add(maximum = propMaximum.createProperty(this, 100.0)); + properties.add(opposite_scale_visible = propOppositeScaleVisible.createProperty(this, false)); + properties.add(show_minor_ticks = propShowMinorTicks.createProperty(this, true)); + properties.add(perpendicular_tick_labels = propPerpendicularTickLabels.createProperty(this, false)); + properties.add(format = propFormat.createProperty(this, FormatOption.DEFAULT)); + properties.add(precision = propScalePrecision.createProperty(this, 2)); properties.add(log_scale = propLogscale.createProperty(this, false)); properties.add(horizontal = propHorizontal.createProperty(this, false)); } @@ -206,22 +246,34 @@ public WidgetProperty propScaleVisible() return scale_visible; } - /** @return 'limits_from_pv' property */ - public WidgetProperty propLimitsFromPV() + /** @return 'show_minor_ticks' property */ + public WidgetProperty propShowMinorTicks() + { + return show_minor_ticks; + } + + /** @return 'perpendicular_tick_labels' property */ + public WidgetProperty propPerpendicularTickLabels() + { + return perpendicular_tick_labels; + } + + /** @return 'opposite_scale_visible' property */ + public WidgetProperty propOppositeScaleVisible() { - return limits_from_pv; + return opposite_scale_visible; } - /** @return 'minimum' property */ - public WidgetProperty propMinimum() + /** @return 'format' property */ + public WidgetProperty propFormat() { - return minimum; + return format; } - /** @return 'maximum' property */ - public WidgetProperty propMaximum() + /** @return 'precision' property */ + public WidgetProperty propPrecision() { - return maximum; + return precision; } /** @return 'log_scale' property */ diff --git a/app/display/model/src/main/resources/org/csstudio/display/builder/model/messages.properties b/app/display/model/src/main/resources/org/csstudio/display/builder/model/messages.properties index 9d66bed068..278fcc4753 100644 --- a/app/display/model/src/main/resources/org/csstudio/display/builder/model/messages.properties +++ b/app/display/model/src/main/resources/org/csstudio/display/builder/model/messages.properties @@ -155,6 +155,7 @@ WidgetCategory_Monitors=Monitors WidgetCategory_Plots=Plots WidgetCategory_Structure=Structure WidgetProperties_Actions=Actions +WidgetProperties_AlarmLimitsFromPV=Alarm limits from PV WidgetProperties_AngleSize=Angle Size WidgetProperties_AngleStart=Angle Start WidgetProperties_ArrayIndex=Array Index @@ -248,6 +249,7 @@ WidgetProperties_LineWidth=Line Width WidgetProperties_Locale=Locale WidgetProperties_LogScale=Logarithmic Scale WidgetProperties_Macros=Macros +WidgetProperties_MajorAlarmColor=Major alarm color (LOLO/HIHI) WidgetProperties_MajorTickSpace=Major Ticks Space WidgetProperties_MajorTickStepHint=Major Ticks Pixel Dist. WidgetProperties_MajorTickVisible=Major Ticks Visible @@ -257,6 +259,7 @@ WidgetProperties_Minimum=Minimum WidgetProperties_MinMaxTolerance=Min/Max Tolerance WidgetProperties_MinorTickSpace=Minor Ticks Space WidgetProperties_MinorTickVisible=Minor Ticks Visible +WidgetProperties_MinorAlarmColor=Minor alarm color (LO/HI) WidgetProperties_MinuteColor=Minute Color WidgetProperties_MinuteTickMarkColor=Minute Tick Mark Color WidgetProperties_MinuteTickMarkVisible=Minute Tick Mark Visible @@ -290,6 +293,7 @@ WidgetProperties_Running=Running WidgetProperties_ScaleFactor=Scale Factor WidgetProperties_ScaleFormat=Scale Format WidgetProperties_ScaleDirection=Scale Direction +WidgetProperties_OppositeScaleVisible=Opposite Scale Visible WidgetProperties_ScaleVisible=Scale Visible WidgetProperties_Scripts=Scripts WidgetProperties_SecondColor=Second Color @@ -301,10 +305,12 @@ WidgetProperties_ShowHigh=Show High WidgetProperties_ShowHiHi=Show HiHi WidgetProperties_ShowIndex=Show Index WidgetProperties_ShowLED=Show LED +WidgetProperties_ShowAlarmLimits=Show Alarm Limits WidgetProperties_ShowLimits=Show Limits WidgetProperties_ShowLow=Show Low WidgetProperties_ShowLoLo=Show LoLo WidgetProperties_ShowMinorTicks=Show minor ticks +WidgetProperties_PerpendicularTickLabels=Labels perpendicular to axis WidgetProperties_ShowOK=Show OK WidgetProperties_ShowScale=Show Scale WidgetProperties_ShowUnits=Show Units diff --git a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java index 418b1957ce..f1eb585e0a 100644 --- a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java +++ b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java @@ -18,6 +18,7 @@ import org.csstudio.display.builder.representation.Preferences; import org.csstudio.display.builder.representation.javafx.JFXUtil; import org.csstudio.javafx.rtplot.RTTank; +import org.epics.util.stats.Range; import org.epics.vtype.Display; import org.epics.vtype.VType; @@ -27,6 +28,8 @@ /** Creates JavaFX item for model widget * @author Kay Kasemir + * @author Heredie Delvalle — CLS, alarm limits, dual scale, + * format/precision wiring */ public class TankRepresentation extends RegionBaseRepresentation { @@ -57,7 +60,20 @@ protected void registerListeners() model_widget.propFillColor().addUntypedPropertyListener(lookListener); model_widget.propEmptyColor().addUntypedPropertyListener(lookListener); model_widget.propScaleVisible().addUntypedPropertyListener(lookListener); - + model_widget.propShowMinorTicks().addUntypedPropertyListener(lookListener); + model_widget.propPerpendicularTickLabels().addUntypedPropertyListener(lookListener); + model_widget.propFormat().addUntypedPropertyListener(lookListener); + model_widget.propPrecision().addUntypedPropertyListener(lookListener); + model_widget.propMinorAlarmColor().addUntypedPropertyListener(lookListener); + model_widget.propMajorAlarmColor().addUntypedPropertyListener(lookListener); + model_widget.propOppositeScaleVisible().addUntypedPropertyListener(lookListener); + + model_widget.propShowAlarmLimits().addUntypedPropertyListener(valueListener); + model_widget.propLevelLoLo().addUntypedPropertyListener(valueListener); + model_widget.propLevelLow().addUntypedPropertyListener(valueListener); + model_widget.propLevelHigh().addUntypedPropertyListener(valueListener); + model_widget.propLevelHiHi().addUntypedPropertyListener(valueListener); + model_widget.propAlarmLimitsFromPV().addUntypedPropertyListener(valueListener); model_widget.propLimitsFromPV().addUntypedPropertyListener(valueListener); model_widget.propMinimum().addUntypedPropertyListener(valueListener); model_widget.propMaximum().addUntypedPropertyListener(valueListener); @@ -78,7 +94,20 @@ protected void unregisterListeners() model_widget.propFillColor().removePropertyListener(lookListener); model_widget.propEmptyColor().removePropertyListener(lookListener); model_widget.propScaleVisible().removePropertyListener(lookListener); - + model_widget.propShowMinorTicks().removePropertyListener(lookListener); + model_widget.propPerpendicularTickLabels().removePropertyListener(lookListener); + model_widget.propFormat().removePropertyListener(lookListener); + model_widget.propPrecision().removePropertyListener(lookListener); + model_widget.propMinorAlarmColor().removePropertyListener(lookListener); + model_widget.propMajorAlarmColor().removePropertyListener(lookListener); + model_widget.propOppositeScaleVisible().removePropertyListener(lookListener); + + model_widget.propShowAlarmLimits().removePropertyListener(valueListener); + model_widget.propLevelLoLo().removePropertyListener(valueListener); + model_widget.propLevelLow().removePropertyListener(valueListener); + model_widget.propLevelHigh().removePropertyListener(valueListener); + model_widget.propLevelHiHi().removePropertyListener(valueListener); + model_widget.propAlarmLimitsFromPV().removePropertyListener(valueListener); model_widget.propLimitsFromPV().removePropertyListener(valueListener); model_widget.propMinimum().removePropertyListener(valueListener); model_widget.propMaximum().removePropertyListener(valueListener); @@ -98,13 +127,12 @@ private void valueChanged(final WidgetProperty property, final Object old_val { final VType vtype = model_widget.runtimePropValue().getValue(); - final boolean limits_from_pv = model_widget.propLimitsFromPV().getValue(); + // --- Min / Max --- double min_val = model_widget.propMinimum().getValue(); double max_val = model_widget.propMaximum().getValue(); - if (limits_from_pv) + if (model_widget.propLimitsFromPV().getValue()) { - // Try display range from PV - final org.epics.vtype.Display display_info = Display.displayOf(vtype); + final Display display_info = Display.displayOf(vtype); if (display_info != null && display_info.getDisplayRange().isFinite()) { min_val = display_info.getDisplayRange().getMinimum(); @@ -113,6 +141,40 @@ private void valueChanged(final WidgetProperty property, final Object old_val } tank.setRange(min_val, max_val); + // --- Alarm limit lines --- + if (model_widget.propShowAlarmLimits().getValue()) + { + double lolo, lo, hi, hihi; + if (model_widget.propAlarmLimitsFromPV().getValue()) + { // Read from PV alarm metadata + final Display display_info = Display.displayOf(vtype); + if (display_info != null) + { + final Range minor = display_info.getWarningRange(); + final Range major = display_info.getAlarmRange(); + lo = minor.getMinimum(); + hi = minor.getMaximum(); + lolo = major.getMinimum(); + hihi = major.getMaximum(); + } + else + { // PV connected but no metadata yet — show nothing + lolo = lo = hi = hihi = Double.NaN; + } + } + else + { // Read from widget properties + lolo = model_widget.propLevelLoLo().getValue(); + lo = model_widget.propLevelLow().getValue(); + hi = model_widget.propLevelHigh().getValue(); + hihi = model_widget.propLevelHiHi().getValue(); + } + tank.setLimits(lolo, lo, hi, hihi); + tank.setLimitsFromPV(model_widget.propAlarmLimitsFromPV().getValue()); + } + else + tank.setLimits(Double.NaN, Double.NaN, Double.NaN, Double.NaN); + double value; if (toolkit.isEditMode()) value = (min_val + max_val) / 2; @@ -168,6 +230,14 @@ public void updateChanges() tank.setFillColor(JFXUtil.convert(model_widget.propFillColor().getValue())); tank.setEmptyColor(JFXUtil.convert(model_widget.propEmptyColor().getValue())); tank.setScaleVisible(model_widget.propScaleVisible().getValue()); + tank.setShowMinorTicks(model_widget.propShowMinorTicks().getValue()); + tank.setPerpendicularTickLabels(model_widget.propPerpendicularTickLabels().getValue()); + tank.setLabelFormat(model_widget.propFormat().getValue(), + model_widget.propPrecision().getValue()); + tank.setAlarmColors( + JFXUtil.convert(model_widget.propMinorAlarmColor().getValue()), + JFXUtil.convert(model_widget.propMajorAlarmColor().getValue())); + tank.setRightScaleVisible(model_widget.propOppositeScaleVisible().getValue()); } } } From 729fb44a4be57283595a0d05c2831e0f82706464 Mon Sep 17 00:00:00 2001 From: Emilio Heredia Date: Thu, 19 Mar 2026 15:52:33 -0600 Subject: [PATCH 3/7] test: add TankWidgetUnitTest and update PR_DESCRIPTION TankWidgetUnitTest (JUnit 5 + hamcrest, 5 tests): testScaledPVWidgetDefaults - verifies all ScaledPVWidget property defaults testTankWidgetDefaults - verifies TankWidget-specific property defaults testPropertyOrdering - alarm props grouped after max; opposite_scale adjacent to scale_visible testXmlRoundTrip - non-default values survive write/read cycle testNewPropertiesAreOptional - default-valued properties omitted from XML (backward-compatible with stock Phoebus) PR_DESCRIPTION.md documents the changes, rationale, backward compatibility analysis, and design decisions for the pull request. --- .../model/widgets/TankWidgetUnitTest.java | 212 ++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java diff --git a/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java b/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java new file mode 100644 index 0000000000..a27b913880 --- /dev/null +++ b/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java @@ -0,0 +1,212 @@ +/******************************************************************************* + * Copyright (c) 2025 Oak Ridge National Laboratory. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + *******************************************************************************/ +package org.csstudio.display.builder.model.widgets; + +import org.csstudio.display.builder.model.DisplayModel; +import org.csstudio.display.builder.model.Widget; +import org.csstudio.display.builder.model.WidgetProperty; +import org.csstudio.display.builder.model.persist.ModelReader; +import org.csstudio.display.builder.model.persist.ModelWriter; +import org.csstudio.display.builder.model.properties.WidgetColor; +import org.junit.jupiter.api.Test; +import org.phoebus.ui.vtype.FormatOption; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.util.List; +import java.util.stream.Collectors; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** JUnit tests for {@link TankWidget} and its {@link ScaledPVWidget} base class. + * + *

Verifies property defaults, property ordering in the panel, and + * XML round-trip persistence for backward-compatible new properties. + * + * @author Heredie Delvalle + */ +@SuppressWarnings("nls") +public class TankWidgetUnitTest +{ + /** Verify that ScaledPVWidget properties have sensible defaults */ + @Test + public void testScaledPVWidgetDefaults() + { + final TankWidget tank = new TankWidget(); + + // Limits-from-PV should default to true (upstream convention) + assertThat(tank.propLimitsFromPV().getValue(), equalTo(true)); + assertThat(tank.propAlarmLimitsFromPV().getValue(), equalTo(true)); + + // Display range + assertThat(tank.propMinimum().getValue(), equalTo(0.0)); + assertThat(tank.propMaximum().getValue(), equalTo(100.0)); + + // Limit lines hidden by default to avoid visual noise on existing .bob files + assertThat(tank.propShowAlarmLimits().getValue(), equalTo(false)); + + // Manual limit levels default to NaN (inactive) + assertTrue(Double.isNaN(tank.propLevelLoLo().getValue())); + assertTrue(Double.isNaN(tank.propLevelLow().getValue())); + assertTrue(Double.isNaN(tank.propLevelHigh().getValue())); + assertTrue(Double.isNaN(tank.propLevelHiHi().getValue())); + + // Alarm colours should reference the named palette entries + final WidgetColor minor = tank.propMinorAlarmColor().getValue(); + final WidgetColor major = tank.propMajorAlarmColor().getValue(); + assertNotNull(minor); + assertNotNull(major); + } + + /** Verify TankWidget-specific defaults */ + @Test + public void testTankWidgetDefaults() + { + final TankWidget tank = new TankWidget(); + + assertThat(tank.propScaleVisible().getValue(), equalTo(true)); + assertThat(tank.propOppositeScaleVisible().getValue(), equalTo(false)); + assertThat(tank.propShowMinorTicks().getValue(), equalTo(true)); + assertThat(tank.propPerpendicularTickLabels().getValue(), equalTo(false)); + assertThat(tank.propFormat().getValue(), equalTo(FormatOption.DEFAULT)); + assertThat(tank.propPrecision().getValue(), equalTo(2)); + assertThat(tank.propLogScale().getValue(), equalTo(false)); + assertThat(tank.propHorizontal().getValue(), equalTo(false)); + } + + /** Verify that alarm properties appear together and in the expected + * order when listed in the property panel. + * + *

{@code border_alarm_sensitive} and {@code alarm_limits_from_pv} + * should appear right after {@code maximum}. + */ + @Test + public void testPropertyOrdering() + { + final TankWidget tank = new TankWidget(); + final List names = tank.getProperties() + .stream() + .map(WidgetProperty::getName) + .collect(Collectors.toList()); + // border_alarm_sensitive comes after maximum, then alarm_limits_from_pv + final int maxIdx = names.indexOf("maximum"); + final int brdIdx = names.indexOf("border_alarm_sensitive"); + final int almIdx = names.indexOf("alarm_limits_from_pv"); + assertTrue(maxIdx >= 0, "maximum not found"); + assertTrue(brdIdx >= 0, "border_alarm_sensitive not found"); + assertTrue(almIdx >= 0, "alarm_limits_from_pv not found"); + assertTrue(brdIdx > maxIdx, + "border_alarm_sensitive should follow maximum"); + assertTrue(almIdx > brdIdx, + "alarm_limits_from_pv should follow border_alarm_sensitive"); + + // scale_visible and opposite_scale_visible should be adjacent + final int scaleIdx = names.indexOf("scale_visible"); + final int oppIdx = names.indexOf("opposite_scale_visible"); + assertTrue(scaleIdx >= 0); + assertTrue(oppIdx >= 0); + assertThat(oppIdx, equalTo(scaleIdx + 1)); + } + + /** Write a tank to XML and read it back, verifying non-default values + * survive the round trip. + */ + @Test + public void testXmlRoundTrip() throws Exception + { + // Create a tank with non-default settings + final TankWidget original = new TankWidget(); + original.propMinimum().setValue(10.0); + original.propMaximum().setValue(200.0); + original.propShowAlarmLimits().setValue(true); + original.propAlarmLimitsFromPV().setValue(false); + original.propLevelLoLo().setValue(15.0); + original.propLevelLow().setValue(30.0); + original.propLevelHigh().setValue(170.0); + original.propLevelHiHi().setValue(190.0); + original.propOppositeScaleVisible().setValue(true); + original.propPerpendicularTickLabels().setValue(true); + original.propFormat().setValue(FormatOption.DECIMAL); + original.propPrecision().setValue(3); + + // Serialize — disable skip_defaults so ALL properties appear + final DisplayModel model = new DisplayModel(); + model.runtimeChildren().addChild(original); + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final boolean saved = ModelWriter.skip_defaults; + ModelWriter.skip_defaults = false; + try + { + final ModelWriter writer = new ModelWriter(out); + writer.writeModel(model); + writer.close(); + } + finally + { + ModelWriter.skip_defaults = saved; + } + final String xml = out.toString(); + + // Quick sanity: new properties should appear in the XML + assertThat(xml, containsString("")); + assertThat(xml, containsString("")); + assertThat(xml, containsString("")); + assertThat(xml, containsString("")); + + // Deserialize + final ModelReader reader = new ModelReader(new ByteArrayInputStream(xml.getBytes())); + final DisplayModel loaded = reader.readModel(); + final Widget w = loaded.getChildren().get(0); + assertTrue(w instanceof TankWidget); + final TankWidget tank = (TankWidget) w; + + assertThat(tank.propMinimum().getValue(), equalTo(10.0)); + assertThat(tank.propMaximum().getValue(), equalTo(200.0)); + assertThat(tank.propShowAlarmLimits().getValue(), equalTo(true)); + assertThat(tank.propAlarmLimitsFromPV().getValue(), equalTo(false)); + assertThat(tank.propLevelLoLo().getValue(), equalTo(15.0)); + assertThat(tank.propLevelLow().getValue(), equalTo(30.0)); + assertThat(tank.propLevelHigh().getValue(), equalTo(170.0)); + assertThat(tank.propLevelHiHi().getValue(), equalTo(190.0)); + assertThat(tank.propOppositeScaleVisible().getValue(), equalTo(true)); + assertThat(tank.propPerpendicularTickLabels().getValue(), equalTo(true)); + assertThat(tank.propFormat().getValue(), equalTo(FormatOption.DECIMAL)); + assertThat(tank.propPrecision().getValue(), equalTo(3)); + } + + /** Verify that XML produced by this version can be read by a stock + * Phoebus that does not know the new properties: unknown elements + * are silently ignored. We simulate this by writing only the new + * properties and confirming they survive as XML text. + */ + @Test + public void testNewPropertiesAreOptional() throws Exception + { + // A default tank should NOT write alarm_limits_from_pv etc. because + // ModelWriter skips properties that equal their default. + final TankWidget tank = new TankWidget(); + final DisplayModel model = new DisplayModel(); + model.runtimeChildren().addChild(tank); + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final ModelWriter writer = new ModelWriter(out); + writer.writeModel(model); + writer.close(); + final String xml = out.toString(); + + // New properties at default values should be omitted + assertThat(xml, not(containsString(""))); + assertThat(xml, not(containsString(""))); + assertThat(xml, not(containsString(""))); + assertThat(xml, not(containsString(""))); + } +} From 5b24a418640224fe2093f766209f0f092e111826 Mon Sep 17 00:00:00 2001 From: Emilio Heredia Date: Sun, 22 Mar 2026 10:06:43 -0600 Subject: [PATCH 4/7] feat(display): add configurable tank_border_width property to TankWidget Add a 'tank_border_width' integer property (default 0, range 0-100 px) to TankWidget. When zero (the default) no border is drawn, preserving the original widget appearance and backward compatibility with existing .bob files. Changes: - RTTank: add border_width field, setBorderWidth(int) setter, and conditional drawRoundRect guarded by border_width > 0 - TankWidget: define propTankBorderWidth descriptor (DISPLAY category, key 'tank_border_width') and expose via propBorderWidth() accessor - TankRepresentation: wire propBorderWidth to tank.setBorderWidth() in register/unregister/updateChanges - TankWidgetUnitTest: assert default=0, round-trip value=3, and that is absent at defaults (ModelWriter skips defaults) Property is named 'tank_border_width' rather than re-using the generic 'border_width' (MISC) to avoid unintended interaction with the CSS border mechanism in RegionBaseRepresentation and to keep all tank display properties consistently in the DISPLAY category. --- .../builder/model/widgets/ScaledPVWidget.java | 8 ++-- .../builder/model/widgets/TankWidget.java | 15 +++++++ .../model/widgets/TankWidgetUnitTest.java | 5 +++ .../javafx/widgets/TankRepresentation.java | 10 +++-- .../org/csstudio/javafx/rtplot/RTTank.java | 43 ++++++++++++++++--- 5 files changed, 68 insertions(+), 13 deletions(-) diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java index c04473d4ff..99e43c950a 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java @@ -78,14 +78,14 @@ public abstract class ScaledPVWidget extends PVWidget newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_lolo", Messages.WidgetProperties_LevelLoLo); - /** 'level_lo' — LO (minor warning) lower threshold; {@code NaN} = inactive */ + /** 'level_low' — LO (minor warning) lower threshold; {@code NaN} = inactive */ public static final WidgetPropertyDescriptor propLevelLow = - newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_lo", + newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_low", Messages.WidgetProperties_LevelLow); - /** 'level_hi' — HI (minor warning) upper threshold; {@code NaN} = inactive */ + /** 'level_high' — HI (minor warning) upper threshold; {@code NaN} = inactive */ public static final WidgetPropertyDescriptor propLevelHigh = - newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_hi", + newDoublePropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "level_high", Messages.WidgetProperties_LevelHigh); /** 'level_hihi' — HIHI (major alarm) upper threshold; {@code NaN} = inactive */ diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java index 2fb9343a1b..5d2fa23abe 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java @@ -77,6 +77,13 @@ public Widget createWidget() } }; + /** 'tank_border_width' — width in pixels of the border drawn around the + * tank body; 0 (default) means no border, preserving the original look. + */ + public static final WidgetPropertyDescriptor propTankBorderWidth = + newIntegerPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "tank_border_width", + Messages.WidgetProperties_BorderWidth, 0, 100); + /** 'empty_color' */ public static final WidgetPropertyDescriptor propEmptyColor = newColorPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "empty_color", Messages.WidgetProperties_EmptyColor); @@ -174,6 +181,7 @@ public WidgetConfigurator getConfigurator(final Version persisted_version) private volatile WidgetProperty precision; private volatile WidgetProperty log_scale; private volatile WidgetProperty horizontal; + private volatile WidgetProperty border_width_prop; /** Constructor */ @@ -199,6 +207,7 @@ protected void defineProperties(final List> properties) properties.add(precision = propScalePrecision.createProperty(this, 2)); properties.add(log_scale = propLogscale.createProperty(this, false)); properties.add(horizontal = propHorizontal.createProperty(this, false)); + properties.add(border_width_prop = propTankBorderWidth.createProperty(this, 0)); } @Override @@ -287,4 +296,10 @@ public WidgetProperty propHorizontal() { return horizontal; } + + /** @return 'border_width' property (0 = no border) */ + public WidgetProperty propBorderWidth() + { + return border_width_prop; + } } diff --git a/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java b/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java index a27b913880..e9abf39cd1 100644 --- a/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java +++ b/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java @@ -82,6 +82,7 @@ public void testTankWidgetDefaults() assertThat(tank.propPrecision().getValue(), equalTo(2)); assertThat(tank.propLogScale().getValue(), equalTo(false)); assertThat(tank.propHorizontal().getValue(), equalTo(false)); + assertThat(tank.propBorderWidth().getValue(), equalTo(0)); } /** Verify that alarm properties appear together and in the expected @@ -135,6 +136,7 @@ public void testXmlRoundTrip() throws Exception original.propLevelHigh().setValue(170.0); original.propLevelHiHi().setValue(190.0); original.propOppositeScaleVisible().setValue(true); + original.propBorderWidth().setValue(3); original.propPerpendicularTickLabels().setValue(true); original.propFormat().setValue(FormatOption.DECIMAL); original.propPrecision().setValue(3); @@ -161,6 +163,7 @@ public void testXmlRoundTrip() throws Exception assertThat(xml, containsString("")); assertThat(xml, containsString("")); assertThat(xml, containsString("")); + assertThat(xml, containsString("")); assertThat(xml, containsString("")); // Deserialize @@ -179,6 +182,7 @@ public void testXmlRoundTrip() throws Exception assertThat(tank.propLevelHigh().getValue(), equalTo(170.0)); assertThat(tank.propLevelHiHi().getValue(), equalTo(190.0)); assertThat(tank.propOppositeScaleVisible().getValue(), equalTo(true)); + assertThat(tank.propBorderWidth().getValue(), equalTo(3)); assertThat(tank.propPerpendicularTickLabels().getValue(), equalTo(true)); assertThat(tank.propFormat().getValue(), equalTo(FormatOption.DECIMAL)); assertThat(tank.propPrecision().getValue(), equalTo(3)); @@ -208,5 +212,6 @@ public void testNewPropertiesAreOptional() throws Exception assertThat(xml, not(containsString(""))); assertThat(xml, not(containsString(""))); assertThat(xml, not(containsString(""))); + assertThat(xml, not(containsString(""))); } } diff --git a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java index f1eb585e0a..f5d381a8d1 100644 --- a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java +++ b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java @@ -67,6 +67,8 @@ protected void registerListeners() model_widget.propMinorAlarmColor().addUntypedPropertyListener(lookListener); model_widget.propMajorAlarmColor().addUntypedPropertyListener(lookListener); model_widget.propOppositeScaleVisible().addUntypedPropertyListener(lookListener); + model_widget.propBorderWidth().addUntypedPropertyListener(lookListener); + model_widget.propLogScale().addUntypedPropertyListener(lookListener); model_widget.propShowAlarmLimits().addUntypedPropertyListener(valueListener); model_widget.propLevelLoLo().addUntypedPropertyListener(valueListener); @@ -77,7 +79,6 @@ protected void registerListeners() model_widget.propLimitsFromPV().addUntypedPropertyListener(valueListener); model_widget.propMinimum().addUntypedPropertyListener(valueListener); model_widget.propMaximum().addUntypedPropertyListener(valueListener); - model_widget.propLogScale().addUntypedPropertyListener(valueListener); model_widget.runtimePropValue().addUntypedPropertyListener(valueListener); model_widget.propHorizontal().addPropertyListener(orientationChangedListener); valueChanged(null, null, null); @@ -101,6 +102,8 @@ protected void unregisterListeners() model_widget.propMinorAlarmColor().removePropertyListener(lookListener); model_widget.propMajorAlarmColor().removePropertyListener(lookListener); model_widget.propOppositeScaleVisible().removePropertyListener(lookListener); + model_widget.propBorderWidth().removePropertyListener(lookListener); + model_widget.propLogScale().removePropertyListener(lookListener); model_widget.propShowAlarmLimits().removePropertyListener(valueListener); model_widget.propLevelLoLo().removePropertyListener(valueListener); @@ -111,7 +114,6 @@ protected void unregisterListeners() model_widget.propLimitsFromPV().removePropertyListener(valueListener); model_widget.propMinimum().removePropertyListener(valueListener); model_widget.propMaximum().removePropertyListener(valueListener); - model_widget.propLogScale().removePropertyListener(valueListener); model_widget.runtimePropValue().removePropertyListener(valueListener); model_widget.propHorizontal().removePropertyListener(orientationChangedListener); super.unregisterListeners(); @@ -181,8 +183,6 @@ private void valueChanged(final WidgetProperty property, final Object old_val else value = VTypeUtil.getValueNumber(vtype).doubleValue(); tank.setValue(value); - - tank.setLogScale(model_widget.propLogScale().getValue()); } private void orientationChanged(final WidgetProperty prop, final Boolean old, final Boolean horizontal) @@ -232,12 +232,14 @@ public void updateChanges() tank.setScaleVisible(model_widget.propScaleVisible().getValue()); tank.setShowMinorTicks(model_widget.propShowMinorTicks().getValue()); tank.setPerpendicularTickLabels(model_widget.propPerpendicularTickLabels().getValue()); + tank.setLogScale(model_widget.propLogScale().getValue()); tank.setLabelFormat(model_widget.propFormat().getValue(), model_widget.propPrecision().getValue()); tank.setAlarmColors( JFXUtil.convert(model_widget.propMinorAlarmColor().getValue()), JFXUtil.convert(model_widget.propMajorAlarmColor().getValue())); tank.setRightScaleVisible(model_widget.propOppositeScaleVisible().getValue()); + tank.setBorderWidth(model_widget.propBorderWidth().getValue()); } } } diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java index 2f17a0dbb5..f8b9206769 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java @@ -90,6 +90,9 @@ public class RTTank extends Canvas /** Is the right-side scale displayed? */ private volatile boolean right_scale_visible = false; + /** Border width in pixels around the tank body; 0 = no border (default) */ + private volatile int border_width = 0; + /** Current value, i.e. fill level */ private volatile double value = 5.0; @@ -204,6 +207,13 @@ public void setFont(final Font font) right_scale.setScaleFont(font); } + /** @param width Border width in pixels around the tank body (0 = no border) */ + public void setBorderWidth(final int width) + { + border_width = Math.max(0, width); + requestUpdate(); + } + /** @param color Background color */ public void setBackground(final javafx.scene.paint.Color color) { @@ -284,11 +294,29 @@ else switch (format) case DECIMAL: fmt = LinearTicks.createDecimalFormat(prec); break; - case ENGINEERING: case EXPONENTIAL: - default: fmt = LinearTicks.createExponentialFormat(prec); break; + case ENGINEERING: + // Engineering format constrains the exponent to multiples of 3. + // RTTank places ticks via the axis algorithm which does not guarantee + // that constraint, so this is a best-effort approximation using + // exponential notation with the requested precision. + fmt = LinearTicks.createExponentialFormat(prec); + break; + case COMPACT: + // COMPACT picks decimal or exponential per value depending on magnitude. + // A scale axis applies one NumberFormat to all tick labels, so per-value + // switching cannot be expressed as a single static format. + // Fall back to automatic formatting, which already chooses a compact + // representation based on the axis range. + fmt = null; + break; + default: + // HEX, STRING, BINARY, SEXAGESIMAL, etc. are not meaningful for a + // numeric scale axis. Fall back to automatic formatting. + fmt = null; + break; } scale.setLabelFormat(fmt); right_scale.setLabelFormat(fmt); @@ -522,9 +550,14 @@ else if (scale.isLogarithmic()) // by mellguth2, https://github.com/ControlSyste else gc.fillRoundRect(plot_bounds.x, plot_bounds.y, plot_bounds.width, level, arc, arc); - // Dark outline around the tank body - gc.setColor(foreground); - gc.drawRoundRect(plot_bounds.x, plot_bounds.y, plot_bounds.width, plot_bounds.height, arc, arc); + // Optional border around the tank body + if (border_width > 0) + { + gc.setColor(foreground); + gc.setStroke(new BasicStroke(border_width)); + gc.drawRoundRect(plot_bounds.x, plot_bounds.y, plot_bounds.width, plot_bounds.height, arc, arc); + gc.setStroke(new BasicStroke(1f)); + } // Draw alarm / warning limit lines over the tank body final double lim_lolo = limit_lolo; From 99febe273acd9e2d85a05a0fa8845a2d10a103f1 Mon Sep 17 00:00:00 2001 From: Emilio Heredia Date: Sun, 29 Mar 2026 16:31:23 -0600 Subject: [PATCH 5/7] feat(ui): add Significant digits format option (FormatOption.SIGNIFICANT) Add a new 'Significant' entry to the FormatOption enum that uses Java's %g specifier, matching EDM's GFloat format. Precision controls the total number of significant digits rather than fraction digits. The formatter chooses decimal or exponential notation per value depending on magnitude: sig3: 0.00123 / 1.23 / 123 / 1.23e+04 Wired through FormatOptionHandler (for TextUpdate and other widgets) and RTTank.setLabelFormat (for axis tick labels via a NumberFormat adapter). Tests added in FormatOptionHandlerTest and RTTankTest. --- .../org/csstudio/javafx/rtplot/RTTank.java | 17 +++++++ .../csstudio/javafx/rtplot/RTTankTest.java | 15 +++++++ .../main/java/org/phoebus/ui/Messages.java | 2 + .../org/phoebus/ui/vtype/FormatOption.java | 7 ++- .../phoebus/ui/vtype/FormatOptionHandler.java | 2 + .../org/phoebus/ui/messages.properties | 1 + .../ui/vtype/FormatOptionHandlerTest.java | 44 +++++++++++++++++++ 7 files changed, 87 insertions(+), 1 deletion(-) diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java index f8b9206769..a2f58df813 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java @@ -312,6 +312,23 @@ else switch (format) // representation based on the axis range. fmt = null; break; + case SIGNIFICANT: + // Significant-digits formatting (like C/Java %g). Each tick value + // is individually formatted with the requested number of significant + // digits, choosing decimal or exponential notation per value. + final String pattern = "%." + prec + "g"; + fmt = new NumberFormat() { + @Override public StringBuffer format(double v, StringBuffer buf, java.text.FieldPosition pos) { + return buf.append(String.format(java.util.Locale.ROOT, pattern, v)); + } + @Override public StringBuffer format(long v, StringBuffer buf, java.text.FieldPosition pos) { + return buf.append(String.format(java.util.Locale.ROOT, pattern, (double) v)); + } + @Override public Number parse(String s, java.text.ParsePosition pos) { + throw new UnsupportedOperationException(); + } + }; + break; default: // HEX, STRING, BINARY, SEXAGESIMAL, etc. are not meaningful for a // numeric scale axis. Fall back to automatic formatting. diff --git a/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java b/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java index 8b44379542..5d576badd0 100644 --- a/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java +++ b/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java @@ -9,6 +9,8 @@ import org.junit.jupiter.api.Test; +import org.phoebus.ui.vtype.FormatOption; + import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; @@ -87,4 +89,17 @@ public void testDualScale() tank.setScaleVisible(false); tank.setRightScaleVisible(false); } + + /** setLabelFormat with SIGNIFICANT should not throw */ + @Test + public void testSignificantFormat() + { + final RTTank tank = new RTTank(); + // Should accept SIGNIFICANT without error + tank.setLabelFormat(FormatOption.SIGNIFICANT, 3); + tank.setLabelFormat(FormatOption.SIGNIFICANT, 1); + // Switching back to other formats should also work + tank.setLabelFormat(FormatOption.DECIMAL, 2); + tank.setLabelFormat(FormatOption.DEFAULT, 0); + } } diff --git a/core/ui/src/main/java/org/phoebus/ui/Messages.java b/core/ui/src/main/java/org/phoebus/ui/Messages.java index 02774b00aa..7c8c81cd13 100644 --- a/core/ui/src/main/java/org/phoebus/ui/Messages.java +++ b/core/ui/src/main/java/org/phoebus/ui/Messages.java @@ -55,6 +55,8 @@ public class Messages public static String Format_SexagesimalDMS; /**Format_SexagesimalHMS */ public static String Format_SexagesimalHMS; + /**Format_Significant */ + public static String Format_Significant; /**Format_String */ public static String Format_String; /**InstallExamples */ diff --git a/core/ui/src/main/java/org/phoebus/ui/vtype/FormatOption.java b/core/ui/src/main/java/org/phoebus/ui/vtype/FormatOption.java index 2aa91a79cc..86e27076c1 100644 --- a/core/ui/src/main/java/org/phoebus/ui/vtype/FormatOption.java +++ b/core/ui/src/main/java/org/phoebus/ui/vtype/FormatOption.java @@ -46,7 +46,12 @@ public enum FormatOption // Binary was added when PVA introduced it. /** Binary, precision determines the number of 01010101 */ - BINARY(Messages.Format_Binary, true); + BINARY(Messages.Format_Binary, true), + + /** Significant-digits formatting (C/Java {@code %g}). Precision determines + * the total number of significant digits rather than fraction digits. + * Chooses decimal or exponential notation per value depending on magnitude. */ + SIGNIFICANT(Messages.Format_Significant, true); // To remain compatible with previous versions of this enum, // new options must be added to the end. diff --git a/core/ui/src/main/java/org/phoebus/ui/vtype/FormatOptionHandler.java b/core/ui/src/main/java/org/phoebus/ui/vtype/FormatOptionHandler.java index d4962514c4..b13407d12c 100644 --- a/core/ui/src/main/java/org/phoebus/ui/vtype/FormatOptionHandler.java +++ b/core/ui/src/main/java/org/phoebus/ui/vtype/FormatOptionHandler.java @@ -284,6 +284,8 @@ private static String formatNumber(final Number value, final Display display, buf.insert(0, "0b"); return buf.toString(); } + if (option == FormatOption.SIGNIFICANT) + return String.format(LOCALE, "%." + precision + "g", value.doubleValue()); if (option == FormatOption.SEXAGESIMAL) return SexagesimalFormat.format(value.doubleValue(), precision); if (option == FormatOption.SEXAGESIMAL_HMS) diff --git a/core/ui/src/main/resources/org/phoebus/ui/messages.properties b/core/ui/src/main/resources/org/phoebus/ui/messages.properties index 2681f4c072..e49d2aefd0 100644 --- a/core/ui/src/main/resources/org/phoebus/ui/messages.properties +++ b/core/ui/src/main/resources/org/phoebus/ui/messages.properties @@ -18,6 +18,7 @@ Format_Hexadecimal=Hexadecimal Format_Sexagesimal=Sexagesimal HH:MM:SS Format_SexagesimalDMS=Sexagesimal DMS 360deg rad Format_SexagesimalHMS=Sexagesimal HMS 24h rad +Format_Significant=Significant Format_String=String InstallExamples=Install Example Displays MagicLastRow=Click to add row diff --git a/core/ui/src/test/java/org/phoebus/ui/vtype/FormatOptionHandlerTest.java b/core/ui/src/test/java/org/phoebus/ui/vtype/FormatOptionHandlerTest.java index 3e4072e915..2b253f7458 100644 --- a/core/ui/src/test/java/org/phoebus/ui/vtype/FormatOptionHandlerTest.java +++ b/core/ui/src/test/java/org/phoebus/ui/vtype/FormatOptionHandlerTest.java @@ -632,4 +632,48 @@ public void testStringArrayParsing() System.out.println(Arrays.toString((String[])parsed)); assertThat(parsed, equalTo(new String[] { "Al \"Ed\" Stone", "Jane" })); } + + @Test + public void testSignificant() + { + // Significant-digits formatting uses Java's %g specifier. + // Precision controls total significant digits, not fraction digits. + VType number = VDouble.of(0.001234, Alarm.none(), Time.now(), display); + String text = FormatOptionHandler.format(number, FormatOption.SIGNIFICANT, 3, false); + System.out.println(text); + assertThat(text, equalTo("0.00123")); + + number = VDouble.of(1.234, Alarm.none(), Time.now(), display); + text = FormatOptionHandler.format(number, FormatOption.SIGNIFICANT, 3, false); + System.out.println(text); + assertThat(text, equalTo("1.23")); + + number = VDouble.of(123.4, Alarm.none(), Time.now(), display); + text = FormatOptionHandler.format(number, FormatOption.SIGNIFICANT, 3, false); + System.out.println(text); + assertThat(text, equalTo("123")); + + number = VDouble.of(12340.0, Alarm.none(), Time.now(), display); + text = FormatOptionHandler.format(number, FormatOption.SIGNIFICANT, 3, false); + System.out.println(text); + assertThat(text, equalTo("1.23e+04")); + + // With units + number = VDouble.of(0.1234, Alarm.none(), Time.now(), display); + text = FormatOptionHandler.format(number, FormatOption.SIGNIFICANT, 3, true); + System.out.println(text); + assertThat(text, equalTo("0.123 V")); + + // Single significant digit + number = VDouble.of(12.34, Alarm.none(), Time.now(), display); + text = FormatOptionHandler.format(number, FormatOption.SIGNIFICANT, 1, false); + System.out.println(text); + assertThat(text, equalTo("1e+01")); + + // Zero + number = VDouble.of(0.0, Alarm.none(), Time.now(), display); + text = FormatOptionHandler.format(number, FormatOption.SIGNIFICANT, 3, false); + System.out.println(text); + assertThat(text, equalTo("0.00")); + } } From 7b615f9ef561c8d8f95b8038c3fa7d1828be8543 Mon Sep 17 00:00:00 2001 From: Emilio Heredia Date: Mon, 30 Mar 2026 11:12:34 -0600 Subject: [PATCH 6/7] refactor(display): replace FormatOption with ScaleFormat for scale widgets Introduce ScaleFormat enum in core/ui containing only the format options meaningful for numeric scale axes: DEFAULT, SIGNIFICANT, DECIMAL, EXPONENTIAL, ENGINEERING, COMPACT. Text-only formats (STRING, HEX, BINARY, SEXAGESIMAL) are excluded from the dropdown. Move format and precision properties from TankWidget into ScaledPVWidget so they are reusable by Thermometer, ProgressBar, etc. RTTank.setLabelFormat() now takes ScaleFormat instead of FormatOption. Files changed: - NEW core/ui/.../ScaleFormat.java - MOD app/rtplot/.../RTTank.java - MOD app/display/model/.../ScaledPVWidget.java - MOD app/display/model/.../TankWidget.java - MOD app/rtplot/test/.../RTTankTest.java - MOD app/display/model/test/.../TankWidgetUnitTest.java --- .../builder/model/widgets/ScaledPVWidget.java | 32 ++++++++++ .../builder/model/widgets/TankWidget.java | 23 ------- .../model/widgets/TankWidgetUnitTest.java | 8 +-- .../org/csstudio/javafx/rtplot/RTTank.java | 10 ++- .../csstudio/javafx/rtplot/RTTankTest.java | 10 +-- .../org/phoebus/ui/vtype/ScaleFormat.java | 61 +++++++++++++++++++ 6 files changed, 106 insertions(+), 38 deletions(-) create mode 100644 core/ui/src/main/java/org/phoebus/ui/vtype/ScaleFormat.java diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java index 99e43c950a..33fa94049d 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java @@ -10,6 +10,7 @@ import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newBooleanPropertyDescriptor; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newColorPropertyDescriptor; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newDoublePropertyDescriptor; +import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.newIntegerPropertyDescriptor; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propLimitsFromPV; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propMaximum; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propMinimum; @@ -17,12 +18,15 @@ import java.util.List; import org.csstudio.display.builder.model.Messages; +import org.csstudio.display.builder.model.Widget; import org.csstudio.display.builder.model.WidgetProperty; import org.csstudio.display.builder.model.WidgetPropertyCategory; import org.csstudio.display.builder.model.WidgetPropertyDescriptor; import org.csstudio.display.builder.model.persist.NamedWidgetColors; import org.csstudio.display.builder.model.persist.WidgetColorService; +import org.csstudio.display.builder.model.properties.EnumWidgetProperty; import org.csstudio.display.builder.model.properties.WidgetColor; +import org.phoebus.ui.vtype.ScaleFormat; /** Base class for PV widgets that display a numeric value on a scale * (Tank, Thermometer, ProgressBar and similar). @@ -63,6 +67,24 @@ public abstract class ScaledPVWidget extends PVWidget { // ---- Property descriptors ------------------------------------------- + /** 'format' — scale label number format (ScaleFormat subset) */ + public static final WidgetPropertyDescriptor propScaleFormat = + new WidgetPropertyDescriptor<>( + WidgetPropertyCategory.DISPLAY, "format", Messages.WidgetProperties_Format) + { + @Override + public EnumWidgetProperty createProperty(final Widget widget, + final ScaleFormat default_value) + { + return new EnumWidgetProperty<>(this, widget, default_value); + } + }; + + /** 'precision' — number of decimal places (0..15) */ + public static final WidgetPropertyDescriptor propScalePrecision = + newIntegerPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "precision", + Messages.WidgetProperties_Precision, 0, 15); + /** 'alarm_limits_from_pv' — use PV alarm metadata for LOLO/LO/HI/HIHI? */ public static final WidgetPropertyDescriptor propAlarmLimitsFromPV = newBooleanPropertyDescriptor(WidgetPropertyCategory.BEHAVIOR, "alarm_limits_from_pv", @@ -105,6 +127,8 @@ public abstract class ScaledPVWidget extends PVWidget // ---- Instance fields ------------------------------------------------ + private volatile WidgetProperty format; + private volatile WidgetProperty precision; private volatile WidgetProperty limits_from_pv; private volatile WidgetProperty alarm_limits_from_pv; private volatile WidgetProperty minimum; @@ -136,6 +160,8 @@ protected void defineProperties(final List> properties) final WidgetProperty alarm_border_prop = propBorderAlarmSensitive(); properties.remove(alarm_border_prop); + properties.add(format = propScaleFormat.createProperty(this, ScaleFormat.DEFAULT)); + properties.add(precision = propScalePrecision.createProperty(this, 2)); properties.add(limits_from_pv = propLimitsFromPV.createProperty(this, true)); properties.add(minimum = propMinimum.createProperty(this, 0.0)); properties.add(maximum = propMaximum.createProperty(this, 100.0)); @@ -152,6 +178,12 @@ protected void defineProperties(final List> properties) WidgetColorService.getColor(NamedWidgetColors.ALARM_MAJOR))); } + /** @return 'format' property (scale label format) */ + public WidgetProperty propFormat() { return format; } + + /** @return 'precision' property */ + public WidgetProperty propPrecision() { return precision; } + /** @return 'limits_from_pv' property (min/max display range) */ public WidgetProperty propLimitsFromPV() { return limits_from_pv; } diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java index 5d2fa23abe..d4e1368e26 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java @@ -14,12 +14,9 @@ import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propFillColor; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propFont; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propForegroundColor; -import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propFormat; import static org.csstudio.display.builder.model.properties.CommonWidgetProperties.propHorizontal; import static org.csstudio.display.builder.model.widgets.plots.PlotWidgetProperties.propLogscale; -import org.phoebus.ui.vtype.FormatOption; - import java.util.Arrays; import java.util.List; @@ -108,10 +105,6 @@ public Widget createWidget() public static final WidgetPropertyDescriptor propOppositeScaleVisible = newBooleanPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "opposite_scale_visible", Messages.WidgetProperties_OppositeScaleVisible); - /** 'precision' — number of decimal places (0..15) */ - public static final WidgetPropertyDescriptor propScalePrecision = - newIntegerPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "precision", Messages.WidgetProperties_Precision, 0, 15); - /** Widget configurator to read legacy *.opi files*/ private static class CustomConfigurator extends WidgetConfigurator { @@ -177,8 +170,6 @@ public WidgetConfigurator getConfigurator(final Version persisted_version) private volatile WidgetProperty show_minor_ticks; private volatile WidgetProperty perpendicular_tick_labels; private volatile WidgetProperty opposite_scale_visible; - private volatile WidgetProperty format; - private volatile WidgetProperty precision; private volatile WidgetProperty log_scale; private volatile WidgetProperty horizontal; private volatile WidgetProperty border_width_prop; @@ -203,8 +194,6 @@ protected void defineProperties(final List> properties) properties.add(opposite_scale_visible = propOppositeScaleVisible.createProperty(this, false)); properties.add(show_minor_ticks = propShowMinorTicks.createProperty(this, true)); properties.add(perpendicular_tick_labels = propPerpendicularTickLabels.createProperty(this, false)); - properties.add(format = propFormat.createProperty(this, FormatOption.DEFAULT)); - properties.add(precision = propScalePrecision.createProperty(this, 2)); properties.add(log_scale = propLogscale.createProperty(this, false)); properties.add(horizontal = propHorizontal.createProperty(this, false)); properties.add(border_width_prop = propTankBorderWidth.createProperty(this, 0)); @@ -273,18 +262,6 @@ public WidgetProperty propOppositeScaleVisible() return opposite_scale_visible; } - /** @return 'format' property */ - public WidgetProperty propFormat() - { - return format; - } - - /** @return 'precision' property */ - public WidgetProperty propPrecision() - { - return precision; - } - /** @return 'log_scale' property */ public WidgetProperty propLogScale() { diff --git a/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java b/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java index e9abf39cd1..cc544614eb 100644 --- a/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java +++ b/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java @@ -14,7 +14,7 @@ import org.csstudio.display.builder.model.persist.ModelWriter; import org.csstudio.display.builder.model.properties.WidgetColor; import org.junit.jupiter.api.Test; -import org.phoebus.ui.vtype.FormatOption; +import org.phoebus.ui.vtype.ScaleFormat; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -78,7 +78,7 @@ public void testTankWidgetDefaults() assertThat(tank.propOppositeScaleVisible().getValue(), equalTo(false)); assertThat(tank.propShowMinorTicks().getValue(), equalTo(true)); assertThat(tank.propPerpendicularTickLabels().getValue(), equalTo(false)); - assertThat(tank.propFormat().getValue(), equalTo(FormatOption.DEFAULT)); + assertThat(tank.propFormat().getValue(), equalTo(ScaleFormat.DEFAULT)); assertThat(tank.propPrecision().getValue(), equalTo(2)); assertThat(tank.propLogScale().getValue(), equalTo(false)); assertThat(tank.propHorizontal().getValue(), equalTo(false)); @@ -138,7 +138,7 @@ public void testXmlRoundTrip() throws Exception original.propOppositeScaleVisible().setValue(true); original.propBorderWidth().setValue(3); original.propPerpendicularTickLabels().setValue(true); - original.propFormat().setValue(FormatOption.DECIMAL); + original.propFormat().setValue(ScaleFormat.DECIMAL); original.propPrecision().setValue(3); // Serialize — disable skip_defaults so ALL properties appear @@ -184,7 +184,7 @@ public void testXmlRoundTrip() throws Exception assertThat(tank.propOppositeScaleVisible().getValue(), equalTo(true)); assertThat(tank.propBorderWidth().getValue(), equalTo(3)); assertThat(tank.propPerpendicularTickLabels().getValue(), equalTo(true)); - assertThat(tank.propFormat().getValue(), equalTo(FormatOption.DECIMAL)); + assertThat(tank.propFormat().getValue(), equalTo(ScaleFormat.DECIMAL)); assertThat(tank.propPrecision().getValue(), equalTo(3)); } diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java index a2f58df813..4f91f76292 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java @@ -26,7 +26,7 @@ import org.csstudio.javafx.rtplot.internal.util.GraphicsUtils; import org.phoebus.ui.javafx.BufferUtil; import org.phoebus.ui.javafx.UpdateThrottle; -import org.phoebus.ui.vtype.FormatOption; +import org.phoebus.ui.vtype.ScaleFormat; import javafx.application.Platform; import javafx.beans.value.ChangeListener; @@ -280,14 +280,14 @@ public void setShowMinorTicks(final boolean show) } /** Configure the number format used for scale tick labels. - * @param format Display format; {@code null} or {@link FormatOption#DEFAULT} restores automatic formatting. + * @param format Display format; {@code null} or {@link ScaleFormat#DEFAULT} restores automatic formatting. * @param precision Number of decimal places; clamped to [0, 15]. */ - public void setLabelFormat(final FormatOption format, final int precision) + public void setLabelFormat(final ScaleFormat format, final int precision) { final int prec = Math.max(0, Math.min(15, precision)); final NumberFormat fmt; - if (format == null || format == FormatOption.DEFAULT) + if (format == null || format == ScaleFormat.DEFAULT) fmt = null; else switch (format) { @@ -330,8 +330,6 @@ else switch (format) }; break; default: - // HEX, STRING, BINARY, SEXAGESIMAL, etc. are not meaningful for a - // numeric scale axis. Fall back to automatic formatting. fmt = null; break; } diff --git a/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java b/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java index 5d576badd0..c9e5177051 100644 --- a/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java +++ b/app/rtplot/src/test/java/org/csstudio/javafx/rtplot/RTTankTest.java @@ -9,7 +9,7 @@ import org.junit.jupiter.api.Test; -import org.phoebus.ui.vtype.FormatOption; +import org.phoebus.ui.vtype.ScaleFormat; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.not; @@ -96,10 +96,10 @@ public void testSignificantFormat() { final RTTank tank = new RTTank(); // Should accept SIGNIFICANT without error - tank.setLabelFormat(FormatOption.SIGNIFICANT, 3); - tank.setLabelFormat(FormatOption.SIGNIFICANT, 1); + tank.setLabelFormat(ScaleFormat.SIGNIFICANT, 3); + tank.setLabelFormat(ScaleFormat.SIGNIFICANT, 1); // Switching back to other formats should also work - tank.setLabelFormat(FormatOption.DECIMAL, 2); - tank.setLabelFormat(FormatOption.DEFAULT, 0); + tank.setLabelFormat(ScaleFormat.DECIMAL, 2); + tank.setLabelFormat(ScaleFormat.DEFAULT, 0); } } diff --git a/core/ui/src/main/java/org/phoebus/ui/vtype/ScaleFormat.java b/core/ui/src/main/java/org/phoebus/ui/vtype/ScaleFormat.java new file mode 100644 index 0000000000..49fb40b079 --- /dev/null +++ b/core/ui/src/main/java/org/phoebus/ui/vtype/ScaleFormat.java @@ -0,0 +1,61 @@ +/******************************************************************************* + * Copyright (c) 2026 Canadian Light Source Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + *******************************************************************************/ +package org.phoebus.ui.vtype; + +import org.phoebus.ui.Messages; + +/** Formatting options for numeric scale/axis labels. + * + *

A subset of {@link FormatOption} containing only the formats that + * are meaningful on a numeric scale axis (Tank, Thermometer, ProgressBar, + * Meter, etc.). Text-only formats like STRING, HEX, BINARY and + * SEXAGESIMAL are excluded because they have no sensible rendering on + * an axis. + * + *

The ordinal order is independent of {@link FormatOption} and + * places the most useful options first. + * + * @author Heredie Delvalle — CLS + */ +public enum ScaleFormat +{ + /** Automatic formatting chosen by the axis tick algorithm */ + DEFAULT(Messages.Format_Default), + + /** Significant-digits formatting (like C/Java {@code %g}). + * Precision controls total significant digits, not fraction digits. */ + SIGNIFICANT(Messages.Format_Significant), + + /** Fixed decimal places */ + DECIMAL(Messages.Format_Decimal), + + /** Scientific notation (e.g. 1.23E4) */ + EXPONENTIAL(Messages.Format_Exponential), + + /** Engineering notation (exponent is a multiple of 3) */ + ENGINEERING(Messages.Format_Engineering), + + /** Decimal when in range 0.0001..10000, else exponential */ + COMPACT(Messages.Format_Compact); + + // New values must be appended at the end to preserve ordinal + // compatibility with serialized .bob files. + + private final String label; + + ScaleFormat(final String label) + { + this.label = label; + } + + @Override + public String toString() + { + return label; + } +} From 29f7ade0b32d8f38b8ee5ee9bf416ea8fbea7c59 Mon Sep 17 00:00:00 2001 From: Emilio Heredia Date: Tue, 31 Mar 2026 13:49:42 -0600 Subject: [PATCH 7/7] fix(rtplot): align tank ticks with inner border edge When border_width > 0 the stroke is centred on the plot_bounds path, so half the stroke (half_bw = border_width/2) bleeds inward and the top/bottom ticks appeared displaced from the visible tank edge. Fix: expand the layout insets by half_bw on all sides in computeLayout() so the scale's first/last ticks land on the inner edge of the border. The border rect is then shifted outward by half_bw so its inner edge coincides with plot_bounds, keeping ticks, fill, and border visually flush. When border_width = 0 (the default), half_bw = 0 and behaviour is identical to before. fix(rtplot): align tank ticks with inner border edge When border_width > 0 the stroke is centred on the plot_bounds path, so half the stroke (half_bw = border_width/2) bleeds inward and the top/bottom ticks appeared displaced from the visible tank edge. Fix: expand the layout insets by half_bw on all sides in computeLayout() so the scale's first/last ticks land on the inner edge of the border. The border rect is then shifted outward by half_bw so its inner edge coincides with plot_bounds, keeping ticks, fill, and border visually flush. When border_width = 0 (the default), half_bw = 0 and behaviour is identical to before. --- .../builder/model/widgets/ScaledPVWidget.java | 6 +- .../builder/model/widgets/TankWidget.java | 2 +- .../model/widgets/TankWidgetUnitTest.java | 2 +- .../javafx/widgets/TankRepresentation.java | 125 +++++++++++------- .../org/csstudio/javafx/rtplot/RTTank.java | 118 +++++++++++------ 5 files changed, 157 insertions(+), 96 deletions(-) diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java index 33fa94049d..6f8e5ecd11 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/ScaledPVWidget.java @@ -22,10 +22,10 @@ import org.csstudio.display.builder.model.WidgetProperty; import org.csstudio.display.builder.model.WidgetPropertyCategory; import org.csstudio.display.builder.model.WidgetPropertyDescriptor; -import org.csstudio.display.builder.model.persist.NamedWidgetColors; -import org.csstudio.display.builder.model.persist.WidgetColorService; +import org.phoebus.ui.color.NamedWidgetColors; +import org.phoebus.ui.color.WidgetColorService; import org.csstudio.display.builder.model.properties.EnumWidgetProperty; -import org.csstudio.display.builder.model.properties.WidgetColor; +import org.phoebus.ui.color.WidgetColor; import org.phoebus.ui.vtype.ScaleFormat; /** Base class for PV widgets that display a numeric value on a scale diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java index d4e1368e26..48666c55da 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/TankWidget.java @@ -79,7 +79,7 @@ public Widget createWidget() */ public static final WidgetPropertyDescriptor propTankBorderWidth = newIntegerPropertyDescriptor(WidgetPropertyCategory.DISPLAY, "tank_border_width", - Messages.WidgetProperties_BorderWidth, 0, 100); + Messages.WidgetProperties_BorderWidth, 0, 5); /** 'empty_color' */ public static final WidgetPropertyDescriptor propEmptyColor = diff --git a/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java b/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java index cc544614eb..f544fa699e 100644 --- a/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java +++ b/app/display/model/src/test/java/org/csstudio/display/builder/model/widgets/TankWidgetUnitTest.java @@ -12,7 +12,7 @@ import org.csstudio.display.builder.model.WidgetProperty; import org.csstudio.display.builder.model.persist.ModelReader; import org.csstudio.display.builder.model.persist.ModelWriter; -import org.csstudio.display.builder.model.properties.WidgetColor; +import org.phoebus.ui.color.WidgetColor; import org.junit.jupiter.api.Test; import org.phoebus.ui.vtype.ScaleFormat; diff --git a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java index f5d381a8d1..8e3d373914 100644 --- a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java +++ b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/TankRepresentation.java @@ -34,8 +34,9 @@ public class TankRepresentation extends RegionBaseRepresentation { private final DirtyFlag dirty_look = new DirtyFlag(); - private final UntypedWidgetPropertyListener lookListener = this::lookChanged; - private final UntypedWidgetPropertyListener valueListener = this::valueChanged; + private final UntypedWidgetPropertyListener lookListener = this::lookChanged; + private final UntypedWidgetPropertyListener valueListener = this::valueChanged; + private final UntypedWidgetPropertyListener limitsListener = this::limitsChanged; private final WidgetPropertyListener orientationChangedListener = this::orientationChanged; private volatile RTTank tank; @@ -70,18 +71,23 @@ protected void registerListeners() model_widget.propBorderWidth().addUntypedPropertyListener(lookListener); model_widget.propLogScale().addUntypedPropertyListener(lookListener); - model_widget.propShowAlarmLimits().addUntypedPropertyListener(valueListener); - model_widget.propLevelLoLo().addUntypedPropertyListener(valueListener); - model_widget.propLevelLow().addUntypedPropertyListener(valueListener); - model_widget.propLevelHigh().addUntypedPropertyListener(valueListener); - model_widget.propLevelHiHi().addUntypedPropertyListener(valueListener); - model_widget.propAlarmLimitsFromPV().addUntypedPropertyListener(valueListener); + // Range and fill-level; need re-evaluation on every PV sample model_widget.propLimitsFromPV().addUntypedPropertyListener(valueListener); model_widget.propMinimum().addUntypedPropertyListener(valueListener); model_widget.propMaximum().addUntypedPropertyListener(valueListener); model_widget.runtimePropValue().addUntypedPropertyListener(valueListener); + // Alarm limits; only need re-evaluation when limit properties change. + // When alarm_limits_from_pv=true, valueChanged() calls applyAlarmLimits() too. + model_widget.propShowAlarmLimits().addUntypedPropertyListener(limitsListener); + model_widget.propAlarmLimitsFromPV().addUntypedPropertyListener(limitsListener); + model_widget.propLevelLoLo().addUntypedPropertyListener(limitsListener); + model_widget.propLevelLow().addUntypedPropertyListener(limitsListener); + model_widget.propLevelHigh().addUntypedPropertyListener(limitsListener); + model_widget.propLevelHiHi().addUntypedPropertyListener(limitsListener); model_widget.propHorizontal().addPropertyListener(orientationChangedListener); + // Initial apply — order matters: range first, then limits, then value valueChanged(null, null, null); + limitsChanged(null, null, null); } @Override @@ -105,16 +111,16 @@ protected void unregisterListeners() model_widget.propBorderWidth().removePropertyListener(lookListener); model_widget.propLogScale().removePropertyListener(lookListener); - model_widget.propShowAlarmLimits().removePropertyListener(valueListener); - model_widget.propLevelLoLo().removePropertyListener(valueListener); - model_widget.propLevelLow().removePropertyListener(valueListener); - model_widget.propLevelHigh().removePropertyListener(valueListener); - model_widget.propLevelHiHi().removePropertyListener(valueListener); - model_widget.propAlarmLimitsFromPV().removePropertyListener(valueListener); model_widget.propLimitsFromPV().removePropertyListener(valueListener); model_widget.propMinimum().removePropertyListener(valueListener); model_widget.propMaximum().removePropertyListener(valueListener); model_widget.runtimePropValue().removePropertyListener(valueListener); + model_widget.propShowAlarmLimits().removePropertyListener(limitsListener); + model_widget.propAlarmLimitsFromPV().removePropertyListener(limitsListener); + model_widget.propLevelLoLo().removePropertyListener(limitsListener); + model_widget.propLevelLow().removePropertyListener(limitsListener); + model_widget.propLevelHigh().removePropertyListener(limitsListener); + model_widget.propLevelHiHi().removePropertyListener(limitsListener); model_widget.propHorizontal().removePropertyListener(orientationChangedListener); super.unregisterListeners(); } @@ -125,11 +131,15 @@ private void lookChanged(final WidgetProperty property, final Object old_valu toolkit.scheduleUpdate(this); } + /** Update the display range and fill level. Called on every PV value change. + * Alarm limits from PV metadata are also refreshed here (the metadata is + * carried inside the VType on every update). Manually-configured limits + * are managed exclusively by {@link #limitsChanged}. + */ private void valueChanged(final WidgetProperty property, final Object old_value, final Object new_value) { final VType vtype = model_widget.runtimePropValue().getValue(); - // --- Min / Max --- double min_val = model_widget.propMinimum().getValue(); double max_val = model_widget.propMaximum().getValue(); if (model_widget.propLimitsFromPV().getValue()) @@ -143,46 +153,63 @@ private void valueChanged(final WidgetProperty property, final Object old_val } tank.setRange(min_val, max_val); - // --- Alarm limit lines --- - if (model_widget.propShowAlarmLimits().getValue()) + // Alarm metadata is embedded in the VType, so re-check it on every update. + // When using widget-configured limits, limitsChanged() handles updates instead. + if (model_widget.propAlarmLimitsFromPV().getValue()) + applyAlarmLimits(vtype); + + final double value = toolkit.isEditMode() + ? (min_val + max_val) / 2 + : VTypeUtil.getValueNumber(vtype).doubleValue(); + tank.setValue(value); + } + + /** Re-apply alarm limit lines. Called when any limit property changes. + * Also invoked from {@link #valueChanged} when limits come from the PV. + */ + private void limitsChanged(final WidgetProperty property, final Object old_value, final Object new_value) + { + applyAlarmLimits(model_widget.runtimePropValue().getValue()); + } + + /** Push the current alarm limits to the tank, reading from PV metadata or + * widget properties depending on {@code alarm_limits_from_pv}. + * Clears all limit lines when {@code show_alarm_limits} is {@code false}. + */ + private void applyAlarmLimits(final VType vtype) + { + if (!model_widget.propShowAlarmLimits().getValue()) + { + tank.setLimits(Double.NaN, Double.NaN, Double.NaN, Double.NaN); + return; + } + final double lolo, lo, hi, hihi; + if (model_widget.propAlarmLimitsFromPV().getValue()) { - double lolo, lo, hi, hihi; - if (model_widget.propAlarmLimitsFromPV().getValue()) - { // Read from PV alarm metadata - final Display display_info = Display.displayOf(vtype); - if (display_info != null) - { - final Range minor = display_info.getWarningRange(); - final Range major = display_info.getAlarmRange(); - lo = minor.getMinimum(); - hi = minor.getMaximum(); - lolo = major.getMinimum(); - hihi = major.getMaximum(); - } - else - { // PV connected but no metadata yet — show nothing - lolo = lo = hi = hihi = Double.NaN; - } + final Display display_info = Display.displayOf(vtype); + if (display_info != null) + { + final Range minor = display_info.getWarningRange(); + final Range major = display_info.getAlarmRange(); + lo = minor.getMinimum(); + hi = minor.getMaximum(); + lolo = major.getMinimum(); + hihi = major.getMaximum(); } else - { // Read from widget properties - lolo = model_widget.propLevelLoLo().getValue(); - lo = model_widget.propLevelLow().getValue(); - hi = model_widget.propLevelHigh().getValue(); - hihi = model_widget.propLevelHiHi().getValue(); + { // PV connected but no metadata yet — show nothing + lolo = lo = hi = hihi = Double.NaN; } - tank.setLimits(lolo, lo, hi, hihi); - tank.setLimitsFromPV(model_widget.propAlarmLimitsFromPV().getValue()); } else - tank.setLimits(Double.NaN, Double.NaN, Double.NaN, Double.NaN); - - double value; - if (toolkit.isEditMode()) - value = (min_val + max_val) / 2; - else - value = VTypeUtil.getValueNumber(vtype).doubleValue(); - tank.setValue(value); + { + lolo = model_widget.propLevelLoLo().getValue(); + lo = model_widget.propLevelLow().getValue(); + hi = model_widget.propLevelHigh().getValue(); + hihi = model_widget.propLevelHiHi().getValue(); + } + tank.setLimits(lolo, lo, hi, hihi); + tank.setLimitsFromPV(model_widget.propAlarmLimitsFromPV().getValue()); } private void orientationChanged(final WidgetProperty prop, final Boolean old, final Boolean horizontal) diff --git a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java index 4f91f76292..670417280f 100644 --- a/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java +++ b/app/rtplot/src/main/java/org/csstudio/javafx/rtplot/RTTank.java @@ -207,10 +207,10 @@ public void setFont(final Font font) right_scale.setScaleFont(font); } - /** @param width Border width in pixels around the tank body (0 = no border) */ + /** @param width Border width in pixels around the tank body (0 = no border, max = 5) */ public void setBorderWidth(final int width) { - border_width = Math.max(0, width); + border_width = Math.max(0, Math.min(5, width)); requestUpdate(); } @@ -313,21 +313,7 @@ else switch (format) fmt = null; break; case SIGNIFICANT: - // Significant-digits formatting (like C/Java %g). Each tick value - // is individually formatted with the requested number of significant - // digits, choosing decimal or exponential notation per value. - final String pattern = "%." + prec + "g"; - fmt = new NumberFormat() { - @Override public StringBuffer format(double v, StringBuffer buf, java.text.FieldPosition pos) { - return buf.append(String.format(java.util.Locale.ROOT, pattern, v)); - } - @Override public StringBuffer format(long v, StringBuffer buf, java.text.FieldPosition pos) { - return buf.append(String.format(java.util.Locale.ROOT, pattern, (double) v)); - } - @Override public Number parse(String s, java.text.ParsePosition pos) { - throw new UnsupportedOperationException(); - } - }; + fmt = significantDigitsFormat(prec); break; default: fmt = null; @@ -338,6 +324,32 @@ else switch (format) requestUpdate(); } + /** Build a {@link NumberFormat} that formats each value to {@code prec} significant + * figures using {@code %g}-style notation (decimal or scientific per value magnitude). + */ + private static NumberFormat significantDigitsFormat(final int prec) + { + final String pattern = "%." + prec + "g"; + return new NumberFormat() + { + @Override + public StringBuffer format(final double v, final StringBuffer buf, final java.text.FieldPosition pos) + { + return buf.append(String.format(java.util.Locale.ROOT, pattern, v)); + } + @Override + public StringBuffer format(final long v, final StringBuffer buf, final java.text.FieldPosition pos) + { + return buf.append(String.format(java.util.Locale.ROOT, pattern, (double) v)); + } + @Override + public Number parse(final String s, final java.text.ParsePosition pos) + { + throw new UnsupportedOperationException(); + } + }; + } + /** Set alarm and warning limit values to display as horizontal lines on the tank. * Pass {@link Double#NaN} for any limit that should not be shown. * @param lolo LOLO (major alarm) lower limit @@ -398,6 +410,7 @@ public void setPerpendicularTickLabels(final boolean perpendicular) { scale.setPerpendicularTickLabels(perpendicular); right_scale.setPerpendicularTickLabels(perpendicular); + need_layout.set(true); // scale width changes between rotated and perpendicular modes requestUpdate(); } @@ -456,6 +469,32 @@ private void drawLimitLineAt(final Graphics2D gc, final Rectangle pb, gc.drawLine(pb.x, y, pb.x + pb.width, y); } + /** Compute the fill height in pixels for the current value. + * Handles both linear and logarithmic scales. + * + * @param plotHeight Pixel height of the plot area + * @param min Scale minimum (< max) + * @param max Scale maximum + * @param current Current PV value + * @param logscale Whether the scale uses log spacing + * @return Fill level in pixels: 0 = empty, plotHeight = full + */ + private static int computeFillLevel(final int plotHeight, final double min, final double max, + final double current, final boolean logscale) + { + if (current <= min) + return 0; + if (current >= max) + return plotHeight; + if (logscale) // by mellguth2, https://github.com/ControlSystemStudio/phoebus/issues/2726 + { // Refuse to map if any input is non-positive (log undefined) + if (min <= 0 || max <= 0 || current <= 0) + return 0; + return (int) (plotHeight * Math.log(current / min) / Math.log(max / min)); + } + return (int) (plotHeight * (current - min) / (max - min) + 0.5); + } + /** Compute layout of plot components. * Supports independent left and right scales; the plot area sits * between them. A 1-pixel inset is added on any edge that has no @@ -480,11 +519,14 @@ private void computeLayout(final Graphics2D gc, final Rectangle bounds) ends[1] = Math.max(ends[1], r_ends[1]); } - // Small inset so the tank outline is not clipped on edges without a scale - final int inset_left = (left_width == 0) ? 1 : 0; - final int inset_right = (right_width == 0) ? 1 : 0; - final int inset_top = (ends[1] == 0) ? 1 : 0; - final int inset_bottom = (ends[0] == 0) ? 1 : 0; + // Inset = ceil(border_width/2) keeps the outer stroke edge inside the canvas. + // On sides with a scale the label area provides ample margin so inset=0. + // When there is no border, inset=1 is the original clip guard. + final int half_bw_ceil = (border_width + 1) / 2; + final int inset_left = (left_width == 0) ? Math.max(1, half_bw_ceil) : 0; + final int inset_right = (right_width == 0) ? Math.max(1, half_bw_ceil) : 0; + final int inset_top = (ends[1] == 0) ? Math.max(1, half_bw_ceil) : 0; + final int inset_bottom = (ends[0] == 0) ? Math.max(1, half_bw_ceil) : 0; final int top = bounds.y + ends[1] + inset_top; final int height = bounds.height - ends[0] - ends[1] - inset_top - inset_bottom; @@ -537,22 +579,7 @@ protected Image updateImageBuffer() final double min = Math.min(range.getLow(), range.getHigh()); final double max = Math.max(range.getLow(), range.getHigh()); final double current = value; - final int level; - if (current <= min) - level = 0; - else if (current >= max) - level = plot_bounds.height; - else if (max == min) - level = 0; - else if (scale.isLogarithmic()) // by mellguth2, https://github.com/ControlSystemStudio/phoebus/issues/2726 - { // refuse to try any mapping if negatives or zero are involved - if (min <= 0 || max <= 0.0 || current <= 0.0) - level = 0; - else - level = (int) (plot_bounds.height * Math.log(current/min) / Math.log(max/min)); - } - else // linear scale - level = (int) (plot_bounds.height * (current - min) / (max - min) + 0.5); + final int level = computeFillLevel(plot_bounds.height, min, max, current, scale.isLogarithmic()); final int arc = Math.min(plot_bounds.width, plot_bounds.height) / 10; gc.setPaint(new GradientPaint(plot_bounds.x, 0, empty, plot_bounds.x+plot_bounds.width/2, 0, empty_shadow, true)); @@ -565,12 +592,21 @@ else if (scale.isLogarithmic()) // by mellguth2, https://github.com/ControlSyste else gc.fillRoundRect(plot_bounds.x, plot_bounds.y, plot_bounds.width, level, arc, arc); - // Optional border around the tank body + // Optional border: stroked CENTRED on plot_bounds — no integer half-pixel + // shifting. The inner half of the stroke covers the fill edge (no gap); + // the outer half extends beyond plot_bounds into the inset margin. + // Ticks land at plot_bounds edges = centre of the border stroke, matching + // the CS-Studio BOY convention. if (border_width > 0) { + // Java2D: fillRoundRect covers x..x+w-1, drawRoundRect strokes x..x+w. + // Using w-1, h-1 aligns the stroke centre with the fill boundary, + // making all four edges symmetric. gc.setColor(foreground); gc.setStroke(new BasicStroke(border_width)); - gc.drawRoundRect(plot_bounds.x, plot_bounds.y, plot_bounds.width, plot_bounds.height, arc, arc); + gc.drawRoundRect(plot_bounds.x, plot_bounds.y, + plot_bounds.width - 1, plot_bounds.height - 1, + arc, arc); gc.setStroke(new BasicStroke(1f)); } @@ -594,8 +630,6 @@ else if (scale.isLogarithmic()) // by mellguth2, https://github.com/ControlSyste gc.setStroke(new BasicStroke(1f)); } - gc.setColor(background); - gc.dispose(); // Convert to JFX