Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spinner: repeated value not correctly shown #3183

Open
Synthetica9 opened this issue Nov 6, 2024 · 5 comments
Open

Spinner: repeated value not correctly shown #3183

Synthetica9 opened this issue Nov 6, 2024 · 5 comments

Comments

@Synthetica9
Copy link

Synthetica9 commented Nov 6, 2024

Description

When entering a value in a spinner, the formatter is only applied every other time. This is especially evident for clipped values which show the wrong value when you do this, but same issue applies for non-clipped values.

To reproduce

  1. Add a spinner box to a display
  2. Enter a value, say 50
  3. Hit enter
  4. (observe the value is formatted)
  5. Hit the exact same value again
  6. Value is left exactly as-is, unformatted. This also means it will show the non-clipped value which differs from what's actually in the PV
Recording.2024-11-06.164138.mp4
Recording.2024-11-06.164459.mp4

Root cause

There seems to be some caching in JavaFX internally that rejects the write to the spinner.

Solution

The following... works but there must be some better way I imagine:

diff --git a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/SpinnerRepresentation.java b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/SpinnerRepresentation.java
index 97fe4b832..91d5f4fe0 100644
--- a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/SpinnerRepresentation.java
+++ b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/SpinnerRepresentation.java
@@ -561,6 +561,7 @@ public class SpinnerRepresentation extends RegionBaseRepresentation<Spinner<Stri
         if (dirty_content.checkAndClear())
         {
             ( (TextSpinnerValueFactory)jfx_node.getValueFactory() ).setVTypeValue(value);
+            jfx_node.getValueFactory().setValue("PLACEHOLDER VALUE TO FORCE UPDATE!");
             jfx_node.getValueFactory().setValue(value_text);
         }

Environment

  • Phoebus 4.7.3, also present on master
  • Java version: Oracle Corporation 21.0.1+12-29
  • JavaFX version: 19+11
  • Windows 11
@kasemir
Copy link
Collaborator

kasemir commented Nov 6, 2024

Have you already narrowed it down to us calling setValue(String) but JFX ignores it?
Or could it be a matter of us not calling setValue(String)?

Mind you this could be related to our attempt of handling network delays, changes from somewhere else, or the IOC clamping the value.
Basic idea: Assume the value is 10. If you enter "100", we don't just set the widget to "100". In fact, we should set it back to the previous value, 10, and write the value 100 to the PV. The IOC may then accept that and return a subscription update of 100, or it may clamp the value to say 50 and return a subscription update of 50. The widget then receives that subscription update and formats it. Or we didn't enter nor write anything. Somebody else did, and we get a subscription update because of that. Finally, the IOC may not permit write access, so the value stays at 10, there's no subscription update; good thing we set the widget back to its previous value of 10 because that hasn't really changed unless we get a subscription update.

If the core issue is indeed JFX ignoring our setValue(String), and setting some bogus intermediate value is a practical workaround, then maybe use setValue("--") instead of setValue("PLACEHOLDER VALUE TO FORCE UPDATE!"), but yes, that's what we'll have to do.

@Synthetica9
Copy link
Author

Have you already narrowed it down to us calling setValue(String) but JFX ignores it?

Yes, I set a breakpoint and it gets hit when the update fails, which I think is what you're asking?

I understand the reasoning, I tried it with local PV's which uses some internal logic I think, but I've also had this happen with a "real" (CA) PV over the network

@kasemir
Copy link
Collaborator

kasemir commented Nov 6, 2024

We should never see the intermediate value, but just in case, to avoid flicker, might this work?

// Intermediate value to force update
jfx_node.getValueFactory().setValue(value_text + " ");
jfx_node.getValueFactory().setValue(value_text);

If adding a space doesn't do it, maybe "!"?

@Synthetica9
Copy link
Author

Yes, the space variant works. I take from this discussion you don't see a better way to force this update either?

@kasemir
Copy link
Collaborator

kasemir commented Nov 7, 2024

don't see a better way

You already narrowed it down: We call "setValue", but the value is ignored.

It's not the only place where we need to hack around a missing refresh, see refreshHack in https://github.com/ControlSystemStudio/phoebus/blob/master/core/ui/src/main/java/org/phoebus/ui/javafx/ToolbarHelper.java

We could add a common comment like

// JavaFX Hack:

to those sections. That way, it'd be easier to find them all and then we can periodically check, like after JavaFX updates, if there's any improvement that might allow us to remove the hacks one by one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants