Skip to content

Commit b399644

Browse files
committed
Fix some layout issues in GraphLayout after analysing test regressions.
1 parent c421069 commit b399644

File tree

2 files changed

+67
-37
lines changed

2 files changed

+67
-37
lines changed

Sources/SwiftPlot/GraphLayout.swift

Lines changed: 53 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ public enum LegendIcon {
55
case shape(ScatterPlotSeriesOptions.ScatterPattern, Color)
66
}
77

8+
// TODO: Add layout tests for:
9+
// - No axes labels
10+
// - No plot markers
11+
// - No axes labels or plot markers
12+
813
/// A component for laying-out and rendering rectangular graphs.
914
///
1015
/// The principle 3 components of a `GraphLayout` are:
@@ -44,8 +49,8 @@ public struct GraphLayout {
4449
/// The region of the plot which will actually be filled with chart data.
4550
let plotBorderRect: Rect
4651

47-
// TODO: Try to move this up to GraphLayout itself.
48-
let components: EdgeComponents<[LayoutComponent]>
52+
/// All of the `LayoutComponent`s on this graph, including built-in components.
53+
let allComponents: EdgeComponents<[LayoutComponent]>
4954

5055
/// The measured sizes of the plot elements.
5156
let sizes: EdgeComponents<[Size]>
@@ -101,29 +106,38 @@ extension GraphLayout {
101106
let plotRect = layoutPlotRect(plotSize: plotSize, componentSizes: sizes)
102107
let roundedMarkers = markers.map { var m = $0; roundMarkers(&m); return m } ?? PlotMarkers()
103108

104-
var results = LayoutPlan(totalSize: size,
105-
plotBorderRect: plotRect,
106-
components: components,
107-
sizes: sizes,
108-
plotMarkers: roundedMarkers,
109-
legendLabels: legendInfo ?? [])
110-
calcMarkerTextLocations(renderer: renderer, plan: &results)
111-
calcLegend(results.legendLabels, renderer: renderer, plan: &results)
112-
return (drawingData, results)
109+
var plan = LayoutPlan(totalSize: size,
110+
plotBorderRect: plotRect,
111+
allComponents: components,
112+
sizes: sizes,
113+
plotMarkers: roundedMarkers,
114+
legendLabels: legendInfo ?? [])
115+
calcMarkerTextLocations(renderer: renderer, plan: &plan)
116+
calcLegend(plan.legendLabels, renderer: renderer, plan: &plan)
117+
return (drawingData, plan)
113118
}
114119

115120
static let xLabelPadding: Float = 12
116121
static let yLabelPadding: Float = 12
117122
static let titleLabelPadding: Float = 16
118123

119-
// FIXME: To be removed. These items should already be LayoutComponents.
124+
static let markerStemLength: Float = 6
125+
/// Padding around y marker-labels.
126+
static let yMarkerSpace: Float = 4
127+
/// Padding around x marker-labels.
128+
static let xMarkerSpace: Float = 6
129+
130+
// FIXME: To be removed. These items should already be `LayoutComponent`s.
120131
private func makeLayoutComponents() -> EdgeComponents<[LayoutComponent]> {
121132
var elements = EdgeComponents<[LayoutComponent]>(left: [], top: [], right: [], bottom: [])
122-
// TODO: Currently, only labels are "LayoutComponent"s.
123133
if !plotLabel.xLabel.isEmpty {
124134
let label = Label(text: plotLabel.xLabel, size: plotLabel.size)
125135
.padding(.all(Self.xLabelPadding))
126136
elements.bottom.append(label)
137+
// Add a space, otherwise the label looks misaligned.
138+
elements.bottom.append(FixedSpace(size: Self.titleLabelPadding))
139+
} else {
140+
elements.bottom.append(FixedSpace(size: Self.xLabelPadding))
127141
}
128142
if !plotLabel.yLabel.isEmpty {
129143
let label = Label(text: plotLabel.yLabel, size: plotLabel.size)
@@ -139,6 +153,8 @@ extension GraphLayout {
139153
let label = Label(text: plotTitle.title, size: plotTitle.size)
140154
.padding(.all(Self.titleLabelPadding))
141155
elements.top.append(label)
156+
} else {
157+
elements.top.append(FixedSpace(size: Self.titleLabelPadding))
142158
}
143159
return elements
144160
}
@@ -155,9 +171,9 @@ extension GraphLayout {
155171

156172
// Subtract space for the markers.
157173
// TODO: Make this more accurate.
158-
plotSize.height -= (2 * markerTextSize) + 10 // X markers
159-
plotSize.width -= yMarkerMaxWidth + 10 // Y markers
160-
plotSize.width -= yMarkerMaxWidth + 10 // Y2 markers
174+
plotSize.height -= (Self.markerStemLength + (2 * Self.xMarkerSpace) + markerTextSize) // X markers
175+
plotSize.width -= (Self.markerStemLength + (2 * Self.yMarkerSpace) + yMarkerMaxWidth) // Y markers
176+
plotSize.width -= (Self.markerStemLength + (2 * Self.yMarkerSpace) + yMarkerMaxWidth) // Y2 markers
161177
// Subtract space for border thickness.
162178
plotSize.height -= 2 * plotBorder.thickness
163179
plotSize.width -= 2 * plotBorder.thickness
@@ -180,16 +196,16 @@ extension GraphLayout {
180196
plotOrigin.x += componentSizes.left.reduce(into: 0) { $0 += $1.width }
181197
plotOrigin.y += componentSizes.bottom.reduce(into: 0) { $0 += $1.height }
182198
// Offset by marker sizes (TODO: they are not PlotElements yet, so not handled above).
183-
let xMarkerHeight = (2 * markerTextSize) + 10 // X markers
184-
let yMarkerWidth = yMarkerMaxWidth + 10 // Y markers
199+
let xMarkerHeight = (Self.markerStemLength + (2 * Self.xMarkerSpace) + markerTextSize) // X markers
200+
let yMarkerWidth = (Self.markerStemLength + (2 * Self.yMarkerSpace) + yMarkerMaxWidth) // Y markers
185201
plotOrigin.y += xMarkerHeight
186202
plotOrigin.x += yMarkerWidth
187203
// Offset by plot thickness.
188204
plotOrigin.x += plotBorder.thickness
189205
plotOrigin.y += plotBorder.thickness
190206

191207
// These are the final coordinates of the plot's internal space, so update `results`.
192-
return Rect(origin: plotOrigin, size: plotSize)
208+
return Rect(origin: plotOrigin, size: plotSize).roundedInwards
193209
}
194210

195211
/// Makes adjustments to the layout as requested by the plot.
@@ -220,14 +236,15 @@ extension GraphLayout {
220236
}
221237

222238
private func calcMarkerTextLocations(renderer: Renderer, plan: inout LayoutPlan) {
223-
239+
let xLabelOffset = plotBorder.thickness + Self.markerStemLength + Self.xMarkerSpace
240+
let yLabelOffset = plotBorder.thickness + Self.markerStemLength + Self.yMarkerSpace
224241
for i in 0..<plan.plotMarkers.xMarkers.count {
225-
let textWidth = renderer.getTextWidth(text: plan.plotMarkers.xMarkersText[i], textSize: markerTextSize)
242+
let textSize = renderer.getTextLayoutSize(text: plan.plotMarkers.xMarkersText[i], textSize: markerTextSize)
226243
let markerLocation = plan.plotMarkers.xMarkers[i]
227-
var textLocation = Point(0, -2.0 * markerTextSize)
244+
var textLocation = Point(0, -xLabelOffset - textSize.height)
228245
switch markerLabelAlignment {
229246
case .atMarker:
230-
textLocation.x = markerLocation - (textWidth/2)
247+
textLocation.x = markerLocation - (textSize.width/2)
231248
case .betweenMarkers:
232249
let nextMarkerLocation: Float
233250
if i < plan.plotMarkers.xMarkers.endIndex - 1 {
@@ -236,7 +253,7 @@ extension GraphLayout {
236253
nextMarkerLocation = plan.plotBorderRect.width
237254
}
238255
let midpoint = markerLocation + (nextMarkerLocation - markerLocation)/2
239-
textLocation.x = midpoint - (textWidth/2)
256+
textLocation.x = midpoint - (textSize.width/2)
240257
}
241258
plan.xMarkersTextLocation.append(textLocation)
242259
}
@@ -263,7 +280,7 @@ extension GraphLayout {
263280
var textSize = renderer.getTextLayoutSize(text: plan.plotMarkers.yMarkersText[i],
264281
textSize: markerTextSize)
265282
textSize.width = min(textSize.width, yMarkerMaxWidth)
266-
var textLocation = Point(-textSize.width - 8, 0)
283+
var textLocation = Point(-yLabelOffset - textSize.width, 0)
267284
textLocation.y = alignYLabel(markers: plan.plotMarkers.yMarkers, index: i, textSize: textSize)
268285
plan.yMarkersTextLocation.append(textLocation)
269286
}
@@ -272,7 +289,7 @@ extension GraphLayout {
272289
var textSize = renderer.getTextLayoutSize(text: plan.plotMarkers.y2MarkersText[i],
273290
textSize: markerTextSize)
274291
textSize.width = min(textSize.width, yMarkerMaxWidth)
275-
var textLocation = Point(plan.plotBorderRect.width + 8, 0)
292+
var textLocation = Point(yLabelOffset + plan.plotBorderRect.width, 0)
276293
textLocation.y = alignYLabel(markers: plan.plotMarkers.y2Markers, index: i, textSize: textSize)
277294
plan.y2MarkersTextLocation.append(textLocation)
278295
}
@@ -324,7 +341,7 @@ extension GraphLayout {
324341
if drawsGridOverForeground {
325342
drawGrid(plan, renderer: renderer)
326343
}
327-
drawLayoutComponents(plan.components, plotRect: plan.plotBorderRect,
344+
drawLayoutComponents(plan.allComponents, plotRect: plan.plotBorderRect,
328345
measuredSizes: plan.sizes, renderer: renderer)
329346
drawLegend(plan.legendLabels, plan: plan, renderer: renderer)
330347
drawAnnotations(resolver: plan, renderer: renderer)
@@ -438,31 +455,32 @@ extension GraphLayout {
438455
let rect = plan.plotBorderRect
439456
let border = plotBorder.thickness
440457
for index in 0..<plan.plotMarkers.xMarkers.count {
441-
let p1 = Point(plan.plotMarkers.xMarkers[index], -border - 6) + rect.origin
442-
let p2 = Point(plan.plotMarkers.xMarkers[index], -border) + rect.origin
458+
// Draw stem.
459+
let p1 = Point(plan.plotMarkers.xMarkers[index], -border) + rect.origin
460+
let p2 = Point(plan.plotMarkers.xMarkers[index], -border - Self.markerStemLength) + rect.origin
443461
renderer.drawLine(startPoint: p1,
444462
endPoint: p2,
445463
strokeWidth: markerThickness,
446464
strokeColor: plotBorder.color,
447465
isDashed: false)
448466
renderer.drawText(text: plan.plotMarkers.xMarkersText[index],
449-
location: plan.xMarkersTextLocation[index] + rect.origin + Pair(0, -border),
467+
location: plan.xMarkersTextLocation[index] + rect.origin,
450468
textSize: markerTextSize,
451469
color: plotBorder.color,
452470
strokeWidth: 0.7,
453471
angle: 0)
454472
}
455473

456474
for index in 0..<plan.plotMarkers.yMarkers.count {
457-
let p1 = Point(-border - 6, plan.plotMarkers.yMarkers[index]) + rect.origin
475+
let p1 = Point(-border - Self.markerStemLength, plan.plotMarkers.yMarkers[index]) + rect.origin
458476
let p2 = Point(-border, plan.plotMarkers.yMarkers[index]) + rect.origin
459477
renderer.drawLine(startPoint: p1,
460478
endPoint: p2,
461479
strokeWidth: markerThickness,
462480
strokeColor: plotBorder.color,
463481
isDashed: false)
464482
renderer.drawText(text: plan.plotMarkers.yMarkersText[index],
465-
location: plan.yMarkersTextLocation[index] + rect.origin + Pair(-border, 0),
483+
location: plan.yMarkersTextLocation[index] + rect.origin,
466484
textSize: markerTextSize,
467485
color: plotBorder.color,
468486
strokeWidth: 0.7,
@@ -471,17 +489,15 @@ extension GraphLayout {
471489

472490
if !plan.plotMarkers.y2Markers.isEmpty {
473491
for index in 0..<plan.plotMarkers.y2Markers.count {
474-
let p1 = Point(plan.plotBorderRect.width + border,
475-
(plan.plotMarkers.y2Markers[index])) + rect.origin
476-
let p2 = Point(plan.plotBorderRect.width + border + 6,
477-
(plan.plotMarkers.y2Markers[index])) + rect.origin
492+
let p1 = Point(rect.width + border, plan.plotMarkers.y2Markers[index]) + rect.origin
493+
let p2 = Point(rect.width + border + Self.markerStemLength, plan.plotMarkers.y2Markers[index]) + rect.origin
478494
renderer.drawLine(startPoint: p1,
479495
endPoint: p2,
480496
strokeWidth: markerThickness,
481497
strokeColor: plotBorder.color,
482498
isDashed: false)
483499
renderer.drawText(text: plan.plotMarkers.y2MarkersText[index],
484-
location: plan.y2MarkersTextLocation[index] + rect.origin + Pair(border, 0),
500+
location: plan.y2MarkersTextLocation[index] + rect.origin,
485501
textSize: markerTextSize,
486502
color: plotBorder.color,
487503
strokeWidth: 0.7,

Sources/SwiftPlot/LayoutComponents/LayoutComponent.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,17 @@ extension LayoutComponent {
5252
return _Padded(base: self, padding: padding)
5353
}
5454
}
55+
56+
struct FixedSpace: LayoutComponent {
57+
var size: Float
58+
func measure(edge: RectEdge, _ renderer: Renderer) -> Size {
59+
if edge.isHorizontal {
60+
// Only the height is important, so we can say width = 1.
61+
return Size(width: 1, height: size)
62+
} else {
63+
return Size(width: size, height: 1)
64+
}
65+
}
66+
func draw(_ rect: Rect, measuredSize: Size, edge: RectEdge, renderer: Renderer) {
67+
}
68+
}

0 commit comments

Comments
 (0)