Fix zoom transformations to be based on display coordinates instead of data coordinates #602

wirew0rm · 2023-08-16T17:52:07Z

Previously the zoomer would perform the scroll-zoom and panning calculations in data coordinates. This is identical to pixel coordinates for linear axes, but for e.g. log axes this leads to strange behaviour since it can result in negative data values.

This changes the Pan and pan and scroll behaviour, the rectangle zoom was naturally always using screen coordinates anyway, so this should also be more consistent.

To archive this a change to AbstractAxis was necessary to update cached variables on range change.

If the axis gets changed multiple times during event handling, but the cache is only updated during layout, axes transforms from the event handlers use outdated transformations. This has the effect that e.g. panning only applies the first mouse event in each pulse and the panning is a lot slower than the actual mouse movement.

Also incorporates a small fix in the pom to use valid automatic module names, converts the ErrorDataSetRendererStylingSample to a correct FxSampler sample and fixes 2 small bugs:

the cached value of isLogAxis in NumericAxis was never updated so it could not be switched by its property
the Plugins have been changed to use Panes instead of Groups for Positioning, resolving a bug where relayouting during an ongoing zoom action would shift the zoom rect to the upper left corner.

pr-explainer-bot · 2023-08-16T17:52:35Z

Pull Request Report

Changes

