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

Reenable MathDataSet and ParameterMeasurements #616

Merged
merged 7 commits into from
Sep 25, 2023
Merged

Conversation

wirew0rm
Copy link
Member

Reenable the DataSet processing capabilities with the new DirtyFlag based Event system.

MathDataSet and Measurements have their own dirty flag, which subscribes with an invalidationListener to the dirty flags of the datasets, indicators and measurements it depends upon. This flag, together with an update function is added to an EventProcessor, which reevaluates all modified entities.

There can be multiple EventProcessor implementations, this PR provides an AnimationTimer based one, which computes updates on the FxApplicationThread (but outside of the render loops dataset locks!) and a Thread based one which computes all updates on a single background daemon thread. This can be extended in the future to use e.g. a ThreadPool.

One thing to keep in mind is to be careful with locking datasets inside of the functions supplied to the MathDataSet. Since the renderer holds read locks to all shown datasets, getting a write lock on more than the MathDataSet itself will cause Deadlocks

The MathDataset can be observed in the TSpectrumSample and the HistogramRendererBasicSample. The ParameterMeasurements can e.g. be used in the ErrorDataSetRenderer sample from the dropdown toolbar menu.

The main part of the changes is in the last commit, the previous commits are mostly independent small changes.

Scenic view adds dependencies on jfx 11 which sometimes override the
jfx13 dependencies of chart-fx. This leads to errors with renderers
which depend on jfx13 features, leading to MethodNotFound exceptions.
The panner plugin has been long superseded and deprecated by the Zooomer
plugin and this is a good chance to finally retire it, since it could
cause confusion if zoomer and panner where both added to a chart.
Delete to force fixing all occurences of the old event system.
@pr-explainer-bot
Copy link

Pull Request Review

Hey there! 👋 I hope you're doing well. I've summarized the previous results for you to write a Pull Request review markdown doc. Let's get started!

Changes

  1. Removed version tags for dependencies javafx-base and javafx-controls in pom.xml.
  2. Removed version tag for dependency javafx-swing in pom.xml.
  3. Removed import statement for io.fair_acc.dataset.events.BitState in AbstractSingleValueIndicator.java.
  4. Removed import statement for io.fair_acc.dataset.event.EventSource in AbstractSingleValueIndicator.java.
  5. Removed unused variable state in AbstractSingleValueIndicator.java.
  6. Removed unused import statement for io.fair_acc.dataset.events.EventSource in AbstractValueIndicator.java.
  7. Removed unused import statement for io.fair_acc.dataset.events.BitState in AbstractValueIndicator.java.
  8. Removed unused import statement for io.fair_acc.dataset.events.ChartBits in AbstractValueIndicator.java.
  9. Removed unused import statement for io.fair_acc.chartfx.plugins.measurements.TrendingMeasurements in ParameterMeasurements.java.
  10. Removed unused import statement for io.fair_acc.dataset.events.EventProcessor in EventProcessor.java.
  11. The entire Panner class has been removed from the file Panner.java.
  12. The DEFAULT_MOUSE_FILTER constant has been moved to another file or deleted.
  13. The Panner class no longer extends ChartPlugin.
  14. The Panner class no longer has the axisMode property.
  15. The Panner class no longer has the dragCursor property.
  16. The Panner class no longer has the mouseFilter property.
  17. The Panner class no longer has the panStartHandler, panDragHandler, and panEndHandler event handlers.
  18. The Panner class no longer has the installCursor(), panChart(), panDragged(), panEnded(), panOngoing(), panStarted(), registerMouseHandlers(), setAxisMode(), setDragCursor(), setMouseFilter(), shiftBounds(), and uninstallCursor() methods.

