feat (display): RTTank-based Progress Bar backend — scale, alarm limits, and stacking support (opt-in via preference)#3768
Open
emilioheredia-source wants to merge 11 commits intoControlSystemStudio:masterfrom
Conversation
Switch ProgressBarWidget from PVWidget to ScaledPVWidget and replace the JFX ProgressBar control with RTTank (border width hardcoded to 0). ProgressBarWidget inherits format, precision, alarm limit lines and alarm colours from ScaledPVWidget. New properties scale_visible and show_minor_ticks (both off/on by default, matching Tank) are added. All existing .bob XML property names are preserved (limits_from_pv, minimum, maximum, fill_color, background_color, horizontal, log_scale) so existing files load without migration. ProgressBarRepresentation wires the same lookChanged / valueChanged / limitsChanged / orientationChanged pattern as TankRepresentation and maps background_color to both setBackground() and setEmptyColor() so the unfilled area matches the declared background.
…and perpendicular labels to ProgressBar Move propScaleVisible, propShowMinorTicks, propOppositeScaleVisible, and propPerpendicularTickLabels from TankWidget to their proper home in ScaledPVWidget. This removes the TankWidget import smell in ProgressBarWidget and makes all four properties available to any ScaledPVWidget subclass. ProgressBarWidget gains opposite_scale_visible and perpendicular_tick_labels, wired through ProgressBarRepresentation to RTTank.setRightScaleVisible() and RTTank.setPerpendicularTickLabels() respectively.
… ProgressBar propTankBorderWidth (XML: 'tank_border_width') removed from TankWidget. Replaced by propBorderWidth (XML: 'border_width') in ScaledPVWidget so all scaled widgets share the same descriptor. TankWidget.CustomConfigurator migrates old 'tank_border_width' XML elements to 'border_width' transparently, so existing .bob files still load. ProgressBarWidget gains border_width_prop (default 0), wired to RTTank.setBorderWidth() in ProgressBarRepresentation.
… format()
In compute(), relabelTicks() is now called after the major tick list is
built so that the user-specified NumberFormat override (set via
setLabelFormat()) takes effect for all ticks, including the forced
boundary ticks added by the guard at the end.
format() now checks getLabelFormatOverride() directly so that one-off
calls (e.g. from axis tooltip code) also respect the override, rather
than always falling back to num_fmt.
RTTank.significantDigitsFormat() now post-processes String.format('%g')
output via normaliseExponent(): uppercase E, no leading zeros on the
exponent, no '+' sign — matching the style produced by
LinearTicks.createExponentialFormat() and standard scientific notation
(MATLAB, NumPy, LabVIEW).
…M/BOY converters
ScaledPVWidget: fix javadoc ('show_limits' -> 'show_alarm_limits');
reorder imports to standard Java convention (static, java.*, org.*).
TankWidget: remove the tank_border_width XML migration block that was
added prematurely before any .bob files using that key existed.
ProgressBarWidget: drop three dead imports left over after property
descriptors moved to ScaledPVWidget. Add BOY OPI migration for
<show_scale> -> scale_visible and <scale_font> -> font, which do not
auto-map because their XML element names differ.
ProgressBarRepresentation: remove stale setBorderWidth(0) call and its
misleading 'always hidden' comment from createJFXNode(); the property
is now user-configurable. Update class javadoc to match.
Convert_activeBarClass (EDM -> Phoebus): map five previously ignored
EDM properties: showScale -> scale_visible; precision (when > 0);
min/max when limitsFromDb is false and the range is valid; border
boolean -> 1 px border_width.
String.replaceFirst() recompiles the Pattern on every call. Move the pattern to a static final EXP_LEADING_ZEROS constant so it is compiled once at class-load time.
Three related improvements to widget rendering performance and architecture: ## 1. RTTank parallel rendering (rtplot) Root cause: UpdateThrottle.TIMER is a single-thread shared executor. On displays with many Tank/ProgressBar widgets all renders serialise on one thread, capping throughput at ~5 s/sweep for 200 widgets. Fix: RTTank constructor now passes Activator.thread_pool (N-core pool) when parallel_rendering=true, TIMER otherwise. Default is false — no behaviour change for existing installs. New preference: org.csstudio.javafx.rtplot/parallel_rendering (default false) ## 2. ProgressBar scale-mode rendering (display) New preference org.csstudio.display.builder.representation/progressbar_scale_mode: - false (default): stock JFX ProgressBar — original behaviour, no change - true: RTProgressBarRepresentation backed by RTTank, adding a numeric scale, tick format/precision, dual scale, alarm-limit lines, and parallel rendering Architecture: - RTScaledWidgetRepresentation<W extends ScaledPVWidget>: new abstract base class that eliminates ~90 % duplication between TankRepresentation and RTProgressBarRepresentation. Handles value/range updates, alarm limits, orientation transforms, and throttle wiring in one place. - TankRepresentation: refactored to a thin subclass (~30 lines). - RTProgressBarRepresentation: new thin subclass for the scale-mode bar. - ProgressBarRepresentation: restored to the unmodified upstream JFX bar (closed for modification). - BaseWidgetRepresentations: one-line conditional dispatch on preference. Performance fix in RTScaledWidgetRepresentation.valueChanged(): scale.setValueRange() (tick layout recomputation) is now skipped when a pure PV value arrives and limits_from_pv=false, saving needless work at up to 20 Hz per widget. ## 3. Property panel filtering (editor) When progressbar_scale_mode=false, scale-only properties (scale_visible, format, precision, alarm limits, etc.) are hidden in the property editor via a set-membership check in PropertyPanelSection.fill(). Uses ProgressBarWidget.SCALE_MODE_PROPS — a static constant in the model class — so the editor (which already depends on representation) can read the preference and apply the filter with no new module dependencies. Tested on CLS OPI workstation with 200 RTTank widgets (Tank + ProgressBar mix). With parallel_rendering=true the visible refresh lag drops from ~5 s to <200 ms.
border_alarm_sensitive and border_width are handled by RegionBaseRepresentation for all RegionBaseRepresentation subclasses, regardless of rendering mode. They were incorrectly included in SCALE_MODE_PROPS, which caused the property editor to hide them when progressbar_scale_mode=false — preventing users from setting alarm-sensitive borders or custom widget borders on the classic JFX progress bar. Also add 'requires restart' note to parallel_rendering preference comment for consistency with progressbar_scale_mode documentation.
Add two RTTank rendering modes used only by RTProgressBarRepresentation: flat_track: paints the unfilled track with a solid colour instead of the default left→center gradient. The stock JFX ProgressBar has a flat background; the gradient was appearing as a dark band in the middle of the track background, opposite to the intended visual. inner_padding: extra inset from all four canvas edges to the plot body. When scale_visible=false the canvas fills edge-to-edge; 3 px of padding recreates the inset margin of the default JFX ProgressBar CSS (~7 px total, ~3 px per side visible), so .bob files from older Phoebus versions render consistently after enabling progressbar_scale_mode. Set to 0 automatically when scale_visible=true (the scale label area provides the visual framing). Both features default to off so the Tank widget is completely unaffected. RTProgressBarRepresentation activates them in configureTank() / applyLookToTank().
…ressBar
Two new widget properties extending the RTTank rendering path:
## show_scale_labels (Tank + ProgressBar, default: true)
When false, the axis scale draws tick marks but suppresses all label
text. This enables tight stacked layouts where one labelled widget
leads and the rest show aligned tick marks only, saving horizontal (or
vertical) space without losing the alignment grid.
Changes:
YAxisImpl - volatile boolean show_labels (render thread / JFX thread)
- getDesiredPixelSize: short-circuits to TICK_LENGTH when false
- getPixelGaps: returns {0,0} — no label overhang at endpoints
- paint: skips drawTickLabel + paintLabels when false
RTTank - setScaleLabelsVisible() delegates to both YAxisImpl axes;
axes fire requestLayout/requestRefresh via plot_part_listener,
so RTTank needs no extra need_layout or requestUpdate calls
ScaledPVWidget - propShowScaleLabels descriptor (shared base)
TankWidget - show_scale_labels property (default true)
TankRepresentation - listener + applyLookToTank call
## inner_padding (ProgressBar only, default: 3, range: 0..20)
Configurable inset between the widget edge and the fill bar. Replaces
the previous hardcoded 'scale_visible ? 0 : 3' logic. Set to 0 for an
edge-to-edge bar; keep at 3 to match the stock JFX ProgressBar CSS margin.
Changes:
ProgressBarWidget - propInnerPadding descriptor + property + accessor
- inner_padding added to SCALE_MODE_PROPS filter
- show_scale_labels added to SCALE_MODE_PROPS filter
RTProgressBarRepresentation - listener + applyLookToTank call
## Review fixes (same batch)
- volatile on YAxisImpl.show_labels (thread-safety: render vs JFX thread)
- removed redundant need_layout+requestUpdate in RTTank.setScaleLabelsVisible
- corrected stale configureTank() javadoc in RTProgressBarRepresentation
- updated ProgressBarWidget class javadoc to mention new properties
- fixed double blank line in ProgressBarWidget imports
- Messages.java + messages.properties: ShowScaleLabels, InnerPadding
- docs/source/changelog.rst: ShowScaleLabels + InnerPadding entries added
Author
|
This is likely my last new-feature PR for a while. If this one is accepted, the natural next step would be the Thermometer widget — applying the same opt-in backend pattern to add a scale, configurable label formats, and alarm limits, reusing the RTScaledWidgetRepresentation base class that Tank and this new ProgressBar implementation already share. |
|
Author
SonarCloud findings — triage noteSonarCloud flagged 54 issues on this PR. I reviewed all of them; here is the breakdown. Fixed (2 issues introduced by this PR)
Both fixes are in the latest commit on this branch. Pre-existing / false positives (52 issues not touched)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a completely new rendering backend for the existing
ProgressBarwidget, replacing the default JavaFXProgressBarcontrol with anRTTank-based implementation.The RTTank backend uses a different rendering path than the stock JavaFX control. In practice update responsiveness can be similar, but it comes at the cost of higher CPU usage (the RT rendering loop is always spinning). For a more detailed discussion of rendering performance and CPU trade-offs, see the comments in #3766, which includes side-by-side video comparisons of both backends under different settings. Because the widget also looks slightly different from the original (font, scale markings, fill aesthetics), we chose not to make this the default — doing so would introduce visual breaking changes for every existing
.bobscreen that uses aProgressBar.Instead, the new backend is exposed as a hidden, opt-in preference:
false(default): the originalProgressBarRepresentationis used — no change to existing screens whatsoever.true:RTProgressBarRepresentationis used instead, globally, for all Progress Bar widgets in that Phoebus instance.This approach keeps the change non-breaking and makes the new backend accessible and testable in production-like conditions. If it proves stable and the visual difference is accepted (or resolved upstream), it could be made the default in a future release.
Design rationale and trade-offs
Dispatching two distinct rendering implementations under the same widget type is a known architectural compromise. The cleaner alternatives — introducing a dedicated widget type, or adding a per-widget renderer selector — were considered but ruled out: a new widget type would require migrating all existing
.bobfiles, while a per-widget selector would complicate the model/representation contract without offering a clear path to adoption.The global preference avoids both problems. It has no impact on the widget model, is trivially reversible, and allows the new backend to be evaluated on real operational screens without any file changes. This is treated as an interim approach: if the implementation proves stable and the visual differences are accepted or resolved, the new backend could be promoted to the default in a future release.
New capabilities (available only when
progressbar_scale_mode=true)Scale and axis
YAxisImplalongside the barscale_visible,limits_from_pv,minimum,maximum,major_ticks,minor_ticksshow_minor_ticks,perpendicular_tick_labelsproperties respectedAlarm limits
show_limits=trueLook parity with Tank
foreground_color,background_color,fill_color,empty_colorskin(none / classic / button) fromScaledPVWidgetflat_track— whentrue, the fill level is drawn as a flat horizontal bandNew properties (contributed by PR2 infrastructure)
show_scale_labels(boolean, defaulttrue): whenfalse, tick marks are drawn but all label text is suppressed — useful for stacking multiple bars that share a single labelled reference scale.inner_padding(integer 0–20 px, default3): controls the gap between the fill bar and the widget border; replaces the previously hardcoded 3 px value.Dependencies
tank_scale_refactor(already merged as TankWidget: dual scales, alarm limit lines, ScaleFormat enum, and ScaledPVWidget base class #3760).display/rtscaled-tank-refactor) which contributesRTScaledWidgetRepresentationand theshow_scale_labelsinfrastructure. That PR should be merged first, but the commits here are rebased cleanly on top of master if needed.Files changed
YAxisImpl.javaRTTank.javaScaledPVWidget.java,TankWidget.java,ProgressBarWidget.java,Messages.java,messages.propertiesTankRepresentation.java,RTProgressBarRepresentation.java,RepresentationFactory.java,ProgressBarRepresentation.javarepresentation_preferences.propertieschangelog.rstTesting
Default / backward-compatibility check
Enable the new backend
org.csstudio.display.builder.representation/progressbar_scale_mode=truetophoebus-settings/settings.iniand restart..bobfiles open normally; the widget editor now shows all new properties.Scale and axis
scale_visibleon/off — scale appears and disappears cleanly.minimumandmaximumexplicitly; verify the axis range and tick spacing update.major_ticksandminor_tickscounts; verify tick density changes on the scale.show_minor_ticks— minor tick marks appear/disappear without affecting major ticks.perpendicular_tick_labels— labels rotate between parallel and perpendicular.Number format
formatandprecisionproperties; verify axis labels update accordingly.Alarm limits
limits_from_pvand connect a PV that has LOLO/LO/HI/HIHI alarm limits defined; verify coloured marker lines appear on the scale at the correct positions.show_limitsoff — markers disappear while the scale remains.Look and colours
foreground_color,background_color,fill_color,empty_color— verify each affects the correct part of the widget.skinvalues (none / classic / button) and verify the fill style changes.flat_track— verify the fill switches between a tapered and a flat horizontal band.New properties
show_scale_labels=falseon both a Tank and a ProgressBar — tick marks must remain visible but all label text must be suppressed.inner_paddingfrom 0 to 20 — verify the gap between the fill bar and the border grows and shrinks live without requiring a restart.No-regression on Tank widget
Tankwidgets (not ProgressBar); confirmshow_scale_labelsandinner_paddingwork identically there, since the properties are shared viaRTTank.