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

fixed setAutoRangePadding() regression bug #277

Merged
merged 2 commits into from
Oct 19, 2020
Merged

Conversation

RalphSteinhagen
Copy link
Member

Fixed setAutoRangePadding() regression bug and simplified auto range handling

recomputation and redrawing of axes (on range-, tick unit, and notably scale changes) and update of the internal caches was done sometimes out-of-order causing the canvas being redrawn before the new axis settings have been in place. Previously this appeared to be less frequently visible since the change of the min/max axis range property triggered subsequent update events and thus drawing of the canvas.

N.B. Interestingly, this did not show up in the numeric unit-tests and only while visually testing. The numerical API results seemed to have been always correct and only the canvas update has been missing sometimes.

Re-ordered recomputation of auto ranges, moved most of the computation into the AbstractAxis::invalidateRange(..) removed redundant code in the AbstractAxis and derived classes (notably 'getAxisRange() which has been performing the same function as the existing 'getRange()). The performance -- especially for rare or one-time updates -- should be more consistent now.

Updated the SimpleChartSample to reflect and test these changes (here with 50% padding on top and bottom of the DataSet):
image

and updated API in the commented code section (became dated/incorrect). Collateral effects remain to be checked/tested.

@codecov
Copy link

codecov bot commented Oct 18, 2020

Codecov Report

Merging #277 into master will decrease coverage by 0.00%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #277      +/-   ##
============================================
- Coverage     49.43%   49.42%   -0.01%     
+ Complexity     6850     6837      -13     
============================================
  Files           380      380              
  Lines         40108    40056      -52     
  Branches       6426     6418       -8     
============================================
- Hits          19826    19799      -27     
+ Misses        18891    18858      -33     
- Partials       1391     1399       +8     
Impacted Files Coverage Δ Complexity Δ
...hartfx-chart/src/main/java/de/gsi/chart/Chart.java 76.47% <ø> (-0.24%) 101.00 <0.00> (-1.00)
...rc/main/java/de/gsi/chart/axes/spi/LinearAxis.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...c/main/java/de/gsi/chart/axes/spi/NumericAxis.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...n/java/de/gsi/chart/axes/spi/OscilloscopeAxis.java 76.25% <35.71%> (-2.98%) 41.00 <2.00> (-1.00)
...java/de/gsi/chart/axes/spi/DefaultNumericAxis.java 68.59% <52.94%> (-0.48%) 60.00 <1.00> (+2.00) ⬇️
...rtfx-chart/src/main/java/de/gsi/chart/XYChart.java 71.42% <71.42%> (+3.58%) 56.00 <1.00> (-4.00) ⬆️
.../main/java/de/gsi/chart/axes/spi/AbstractAxis.java 75.52% <95.00%> (-0.65%) 176.00 <4.00> (-7.00)
...a/de/gsi/chart/axes/spi/AbstractAxisParameter.java 97.64% <100.00%> (-0.07%) 199.00 <1.00> (-6.00)
...main/java/de/gsi/chart/ui/HiddenSidesPaneSkin.java 43.54% <0.00%> (+0.47%) 29.00% <0.00%> (+1.00%)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d41feb8...feb6cdf. Read the comment docs.

@RalphSteinhagen RalphSteinhagen force-pushed the fixAutoRangePadding branch 2 times, most recently from 3a68a7d to feb6cdf Compare October 19, 2020 14:29
…oscopeAxis and DefaultNumericAxis

N.B. fixes problem in OscilloscopeAxisSample regarding switching from an auto-ranging to user-defined axis two-fold:
a) an ill-defined tickUnit could previously yield an nearly unlimited amount of tick marks (i.e. tickUnit == 200 for > 2e11 range) -> fixed in calculateMajorTickValues(...) and  calculateMinorTickValues(...)
b) switching between auto-ranging and user-based ranging could leave the scale/tick unit dangling for one update if forceRedraw() function was called ... fixed in recomputeTickMarks() by explicitly updating the tick unit in both layoutChildren and forceRedraw() function...
Copy link
Member Author

Codacy Here is an overview of what got changed by this pull request:

Complexity decreasing per file
==============================
+ chartfx-chart/src/main/java/de/gsi/chart/XYChart.java  -2
         

See the complete overview on Codacy

Copy link
Member

@wirew0rm wirew0rm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@wirew0rm wirew0rm merged commit 246a1f9 into master Oct 19, 2020
@wirew0rm wirew0rm deleted the fixAutoRangePadding branch October 19, 2020 15:31
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.

None yet

2 participants