Skip to content

Commit 9d61567

Browse files
authored
Bounds not calculated properly for FastXYSeries (halfhp#49)
* uprev Gradle build tools 2.3.1 -> 2.3.2 * FastNumber's constructor is now private. Added safe static initializer; FastNumber.orNull which returns null if a null Number is passed into it. * halfhp#45 SeriesUtils now falls back to default min/max calculation on FastXYSeries whose bounds fall completely within the owning XYPlot's constraints.
1 parent 7dc7055 commit 9d61567

File tree

11 files changed

+172
-87
lines changed

11 files changed

+172
-87
lines changed

androidplot-core/src/main/java/com/androidplot/Region.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public Number length() {
7171
Number l = getMax() == null || getMin() == null ?
7272
null : getMax().doubleValue() - getMin().doubleValue();
7373
if(l != null) {
74-
cachedLength = new FastNumber(l);
74+
cachedLength = FastNumber.orNull(l);
7575
}
7676
}
7777
return cachedLength;
@@ -216,7 +216,7 @@ public void setMin(Number min) {
216216
this.min = null;
217217
}
218218
} else if (this.min == null || !this.min.equals(min)) {
219-
this.min = new FastNumber(min);
219+
this.min = FastNumber.orNull(min);
220220
}
221221
}
222222

@@ -238,7 +238,7 @@ public void setMax(Number max) {
238238
this.max = null;
239239
}
240240
} else if (this.max == null || !this.max.equals(max)) {
241-
this.max = new FastNumber(max);
241+
this.max = FastNumber.orNull(max);
242242
}
243243
}
244244

