feat (display/rtplot): extract RTScaledWidgetRepresentation + add show_scale_labels to Tank#3767
Open
emilioheredia-source wants to merge 4 commits intoControlSystemStudio:masterfrom
Conversation
…r RTTank widgets TankRepresentation contained all RTTank lifecycle logic inline: PV value and range forwarding, alarm limit evaluation, orientation transforms, and throttle configuration. Any further RTTank-backed widget (e.g. RTProgressBar) would need every line of it duplicated. Fix: extract RTScaledWidgetRepresentation<W extends ScaledPVWidget>, a generic abstract base class that owns all shared behaviour: - RTTank creation and update-throttle configuration - forwarding PV value / display-range changes to the tank - evaluating alarm limit lines from PV metadata or widget properties - 90 deg orientation transform for horizontal layout - scheduling JFX representation updates on property changes Performance improvement in valueChanged(): tank.setRange() calls scale.setValueRange(), which recomputes the full tick layout. This is now skipped when a pure PV value arrives and limits_from_pv is false, avoiding needless tick recomputation at up to 20 Hz per widget. Subclasses implement four small abstract methods: isHorizontal() — read the widget horizontal property registerLookListeners() — add listeners on widget-specific props unregisterLookListeners() — matching removals applyLookToTank(width, height) — push appearance props to the tank TankRepresentation is now a ~30-line thin subclass with no duplicated logic. No behaviour change for existing Tank widget displays.
Allow the numeric scale to render tick marks without label text.
When show_scale_labels=false, tick positions are identical to the
labelled mode, so multiple stacked Tank or ProgressBar widgets can
share a single labelled scale: only the first widget shows text;
the rest show aligned ticks only, saving space without losing the
alignment grid.
## YAxisImpl changes
- volatile boolean show_labels (default true)
Read on the Java2D render thread, written on the JFX application
thread; must be volatile.
- getDesiredPixelSize: returns TICK_LENGTH (10 px) immediately when
show_labels=false, skipping all label metric computation.
- getPixelGaps: returns {0,0} when show_labels=false; rotated labels
no longer overhang past the axis endpoints.
- paint: skips drawTickLabel and paintLabels when show_labels=false;
tick marks and the axis line are still drawn normally.
- setScaleLabelsVisible: guarded early-return prevents spurious
relayout when the value has not changed.
## RTTank changes
- setScaleLabelsVisible(boolean): delegates to both left and right
YAxisImpl axes. Each axis calls requestLayout()/requestRefresh()
internally when its state changes; these propagate to RTTank's
need_layout flag and requestUpdate() via plot_part_listener, so
RTTank requires no additional calls.
## Model / representation changes
ScaledPVWidget - propShowScaleLabels descriptor (shared by Tank + ProgressBar)
TankWidget - show_scale_labels property, default true
TankRepresentation - lookListener + applyLookToTank: tank.setScaleLabelsVisible()
Messages / messages.properties - ShowScaleLabels, InnerPadding display strings
|
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
Refactors the Tank widget's JavaFX wiring to eliminate the duplication that
existed between
TankRepresentationand (the future)RTProgressBarRepresentation,and adds a new
show_scale_labelsproperty toTankWidgetand the underlyingYAxisImplcanvas axis.This PR depends on: tank_scale_refactor (already merged as #3760).
It is recommended to merge this after #3766
(parallel_rendering preference for RTTank), but the two PRs are independent —
each applies cleanly on top of master without the other.
Merging this PR, also sets up the base class and show_scale_labels infrastructure
needed by the RTTank-based Progress Bar refactor, which will be submitted as the next PR.
What Changed
New:
RTScaledWidgetRepresentation<W>abstract base classA generic base for all widget representations whose JFX node is an
RTTank.Handles everything that depends only on the
ScaledPVWidgetcontract:RTTankSubclasses provide four template methods:
isHorizontal()horizontalpropertyregisterLookListeners()unregisterLookListeners()applyLookToTank(w, h)configureTank()is an optional no-op hook called once after tank creation.Result:
TankRepresentationcollapses from ~270 lines to ~90 lines ofwidget-specific code.
RTProgressBarRepresentation(PR3) is ~110 lines.ScaledPVWidget— newpropShowScaleLabelsdescriptorshow_scale_labels(boolean, defaulttrue) is added toScaledPVWidgetso both Tank and ProgressBar inherit it without duplication.
TankWidget— newshow_scale_labelspropertyExposes
show_scale_labelsin the property editor. Defaulttruepreservesexisting behaviour.
RTTank—setScaleLabelsVisible(boolean)Delegates to both
YAxisImplinstances (left and right scale). Each axisfires
requestLayout()/requestRefresh()internally viaplot_part_listenerwhen its state changes, so no extra
need_layoutorrequestUpdate()callsare needed in RTTank.
YAxisImpl— ticks-only rendering mode (show_labels)show_labels = falsegetDesiredPixelSizeTICK_LENGTH(10 px) immediately — no label metrics computedgetPixelGaps{0, 0}— rotated labels no longer overhang endpointspaintdrawTickLabelandpaintLabels; tick marks and axis line draw normallyshow_labelsisvolatilebecause it is read on the Java2D render threadand written on the JFX application thread.
setScaleLabelsVisiblehas an early-return guard to suppress spuriousrelayout when the value has not actually changed.
Use case: stacked widgets sharing a single labelled scale
Backward Compatibility
show_scale_labels=true— existing.bobfiles load and displayidentically.
<show_scale_labels>is silently ignored by older Phoebus.RTScaledWidgetRepresentationis package-private. No public API change.Files Changed
RTScaledWidgetRepresentation.java | 343 +++ (new)
TankRepresentation.java | 241 +/--
RTTank.java | 102 ++-
YAxisImpl.java | 38 ++-
ScaledPVWidget.java | 36 ++-
TankWidget.java | 39 +/-
Messages.java | 2 +
messages.properties | 2 +
8 files changed, ~548 insertions, ~255 deletions
Design Decisions
show_labelsasvolatilesetScaleLabelsVisible) and read on the Java2D render thread (paint,getDesiredPixelSize). Withoutvolatilethe write may not be visible to the render thread.requestUpdateinsetScaleLabelsVisibleYAxisImplaxes callrequestLayout()/requestRefresh()themselves when the value changes; these propagate toRTTank.need_layoutandrequestUpdate()viaplot_part_listener. A second explicit call would cause a redundant double-render.show_scale_labelsinScaledPVWidgetdefinePropertiesentry inScaledPVWidgettrueand a future Meter or Thermometer to choose differently.