Skip to content

Commit 0aa4f5e

Browse files
odmirKarthikRIyer
authored andcommitted
Histogram bug fixes (#64)
* bfix: Check the amount of points passed in to `drawSolidPolygon` and `drawPlotLines` for the various renderers before rendering anything and add missing image verification to tests. * bfix: "fix" bug where in some cases and some renderers the x labels showed values with too many decimal places due to numeric error. In reality this just decreases the possibility of this happening again by reducing an extra division followed by a multiplication by the same value, which in some cases would result in loss of precision to the point where the string representation also had to change and show more decimal places. We should in the future round the number to a certain number of decimal places by default and give the user control over it. * bfix: Fix bug where the histogram bin edges were drawn slightly misaligned with the grid/markers And at the same time reveal a bug with drawing rectangles that touch each other where the x coordinate is not a whole number. When rendering with raster renderers this results in white lines with differing widths. * [bfix][rfac]: Fix bug where white lines appear through histogram bin edges due to previous commit. This is the easy approach, in the case of multiple series the colour of the series in the back might/will bleed through the edges of the one in the front, even when/if there are no columns of the back series nearby. Also refactored the code inside `drawData` slightly for better code reuse between `.bar` and `.step` cases. * Rename test numbers to make space for a new one. * Add new test for colour bleeding. 6 series, all the ways to pair 6 different colours side by side. * bfix: Colour from some series in the background were bleeding through some edges due to the last commit, this fixes that with an improved algorithm. This is the same idea as with the `.step` case, using conditions and rules to decide what points to add to form a polygon. In this case "tucking" the background series a little behind the foreground one by `floor`ing or `ceil`ing the right points. Also improved the algorithm for `.step` by removing one "instruction" and reducing the cases some of the instructions were being emitted redundantly to the minimum possible, while preserving the same output. * Change `floor`/`ceil` to `.rounded(.up)`/`.rounded(.down)`. * Change guard statements to preconditions. * Revert back to using rectangles for `.bar`. * Revert `barWidth` to being rounded, update the test images and run `swift test --generate-linuxmain`.
1 parent a2a94bb commit 0aa4f5e

25 files changed

+253
-35
lines changed

Sources/AGGRenderer/AGGRenderer/AGGRenderer.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,8 @@ public class AGGRenderer: Renderer{
142142

143143
public func drawSolidPolygon(points: [Point],
144144
fillColor: Color) {
145+
precondition(points.count > 2, "drawSolidPolygon: Cannot draw a polygon with \(points.count) points.")
146+
145147
var x = [Float]()
146148
var y = [Float]()
147149
for index in 0..<points.count {
@@ -186,6 +188,8 @@ public class AGGRenderer: Renderer{
186188
strokeWidth thickness: Float,
187189
strokeColor: Color,
188190
isDashed: Bool) {
191+
precondition(p.count > 1, "drawPlotLines: Cannot draw lines with \(p.count) points.")
192+
189193
var x = [Float]()
190194
var y = [Float]()
191195

Sources/QuartzRenderer/QuartzRenderer.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,8 @@ public class QuartzRenderer: Renderer {
371371

372372
public func drawSolidPolygon(points: [Point],
373373
fillColor: Color) {
374+
precondition(points.count > 2, "drawSolidPolygon: Cannot draw a polygon with \(points.count) points.")
375+
374376
let polygonPath = CGMutablePath()
375377
polygonPath.move(to: CGPoint(x: Double(points[0].x + xOffset), y: Double(points[0].y + yOffset)))
376378
for index in 1..<points.count {
@@ -405,7 +407,8 @@ public class QuartzRenderer: Renderer {
405407
strokeWidth thickness: Float,
406408
strokeColor: Color,
407409
isDashed: Bool) {
408-
guard !p.isEmpty else { return }
410+
precondition(p.count > 1, "drawPlotLines: Cannot draw lines with \(p.count) points.")
411+
409412
for i in 0..<p.count-1 {
410413
drawLine(startPoint: p[i],
411414
endPoint: p[i+1],

Sources/SVGRenderer/SVGRenderer.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ public class SVGRenderer: Renderer{
159159

160160
public func drawSolidPolygon(points: [Point],
161161
fillColor: Color) {
162+
precondition(points.count > 2, "drawSolidPolygon: Cannot draw a polygon with \(points.count) points.")
163+
162164
let pts = points.map { convertToSVGCoordinates($0) }
163165
var pointsString = ""
164166
for index in 0..<pts.count {
@@ -189,8 +191,8 @@ public class SVGRenderer: Renderer{
189191
strokeWidth thickness: Float,
190192
strokeColor: Color,
191193
isDashed: Bool) {
192-
guard p.count > 1 else { return }
193-
194+
precondition(p.count > 1, "drawPlotLines: Cannot draw lines with \(p.count) points.")
195+
194196
let pointsString = p.lazy.map { point in
195197
let convertedPoint = self.convertToSVGCoordinates(point)
196198
return "\(convertedPoint.x),\(convertedPoint.y)"

Sources/SwiftPlot/Histogram.swift

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,13 @@ extension Histogram: HasGraphLayout {
200200
}
201201

202202
let nX: Float = v2/scaleX
203-
var inc2: Float = nX
203+
var inc2: Float = v2
204204
if(size.width/nX > MAX_DIV){
205-
inc2 = (size.height/nX)*inc1/MAX_DIV
205+
inc2 = (size.height/v2)*inc1/MAX_DIV
206206
}
207207
let xM: Float = results.xMargin
208208
let scaleXInv = 1.0/scaleX
209-
let xIncrement = inc2*scaleX
209+
let xIncrement = inc2
210210
for i in stride(from: Float(minimumX), through: Float(maximumX), by: xIncrement) {
211211
markers.xMarkers.append((i-Float(minimumX))*scaleXInv + xM)
212212
markers.xMarkersText.append("\(i)")
@@ -221,37 +221,38 @@ extension Histogram: HasGraphLayout {
221221

222222
return (results, markers)
223223
}
224-
//functions to draw the plot
224+
225+
/// Draw with rectangles if `histogramType` is `.bar` or with lines if `histogramType` is `.step`.
225226
public func drawData(_ data: DrawingData, size: Size, renderer: Renderer) {
226227
let binCount = histogramSeries.bins
227228
let allSeries = [data.series_scaledBinFrequency] + data.stack_scaledBinFrequency
228229
let allSeriesInfo = [histogramSeries] + histogramStackSeries
230+
229231
switch histogramSeries.histogramSeriesOptions.histogramType {
230232
case .bar:
231233
let xStart = Float(data.xMargin)
232234
let xValues = stride(from: xStart, to: xStart + Float(binCount) * data.barWidth, by: data.barWidth)
233235

234-
// Get a `Slice` of frequencies for each series so we can take one element from each series for each x value
235-
var frequencySlices = allSeries.map { $0[...] }
236-
for x in xValues {
236+
// Iterate through each bar stacking the corresponding bar of each series.
237+
for (x, binIdx) in zip(xValues, 0..<binCount) {
237238
var currentHeight: Float = 0.0
238-
for (series, index) in zip(allSeries, frequencySlices.indices) {
239-
let height = frequencySlices[index].removeFirst()
239+
for seriesIdx in allSeriesInfo.indices {
240+
let height = allSeries[seriesIdx][binIdx]
240241
let rect = Rect(origin: Point(x, currentHeight), size: Size(width: data.barWidth, height: height))
241-
renderer.drawSolidRect(rect, fillColor: allSeriesInfo[index].color,
242-
hatchPattern: .none)
242+
renderer.drawSolidRect(rect, fillColor: allSeriesInfo[seriesIdx].color, hatchPattern: .none)
243243
currentHeight += height
244244
}
245-
currentHeight = 0.0
246245
}
247246
case .step:
247+
/// Accumulate the frequencies of each series.
248+
// One heights array for each series.
248249
let xStart = Float(data.xMargin)
249250
let xValues = stride(from: xStart, through: xStart + Float(binCount) * data.barWidth, by: data.barWidth)
250-
251+
251252
// One heights array for each series
252253
var seriesHeights: [[Float]] = [[Float](repeating: 0.0, count: binCount + 2)]
253254

254-
// Update `currentHeights` with the height from the series and add ìt to `heights`
255+
// Sum the bin frequencies from two series together and append to `seriesHeights`.
255256
var currentHeights = seriesHeights[seriesHeights.startIndex]
256257
for series in allSeries {
257258
for (newHeight, index) in zip(series, currentHeights.indices.dropFirst().dropLast()) {
@@ -260,40 +261,37 @@ extension Histogram: HasGraphLayout {
260261
seriesHeights.append(currentHeights)
261262
}
262263

263-
// Draw only the line segments that will actually be visible, unobstructed from other lines that will be on top
264-
// We iterate over the series in reverse to draw them from back to front
264+
// Iterate over the series in reverse to draw from back to front.
265265
var seriesHeightsSlice = seriesHeights.reversed()[...]
266266
var backHeightsSlice = seriesHeightsSlice.removeFirst()[...]
267267
for (frontHeights, seriesIdx) in zip(seriesHeightsSlice, allSeries.indices.reversed()) {
268268
var frontHeightsSlice = frontHeights[...]
269269
let series = allSeries[seriesIdx]
270270

271-
// Iterate over bin edges focusing on the height of the left and right bins of the series on the back and in front
271+
/// Iterate over bin edges focusing on the height of the left and right bins of the series on the back and in front.
272+
var line = [Point]()
272273
var backLeftBinHeight = backHeightsSlice.removeFirst()
273274
var frontLeftBinHeight = frontHeightsSlice.removeFirst()
274-
var line = [Point]()
275275
for ((backRightBinHeight, frontRightBinHeight), x) in zip(zip(backHeightsSlice, frontHeightsSlice), xValues) {
276-
277276
func endLine() {
278277
renderer.drawPlotLines(points: line, strokeWidth: strokeWidth,
279278
strokeColor: allSeriesInfo[seriesIdx].color,
280279
isDashed: false)
281280
line.removeAll(keepingCapacity: true)
282281
}
283282

284-
// Conditions for appending specific points or ending the line at different places based on the relative heights (4 measures)
283+
// Conditions for appending specific points or ending the line at different places based on the relative heights.
285284
let c1 = backLeftBinHeight > frontLeftBinHeight
286285
let c2 = backRightBinHeight > frontRightBinHeight
287286
let c3 = backLeftBinHeight > frontRightBinHeight
288287
let c4 = backRightBinHeight > frontLeftBinHeight
289288

290-
if c1 || c3 && c4 { line.append(Point(x, backLeftBinHeight)) }
291-
if !c3 { endLine() }
292-
if c1 && !c4 { line.append(Point(x, frontLeftBinHeight)) }
293-
if !c4 { endLine() }
294-
if c2 && !c3 { line.append(Point(x, frontRightBinHeight)) }
295-
if c2 || c3 && c4 { line.append(Point(x, backRightBinHeight)) }
296-
if !c2 { endLine() }
289+
if c1 || c3 && c4 { line.append(Point(x, backLeftBinHeight)) }
290+
if c1 && !c4 { line.append(Point(x, frontLeftBinHeight)) }
291+
if c1 && (!c3 || !c4) { endLine() }
292+
if c2 && !c3 { line.append(Point(x, frontRightBinHeight)) }
293+
if c2 || c3 && c4 { line.append(Point(x, backRightBinHeight)) }
294+
if !c2 && c3 && c4 { endLine() }
297295

298296
backLeftBinHeight = backRightBinHeight
299297
frontLeftBinHeight = frontRightBinHeight

Tests/SwiftPlotTests/Histogram/histogram-stacked-step.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ extension HistogramTests {
2424
}
2525

2626
func testHistogramStackedStepLineJoins() throws {
27-
let fileName = "_27_histogram_stacked_step_line_joins"
27+
let fileName = "_28_histogram_stacked_step_line_joins"
2828

2929
var histogram = Histogram<Float>(isNormalized: false, enableGrid: true)
3030
histogram.addSeries(data: [0, 13, 17, 17, 21, 25, 30, 34, 34, 38, 42, 45], bins: 49, label: "Plot 1", color: .blue, histogramType: .step)
@@ -36,7 +36,7 @@ extension HistogramTests {
3636
}
3737

3838
func testHistogramMultiStackedStep() throws {
39-
let fileName = "_26_histogram_multi_stacked_step"
39+
let fileName = "_27_histogram_multi_stacked_step"
4040

4141
let x: StrideTo<Float> = stride(from: 0, to: 2 * .pi, by: (2 * .pi)/100)
4242
let data1: [Float] = x.flatMap { [Float](repeating: $0, count: Int((sin($0) + 1)*10)) }

Tests/SwiftPlotTests/Histogram/histogram-stacked.swift

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,42 @@ extension HistogramTests {
2222

2323
try renderAndVerify(histogram, fileName: fileName)
2424
}
25-
25+
26+
func testHistogramMultiStackedColorBleed() throws {
27+
let fileName = "_25_histogram_multi_stacked_color_bleed"
28+
29+
var histogram = Histogram<Float>(isNormalized: false, enableGrid: true)
30+
histogram.addSeries(data: [5, 6, 7, 8, 9, 10, 15, 18], bins: 21, label: "Plot 1", color: .yellow, histogramType: .bar)
31+
histogram.addStackSeries(data: [4, 5, 6, 7, 8, 9, 15, 18], label: "Plot 2", color: .orange)
32+
histogram.addStackSeries(data: [3, 4, 5, 6, 7, 8, 15, 16], label: "Plot 3", color: .green)
33+
histogram.addStackSeries(data: [2, 3, 4, 5, 6, 7, 14, 15, 16], label: "Plot 4", color: .red)
34+
histogram.addStackSeries(data: [1, 2, 3, 4, 5, 6, 14, 16, 19], label: "Plot 5", color: .blue)
35+
histogram.addStackSeries(data: [0, 1, 2, 3, 4, 5, 14, 16, 17, 19], label: "Plot 6", color: .purple)
36+
37+
histogram.plotTitle = PlotTitle("HISTOGRAM MULTI STACKED")
38+
histogram.plotLabel = PlotLabel(xLabel: "X", yLabel: "Frequency")
39+
40+
41+
let svg_renderer = SVGRenderer()
42+
try histogram.drawGraphAndOutput(fileName: svgOutputDirectory+fileName,
43+
renderer: svg_renderer)
44+
verifyImage(name: fileName, renderer: .svg)
45+
#if canImport(AGGRenderer)
46+
let agg_renderer = AGGRenderer()
47+
try histogram.drawGraphAndOutput(fileName: aggOutputDirectory+fileName,
48+
renderer: agg_renderer)
49+
verifyImage(name: fileName, renderer: .agg)
50+
#endif
51+
#if canImport(QuartzRenderer)
52+
let quartz_renderer = QuartzRenderer()
53+
try histogram.drawGraphAndOutput(fileName: coreGraphicsOutputDirectory+fileName,
54+
renderer: quartz_renderer)
55+
verifyImage(name: fileName, renderer: .coreGraphics)
56+
#endif
57+
}
58+
2659
func testHistogramMultiStacked() throws {
27-
let fileName = "_25_histogram_multi_stacked"
60+
let fileName = "_26_histogram_multi_stacked"
2861

2962
let x: StrideTo<Float> = stride(from: 0, to: 2 * .pi, by: (2 * .pi)/100)
3063
let data1: [Float] = x.flatMap { [Float](repeating: $0, count: Int((sin($0) + 1)*10)) }
-58 Bytes
Loading
32 Bytes
Loading
18.3 KB
Loading

0 commit comments

Comments
 (0)