androidplot-core/src/main/java/com/androidplot/util/FastNumber.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88
*/
99
public class FastNumber extends Number {
1010

11-
@NonNull
12-
private final Number number;
11+
@NonNull private final Number number;
1312
private boolean hasDoublePrimitive;
1413
private boolean hasFloatPrimitive;
1514
private boolean hasIntPrimitive;
@@ -18,7 +17,21 @@ public class FastNumber extends Number {
1817
private float floatPrimitive;
1918
private int intPrimitive;
2019

21-
public FastNumber(@NonNull Number number) {
20+
/**
21+
* Safe-instantiator of FastNumber; returns a null result if the input Number is also null.
22+
* @param number
23+
* @return
24+
*/
25+
public static FastNumber orNull(@NonNull Number number) {
26+
if(number == null) {
27+
return null;
28+
} else {
29+
return new FastNumber(number);
30+
}
31+
}
32+
33+
private FastNumber(@NonNull Number number) {
34+
2235
//noinspection ConstantConditions //in case someone ignores the @NonNull annotation
2336
if (number == null) {
2437
throw new IllegalArgumentException("number parameter cannot be null");

androidplot-core/src/main/java/com/androidplot/util/SeriesUtils.java

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ public static Region minMaxY(XYSeries... seriesList) {
6565
* @since 0.9.7
6666
*/
6767
public static RectRegion minMax(XYConstraints constraints, List<XYSeries> seriesList) {
68-
// TODO: this is inefficient...clean it up!
6968
return minMax(constraints, seriesList.toArray(new XYSeries[seriesList.size()]));
7069
}
7170

@@ -86,23 +85,17 @@ public static RectRegion minMax(XYConstraints constraints, XYSeries... seriesArr
8685
for (XYSeries series : seriesArray) {
8786

8887
// if this is an advanced xy series then minMax have already been calculated for us:
89-
if(series instanceof FastXYSeries) {
90-
final RectRegion seriesBounds = ((FastXYSeries) series).minMax();
91-
if (seriesBounds == null) {
88+
boolean isPreCalculated = false;
89+
if (series instanceof FastXYSeries) {
90+
final RectRegion b = ((FastXYSeries) series).minMax();
91+
if(b == null) {
9292
continue;
9393
}
94-
if(constraints == null) {
95-
bounds.union(seriesBounds);
96-
} else {
97-
if (constraints.contains(seriesBounds.getMinX(), seriesBounds.getMinY())) {
98-
bounds.union(seriesBounds.getMinX(), seriesBounds.getMinY());
99-
}
100-
if (constraints.contains(seriesBounds.getMaxX(), seriesBounds.getMaxY())) {
101-
bounds.union(seriesBounds.getMaxX(), seriesBounds.getMaxY());
102-
}
94+
if(constraints == null || constraints.contains(b)) {
95+
bounds.union(b);
10396
}
104-
105-
} else if (series.size() > 0) {
97+
}
98+
if (!isPreCalculated && series.size() > 0) {
10699
for (int i = 0; i < series.size(); i++) {
107100
final Number xi = series.getX(i);
108101
final Number yi = series.getY(i);

androidplot-core/src/main/java/com/androidplot/xy/FastXYSeries.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
/**
44
* An implementation of {@link XYSeries} that defines additional methods to speed up rendering by
55
* giving a hint to the renderer about the min/max values contained in the series.
6+
*
7+
* Note that these hints can only be leveraged if the containing XYPlot's constraints completely
8+
* contain the FastXYSeries min/max values. If this condition is not met then XYPlot falls back
9+
* to manually determining the min/max values of the series that exist within the defined constraints.
610
*/
711
public interface FastXYSeries extends XYSeries {
812

androidplot-core/src/main/java/com/androidplot/xy/FixedSizeEditableXYSeries.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.androidplot.xy;
22

33
import android.support.annotation.NonNull;
4+
import android.support.annotation.Nullable;
45

56
import com.androidplot.util.FastNumber;
67

@@ -19,6 +20,7 @@ public class FixedSizeEditableXYSeries implements EditableXYSeries {
1920

2021
@NonNull
2122
private List<FastNumber> xVals = new ArrayList<>();
23+
2224
@NonNull
2325
private List<FastNumber> yVals = new ArrayList<>();
2426
private String title;
@@ -29,13 +31,13 @@ public FixedSizeEditableXYSeries(String title, int size) {
2931
}
3032

3133
@Override
32-
public void setX(@NonNull Number x, int index) {
33-
xVals.set(index, new FastNumber(x));
34+
public void setX(@Nullable Number x, int index) {
35+
xVals.set(index, FastNumber.orNull(x));
3436
}
3537

3638
@Override
37-
public void setY(@NonNull Number y, int index) {
38-
yVals.set(index, new FastNumber(y));
39+
public void setY(@Nullable Number y, int index) {
40+
yVals.set(index, FastNumber.orNull(y));
3941
}
4042

4143
/**

androidplot-core/src/main/java/com/androidplot/xy/XYConstraints.java

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
package com.androidplot.xy;
1818

19+
import android.support.annotation.NonNull;
20+
import android.support.annotation.Nullable;
21+
1922
/**
2023
* Calculates the min/max constraints for an xy plane.
2124
*
@@ -45,13 +48,18 @@ public XYConstraints() {
4548
this(null, null, null, null);
4649
}
4750

48-
public XYConstraints(Number minX, Number maxX, Number minY, Number maxY) {
51+
public XYConstraints(@Nullable Number minX, @Nullable Number maxX, @Nullable Number minY, @Nullable Number maxY) {
4952
this.minX = minX;
5053
this.minY = minY;
5154
this.maxX = maxX;
5255
this.maxY = maxY;
5356
}
5457

58+
public boolean contains(@NonNull RectRegion rectRegion) {
59+
return contains(rectRegion.getMinY(), rectRegion.getMinY())
60+
&& contains(rectRegion.getMaxX(), rectRegion.getMaxY());
61+
}
62+
5563
public boolean contains(Number x, Number y) {
5664
if (x == null || y == null) {
5765
// this is essentially an invisible point:
@@ -80,86 +88,96 @@ public boolean contains(Number x, Number y) {
8088
return true;
8189
}
8290

91+
@Nullable
8392
public Number getMinX() {
8493
return minX;
8594
}
8695

96+
@Nullable
8797
public Number getMaxX() {
8898
return maxX;
8999
}
90100

101+
@Nullable
91102
public Number getMinY() {
92103
return minY;
93104
}
94105

106+
@Nullable
95107
public Number getMaxY() {
96108
return maxY;
97109
}
98110

111+
public void setMinX(@Nullable Number minX) {
112+
this.minX = minX;
113+
}
114+
115+
public void setMaxX(@Nullable Number maxX) {
116+
this.maxX = maxX;
117+
}
118+
119+
public void setMinY(@Nullable Number minY) {
120+
this.minY = minY;
121+
}
122+
123+
public void setMaxY(@Nullable Number maxY) {
124+
this.maxY = maxY;
125+
}
126+
127+
@NonNull
99128
public XYFramingModel getDomainFramingModel() {
100129
return domainFramingModel;
101130
}
102131

103-
public void setDomainFramingModel(XYFramingModel domainFramingModel) {
132+
public void setDomainFramingModel(@NonNull XYFramingModel domainFramingModel) {
104133
this.domainFramingModel = domainFramingModel;
105134
}
106135

136+
@NonNull
107137
public XYFramingModel getRangeFramingModel() {
108138
return rangeFramingModel;
109139
}
110140

111-
public void setRangeFramingModel(XYFramingModel rangeFramingModel) {
141+
public void setRangeFramingModel(@NonNull XYFramingModel rangeFramingModel) {
112142
this.rangeFramingModel = rangeFramingModel;
113143
}
114144

145+
@NonNull
115146
public BoundaryMode getDomainUpperBoundaryMode() {
116147
return domainUpperBoundaryMode;
117148
}
118149

119-
public void setDomainUpperBoundaryMode(BoundaryMode domainUpperBoundaryMode) {
150+
public void setDomainUpperBoundaryMode(@NonNull BoundaryMode domainUpperBoundaryMode) {
120151
this.domainUpperBoundaryMode = domainUpperBoundaryMode;
121152
}
122153

154+
@NonNull
123155
public BoundaryMode getDomainLowerBoundaryMode() {
124156
return domainLowerBoundaryMode;
125157
}
126158

127-
public void setDomainLowerBoundaryMode(BoundaryMode domainLowerBoundaryMode) {
159+
public void setDomainLowerBoundaryMode(@NonNull BoundaryMode domainLowerBoundaryMode) {
128160
this.domainLowerBoundaryMode = domainLowerBoundaryMode;
129161
}
130162

163+
@NonNull
131164
public BoundaryMode getRangeUpperBoundaryMode() {
132165
return rangeUpperBoundaryMode;
133166
}
134167

135-
public void setRangeUpperBoundaryMode(BoundaryMode rangeUpperBoundaryMode) {
168+
public void setRangeUpperBoundaryMode(@NonNull BoundaryMode rangeUpperBoundaryMode) {
136169
this.rangeUpperBoundaryMode = rangeUpperBoundaryMode;
137170
}
138171

172+
@NonNull
139173
public BoundaryMode getRangeLowerBoundaryMode() {
140174
return rangeLowerBoundaryMode;
141175
}
142176

143-
public void setRangeLowerBoundaryMode(BoundaryMode rangeLowerBoundaryMode) {
177+
public void setRangeLowerBoundaryMode(@NonNull BoundaryMode rangeLowerBoundaryMode) {
144178
this.rangeLowerBoundaryMode = rangeLowerBoundaryMode;
145179
}
146180

147-
public void setMinX(Number minX) {
148-
this.minX = minX;
149-
}
150-
151-
public void setMaxX(Number maxX) {
152-
this.maxX = maxX;
153-
}
154-
155-
public void setMinY(Number minY) {
156-
this.minY = minY;
157-
}
158-
159-
public void setMaxY(Number maxY) {
160-
this.maxY = maxY;
161-
}
162-
163181
@Override
164182
public String toString() {
165183
return "XYConstraints{" + "domainFramingModel=" + domainFramingModel +

androidplot-core/src/main/java/com/androidplot/xy/XYPlot.java

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -694,44 +694,44 @@ private static double distance(double x, double y) {
694694

695695
public void updateDomainMinMaxForOriginModel() {
696696
double origin = userDomainOrigin.doubleValue();
697-
double maxXDelta = distance(bounds.getMaxX().doubleValue(), origin);
698-
double minXDelta = distance(bounds.getMinX().doubleValue(), origin);
699-
double delta = maxXDelta > minXDelta ? maxXDelta : minXDelta;
700-
double dlb = origin - delta;
701-
double dub = origin + delta;
697+
double maxDelta = distance(bounds.getMaxX().doubleValue(), origin);
698+
double minDelta = distance(bounds.getMinX().doubleValue(), origin);
699+
double delta = maxDelta > minDelta ? maxDelta : minDelta;
700+
double lowerBoundary = origin - delta;
701+
double upperBoundary = origin + delta;
702702
switch (domainOriginBoundaryMode) {
703703
case AUTO:
704-
bounds.setMinX(dlb);
705-
bounds.setMaxX(dub);
704+
bounds.setMinX(lowerBoundary);
705+
bounds.setMaxX(upperBoundary);
706706

707707
break;
708708
// if fixed, then the value already exists within "user" vals.
709709
case FIXED:
710710
break;
711711
case GROW: {
712712

713-
if (prevMinX == null || dlb < prevMinX.doubleValue()) {
714-
bounds.setMinX(dlb);
713+
if (prevMinX == null || lowerBoundary < prevMinX.doubleValue()) {
714+
bounds.setMinX(lowerBoundary);
715715
} else {
716716
bounds.setMinX(prevMinX);
717717
}
718718

719-
if (prevMaxX == null || dub > prevMaxX.doubleValue()) {
720-
bounds.setMaxX(dub);
719+
if (prevMaxX == null || upperBoundary > prevMaxX.doubleValue()) {
720+
bounds.setMaxX(upperBoundary);
721721
} else {
722722
bounds.setMaxX(prevMaxX);
723723
}
724724
}
725725
break;
726726
case SHRINK:
727-
if (prevMinX == null || dlb > prevMinX.doubleValue()) {
728-
bounds.setMinX(dlb);
727+
if (prevMinX == null || lowerBoundary > prevMinX.doubleValue()) {
728+
bounds.setMinX(lowerBoundary);
729729
} else {
730730
bounds.setMinX(prevMinX);
731731
}
732732

733-
if (prevMaxX == null || dub < prevMaxX.doubleValue()) {
734-
bounds.setMaxX(dub);
733+
if (prevMaxX == null || upperBoundary < prevMaxX.doubleValue()) {
734+
bounds.setMaxX(upperBoundary);
735735
} else {
736736
bounds.setMaxX(prevMaxX);
737737
}
@@ -745,14 +745,14 @@ public void updateRangeMinMaxForOriginModel() {
745745
switch (rangeOriginBoundaryMode) {
746746
case AUTO:
747747
double origin = userRangeOrigin.doubleValue();
748-
double maxYDelta = distance(bounds.getMaxY().doubleValue(), origin);
749-
double minYDelta = distance(bounds.getMinY().doubleValue(), origin);
750-
if (maxYDelta > minYDelta) {
751-
bounds.setMinY(origin - maxYDelta);
752-
bounds.setMaxY(origin + maxYDelta);
748+
double maxDelta = distance(bounds.getMaxY().doubleValue(), origin);
749+
double minDelta = distance(bounds.getMinY().doubleValue(), origin);
750+
if (maxDelta > minDelta) {
751+
bounds.setMinY(origin - maxDelta);
752+
bounds.setMaxY(origin + maxDelta);
753753
} else {
754-
bounds.setMinY(origin - minYDelta);
755-
bounds.setMaxY(origin + minYDelta);
754+
bounds.setMinY(origin - minDelta);
755+
bounds.setMaxY(origin + minDelta);
756756
}
757757
break;
758758
case FIXED:

0 commit comments

Comments
 (0)