Updated chartfx-acc/pom.xml:
- Changed <project.moduleName> from io.fair-acc.acc to io.fair_acc.acc.
- Updated <javalin.version> to 3.9.0.
- Updated <jetty.version> to 9.4.29.v20200521.
- Updated <micrometer.version> to 1.5.1.
Updated chartfx-chart/pom.xml:
- Changed <project.moduleName> from io.fair-acc.chartfx to io.fair_acc.chartfx.
- Updated <sass.version> to 1.64.2.
- Updated <scss.inputDir> to ${project.basedir}/src/main/resources/io/fair_acc/chartfx/.
- Updated <css.outputDir> to ${scss.inputDir}.
Updated chartfx-chart/src/main/java/io/fair_acc/chartfx/axes/spi/AbstractAxisParameter.java:
- Added code to update cached variables after setting min and max properties.
Updated chartfx-chart/src/main/java/io.fair_acc/chartfx/plugins/Zoomer.java:
- Modified panChart method to fix panning behavior.
- Modified zoomOnAxis method to fix zooming behavior.
Updated chartfx-dataset/pom.xml:
- Changed <project.moduleName> from io.fair-acc.dataset to io.fair_acc.dataset.
Updated chartfx-generate/pom.xml:
- Changed <project.moduleName> from io.fair-acc.generate to io.fair_acc.generate.
Updated chartfx-math/pom.xml:
- Changed <project.moduleName> from io.fair-acc.math to io.fair_acc.math.
Updated chartfx-report/pom.xml:
- Changed <project.moduleName> from io.fair-acc.chartfx_report to io.fair_acc.chartfx_report.
Updated `chartfx-s

codecov · 2023-08-16T17:54:26Z

Codecov Report

Patch coverage: 58.62% and no project coverage change.

Comparison is base (14fcc0c) 49.62% compared to head (e63d771) 49.62%.

Additional details and impacted files

@@            Coverage Diff            @@
##               main     #602   +/-   ##
=========================================
  Coverage     49.62%   49.62%           
- Complexity     6238     6239    +1     
=========================================
  Files           378      379    +1     
  Lines         37583    37593   +10     
  Branches       6158     6159    +1     
=========================================
+ Hits          18652    18657    +5     
- Misses        17701    17710    +9     
+ Partials       1230     1226    -4

Files Changed	Coverage Δ
...t/src/main/java/io/fair_acc/chartfx/axes/Axis.java	`0.00% <0.00%> (ø)`
.../main/java/io/fair_acc/chartfx/plugins/Zoomer.java	`25.92% <0.00%> (-0.38%)`	⬇️
.../fair_acc/chartfx/axes/spi/DefaultNumericAxis.java	`68.53% <75.00%> (+0.13%)`	⬆️
...chart/src/main/java/io/fair_acc/chartfx/Chart.java	`79.41% <100.00%> (-0.28%)`	⬇️
...ava/io/fair_acc/chartfx/axes/spi/AbstractAxis.java	`80.89% <100.00%> (ø)`
...ir_acc/chartfx/axes/spi/AbstractAxisParameter.java	`92.66% <100.00%> (+0.02%)`	⬆️
...io/fair_acc/chartfx/axes/spi/OscilloscopeAxis.java	`75.15% <100.00%> (ø)`
...c/main/java/io/fair_acc/chartfx/utils/FXUtils.java	`89.34% <100.00%> (+0.25%)`	⬆️

... and 3 files with indirect coverage changes

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

There was a small bug in the logAxis properties invalidation listener. It updated the cached value with a method that used the cached value resulting in a nop instead of updating it with the new value of the property.

chartfx-chart/src/main/java/io/fair_acc/chartfx/axes/spi/AbstractAxisParameter.java

chartfx-chart/src/main/java/io/fair_acc/chartfx/Chart.java

If the axis gets changed multiple times during event handling, but the cache is only updated during layout, axes transforms from the event handlers use outdated transformations. This has the effect that e.g. panning only applies the first mouse event in each pulse and the panning is a lot slower than the actual mouse movement. This adds a method to the axes which can be called by plugins which use and update the axis range.

Previously the zoomer would perform the scroll-zoom and panning calculations in data coordinates. This is identical to pixel coordinates for linear axes, but for e.g. log axes this leads to strange behaviour since it can result in negative data values. This changes the Pan and pan and scroll behaviour, the rectangle zoom was naturally always using screen coordinates anyway, so this should also be more consistent.

It seems that groups are not well suited to positioning nodes on top of the graph. Replacing the Groups for the Plugins by Panes fixes an issue where the zoom rectangle would shift to the upper left corner of the chart area whenever there was a relayout while a rectangle zoom was ongoing.

sonarcloud · 2023-08-17T13:33:34Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
0.0% Duplication

wirew0rm temporarily deployed to coverage August 16, 2023 17:52 — with GitHub Actions Inactive

wirew0rm temporarily deployed to deploy August 16, 2023 17:52 — with GitHub Actions Inactive

wirew0rm temporarily deployed to deploy August 16, 2023 17:56 — with GitHub Actions Inactive

wirew0rm changed the title ~~Fix zoom transformations to be based on display coordinates rather than~~ Fix zoom transformations to be based on display coordinates instead of data coordinates Aug 16, 2023

wirew0rm temporarily deployed to coverage August 17, 2023 08:46 — with GitHub Actions Inactive

wirew0rm temporarily deployed to deploy August 17, 2023 08:49 — with GitHub Actions Inactive

wirew0rm temporarily deployed to deploy August 17, 2023 08:50 — with GitHub Actions Inactive

wirew0rm and others added 3 commits August 17, 2023 13:56

maven: fix automatic module name

66fedf5

NumericAxis: fix switching between log and linear axis

6d586af

There was a small bug in the logAxis properties invalidation listener. It updated the cached value with a method that used the cached value resulting in a nop instead of updating it with the new value of the property.

ErrorDataSetRendererStylingSample: convert to FxSampler sample

da296dd

wirew0rm force-pushed the fixZoomTransformations branch from c05ad2f to 10d7173 Compare August 17, 2023 11:59

wirew0rm temporarily deployed to coverage August 17, 2023 11:59 — with GitHub Actions Inactive

wirew0rm temporarily deployed to deploy August 17, 2023 12:02 — with GitHub Actions Inactive

wirew0rm temporarily deployed to coverage August 17, 2023 13:03 — with GitHub Actions Inactive

wirew0rm temporarily deployed to deploy August 17, 2023 13:07 — with GitHub Actions Inactive

wirew0rm requested a review from ennerf August 17, 2023 13:07

wirew0rm temporarily deployed to deploy August 17, 2023 13:07 — with GitHub Actions Inactive

ennerf reviewed Aug 17, 2023

View reviewed changes

chartfx-chart/src/main/java/io/fair_acc/chartfx/axes/spi/AbstractAxisParameter.java Outdated Show resolved Hide resolved

ennerf reviewed Aug 17, 2023

View reviewed changes

chartfx-chart/src/main/java/io/fair_acc/chartfx/Chart.java Outdated Show resolved Hide resolved

ennerf previously approved these changes Aug 17, 2023

View reviewed changes

wirew0rm and others added 3 commits August 17, 2023 15:31

wirew0rm dismissed ennerf’s stale review via e63d771 August 17, 2023 13:32

wirew0rm force-pushed the fixZoomTransformations branch from 07a3168 to e63d771 Compare August 17, 2023 13:32

wirew0rm temporarily deployed to coverage August 17, 2023 13:32 — with GitHub Actions Inactive

wirew0rm temporarily deployed to deploy August 17, 2023 13:36 — with GitHub Actions Inactive

wirew0rm merged commit ca15519 into main Aug 17, 2023
13 checks passed

wirew0rm deleted the fixZoomTransformations branch August 17, 2023 14:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix zoom transformations to be based on display coordinates instead of data coordinates #602

Fix zoom transformations to be based on display coordinates instead of data coordinates #602

wirew0rm commented Aug 16, 2023 •

edited

pr-explainer-bot bot commented Aug 16, 2023

codecov bot commented Aug 16, 2023 •

edited

sonarcloud bot commented Aug 17, 2023

Fix zoom transformations to be based on display coordinates instead of data coordinates #602

Fix zoom transformations to be based on display coordinates instead of data coordinates #602

Conversation

wirew0rm commented Aug 16, 2023 • edited

pr-explainer-bot bot commented Aug 16, 2023

Pull Request Report

Changes

codecov bot commented Aug 16, 2023 • edited

Codecov Report

sonarcloud bot commented Aug 17, 2023

wirew0rm commented Aug 16, 2023 •

edited

codecov bot commented Aug 16, 2023 •

edited