Suggestions

  1. In Chart.java, line 29, import javafx.scene.Group; is added.
  2. In Chart.java, line 92, protected final Group pluginsArea = FXUtils.createUnmanagedGroup(); is added.
  3. In Chart.java, line 824, private final Map<ChartPlugin, Group> pluginGroups = new HashMap<>(); is added.
  4. In Chart.java, line 834, protected void pluginAdded(final ChartPlugin plugin) { method is modified to use Group instead of Pane.
  5. In Chart.java, line 834, protected void pluginRemoved(final ChartPlugin plugin) { method is modified to use Group instead of Pane.
  6. In Chart.java, line 912, protected void updatePluginsArea() { method is modified to use Group instead of Pane.
  7. In FxEventProcessor.java, line 15, import org.apache.commons.lang3.tuple.Pair; is added.
  8. In FxEventProcessor.java, line 34, private static final AtomicReference<FxEventProcessor> INSTANCE = new AtomicReference<>(); is added.
  9. In FxEventProcessor.java, line 36-39, public static FxEventProcessor getInstance() { method is added.
  10. In FxEventProcessor.java, line 45, public FxEventProcessor() { constructor is added.
  11. In AbstractSingleValueIndicator.java, line 60, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPluginState, ChartBits.ChartLayout).
  12. In AbstractSingleValueIndicator.java, line 70, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPluginState).
  13. In AbstractValueIndicator.java, line 49, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPlugins).
  14. In AbstractValueIndicator.java, line 57, instead of calling layoutChildren(), it could call...

Bugs

  1. In Chart.java, line 29, import javafx.scene.Group; is added.
  2. In Chart.java, line 92, protected final Group pluginsArea = FXUtils.createUnmanagedGroup(); is added.
  3. In Chart.java, line 824, private final Map<ChartPlugin, Group> pluginGroups = new HashMap<>(); is added.
  4. In Chart.java, line 834, protected void pluginAdded(final ChartPlugin plugin) { method is modified to use Group instead of Pane.
  5. In Chart.java, line 834, protected void pluginRemoved(final ChartPlugin plugin) { method is modified to use Group instead of Pane.
  6. In Chart.java, line 912, protected void updatePluginsArea() { method is modified to use Group instead of Pane.
  7. In FxEventProcessor.java, line 15, import org.apache.commons.lang3.tuple.Pair; is added.
  8. In FxEventProcessor.java, line 34, private static final AtomicReference<FxEventProcessor> INSTANCE = new AtomicReference<>(); is added.
  9. In FxEventProcessor.java, line 36-39, public static FxEventProcessor getInstance() { method is added.
  10. In FxEventProcessor.java, line 45, public FxEventProcessor() { constructor is added.
  11. In AbstractSingleValueIndicator.java, line 60, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPluginState, ChartBits.ChartLayout).
  12. In AbstractSingleValueIndicator.java, line 70, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPluginState).
  13. In AbstractValueIndicator.java, line 49, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPlugins).
  14. In AbstractValueIndicator.java, line 57, instead of calling layoutChildren(), it could call...

Improvements

  1. In Chart.java, line 29, import javafx.scene.Group; is added.
  2. In Chart.java, line 92, protected final Group pluginsArea = FXUtils.createUnmanagedGroup(); is added.
  3. In Chart.java, line 824, private final Map<ChartPlugin, Group> pluginGroups = new HashMap<>(); is added.
  4. In Chart.java, line 834, protected void pluginAdded(final ChartPlugin plugin) { method is modified to use Group instead of Pane.
  5. In Chart.java, line 834, protected void pluginRemoved(final ChartPlugin plugin) { method is modified to use Group instead of Pane.
  6. In Chart.java, line 912, protected void updatePluginsArea() { method is modified to use Group instead of Pane.
  7. In FxEventProcessor.java, line 15, import org.apache.commons.lang3.tuple.Pair; is added.
  8. In FxEventProcessor.java, line 34, private static final AtomicReference<FxEventProcessor> INSTANCE = new AtomicReference<>(); is added.
  9. In FxEventProcessor.java, line 36-39, public static FxEventProcessor getInstance() { method is added.
  10. In FxEventProcessor.java, line 45, public FxEventProcessor() { constructor is added.
  11. In AbstractSingleValueIndicator.java, line 60, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPluginState, ChartBits.ChartLayout).
  12. In AbstractSingleValueIndicator.java, line 70, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPluginState).
  13. In AbstractValueIndicator.java, line 49, instead of calling layoutChildren(), it could call getBitState().setDirty(ChartBits.ChartPlugins).
  14. In AbstractValueIndicator.java, line 57, instead of calling layoutChildren(), it could call...

Rating

Rate the code from 0 to 10 based on the following criteria:

  • Readability:
  • Performance:
  • Security:

Please provide a brief explanation for each criterion.

That's it! You're all set to write your Pull Request review markdown doc. Good luck! 🚀

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Patch coverage: 34.02% and project coverage change: +0.11% 🎉

Comparison is base (16a1602) 48.00% compared to head (cfadee2) 48.11%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #616      +/-   ##
============================================
+ Coverage     48.00%   48.11%   +0.11%     
- Complexity     6180     6210      +30     
============================================
  Files           394      373      -21     
  Lines         38176    38206      +30     
  Branches       6085     6101      +16     
============================================
+ Hits          18325    18383      +58     
+ Misses        18703    18676      -27     
+ Partials       1148     1147       -1     
Files Changed Coverage Δ
...a/io/fair_acc/chartfx/events/FxEventProcessor.java 0.00% <0.00%> (ø)
...c/chartfx/plugins/AbstractRangeValueIndicator.java 0.00% <0.00%> (ø)
.../io/fair_acc/chartfx/plugins/UpdateAxisLabels.java 0.00% <0.00%> (ø)
...a/io/fair_acc/chartfx/plugins/XRangeIndicator.java 0.00% <0.00%> (ø)
...a/io/fair_acc/chartfx/plugins/XValueIndicator.java 69.04% <0.00%> (+2.38%) ⬆️
...a/io/fair_acc/chartfx/plugins/YRangeIndicator.java 0.00% <0.00%> (ø)
...a/io/fair_acc/chartfx/plugins/YValueIndicator.java 69.04% <0.00%> (+5.63%) ⬆️
...fair_acc/chartfx/plugins/YWatchValueIndicator.java 84.31% <ø> (-1.97%) ⬇️
...artfx/plugins/measurements/SimpleMeasurements.java 81.76% <0.00%> (+49.00%) ⬆️
.../plugins/measurements/utils/CheckedValueField.java 88.88% <ø> (-8.57%) ⬇️
... and 22 more

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* Port Measurements and MathDataSets to new event system
* indicators: use and set dirty flags
* event processors: thread and animation timer based
* split trending measurements from dataset measurements
* feedback from @ennerf:
  * changed to fireInvalidated shorthand
  * removed locks from bitstate
  * ValueIndicators: move from bitstate to directly calling postListener
  * YIndicator: fix drag offset
@sonarcloud
Copy link

sonarcloud bot commented Sep 25, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 40 Code Smells

No Coverage information No Coverage information
26.9% 26.9% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@wirew0rm wirew0rm merged commit 4005ab0 into main Sep 25, 2023
11 of 14 checks passed
@wirew0rm wirew0rm deleted the dataset-events branch September 25, 2023 16:45
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

Successfully merging this pull request may close these issues.

1 participant