Skip to content

Commit

Permalink
Simplify code
Browse files Browse the repository at this point in the history
  • Loading branch information
@danielgindi
danielgindi committed Jan 23, 2020
1 parent 49ed013 commit ae6fd3e
Showing 1 changed file with 103 additions and 96 deletions.
199 changes: 103 additions & 96 deletions Source/Charts/Renderers/PieChartRenderer.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,122 +163,129 @@ open class PieChartRenderer: DataRenderer
var innerRadius = userInnerRadius

guard let e = dataSet.entryForIndex(j) else { continue }

defer
{
// From here on, even when skipping (i.e for highlight),
// increase the angle
angle += sliceAngle * CGFloat(phaseX)
}

// draw only if the value is greater than zero
if (abs(e.y) > Double.ulpOfOne)
if abs(e.y) < Double.ulpOfOne { continue }

This comment has been minimized.

Sign in to view

Copy link
@liuxuan30

liuxuan30 Aug 17, 2020

Member

@danielgindi @jjatie this is somehow inconsistent with old code.
do we want to draw when abs(e.y) <= or < Double.ulpOfOne?

This comment has been minimized.

Sign in to view

Copy link
@jjatie

jjatie Aug 17, 2020

Collaborator

This should have been

guard abs(e.y) > .uplOfOne else { continue }

This comment has been minimized.

Sign in to view

Copy link
@danielgindi

danielgindi Aug 17, 2020

Author Collaborator

@danielgindi @jjatie this is somehow inconsistent with old code.
do we want to draw when abs(e.y) <= or < Double.ulpOfOne?

The suggested change will not skip if it's equal to zero...

This comment has been minimized.

Sign in to view

Copy link
@liuxuan30

liuxuan30 Aug 18, 2020 •
edited
Loading

Member

@danielgindi if theoretically abs(e.y) == .uplOfOne, shall we draw or continue?
I'm keeping if abs(e.y) < Double.ulpOfOne { continue } now on my side.

This comment has been minimized.

Sign in to view

Copy link
@danielgindi

danielgindi Aug 18, 2020

Author Collaborator

@danielgindi if theoretically abs(e.y) == .uplOfOne, shall we draw or continue?
I'm keeping if abs(e.y) < Double.ulpOfOne { continue } now on my side.

Should continue. The suggested behavior is incorrect. @jjatie's suggestion is correct

// Skip if highlighted
if dataSet.isHighlightEnabled && chart.needsHighlight(index: j)

This comment has been minimized.

Sign in to view

Copy link
@jjatie

jjatie Aug 17, 2020

Collaborator

This should be turned into a guard statement too

This comment has been minimized.

Sign in to view

Copy link
@liuxuan30

liuxuan30 Aug 18, 2020

Member

I have finished merging this file, so we have to refactor again after I resolve all.

A quick update is I already finished most of them, only axis renderers files left unresolved. I will try to finish this week so we can move forward.
{
if !dataSet.isHighlightEnabled || !chart.needsHighlight(index: j)
{
let accountForSliceSpacing = sliceSpace > 0.0 && sliceAngle <= 180.0
continue
}

let accountForSliceSpacing = sliceSpace > 0.0 && sliceAngle <= 180.0

context.setFillColor(dataSet.color(atIndex: j).cgColor)
context.setFillColor(dataSet.color(atIndex: j).cgColor)

let sliceSpaceAngleOuter = visibleAngleCount == 1 ?
0.0 :
sliceSpace / radius.DEG2RAD
let startAngleOuter = rotationAngle + (angle + sliceSpaceAngleOuter / 2.0) * CGFloat(phaseY)
var sweepAngleOuter = (sliceAngle - sliceSpaceAngleOuter) * CGFloat(phaseY)
if sweepAngleOuter < 0.0
{
sweepAngleOuter = 0.0
}
let sliceSpaceAngleOuter = visibleAngleCount == 1 ?
0.0 :
sliceSpace / radius.DEG2RAD
let startAngleOuter = rotationAngle + (angle + sliceSpaceAngleOuter / 2.0) * CGFloat(phaseY)
var sweepAngleOuter = (sliceAngle - sliceSpaceAngleOuter) * CGFloat(phaseY)

This comment has been minimized.

Sign in to view

Copy link
@jjatie

jjatie Aug 17, 2020

Collaborator

This should just be:

let sweepAngleOuter = max(0, (sliceAngle - sliceSpaceAngleOuter) * CGFloat(phaseY))
if sweepAngleOuter < 0.0
{
sweepAngleOuter = 0.0
}

let arcStartPointX = center.x + radius * cos(startAngleOuter.DEG2RAD)
let arcStartPointY = center.y + radius * sin(startAngleOuter.DEG2RAD)
let arcStartPointX = center.x + radius * cos(startAngleOuter.DEG2RAD)
let arcStartPointY = center.y + radius * sin(startAngleOuter.DEG2RAD)

let path = CGMutablePath()
let path = CGMutablePath()

path.move(to: CGPoint(x: arcStartPointX,
y: arcStartPointY))
path.move(to: CGPoint(x: arcStartPointX,
y: arcStartPointY))

path.addRelativeArc(center: center, radius: radius, startAngle: startAngleOuter.DEG2RAD, delta: sweepAngleOuter.DEG2RAD)
path.addRelativeArc(center: center, radius: radius, startAngle: startAngleOuter.DEG2RAD, delta: sweepAngleOuter.DEG2RAD)

if drawInnerArc &&
(innerRadius > 0.0 || accountForSliceSpacing)
if drawInnerArc &&
(innerRadius > 0.0 || accountForSliceSpacing)
{
if accountForSliceSpacing
{
var minSpacedRadius = calculateMinimumRadiusForSpacedSlice(
center: center,
radius: radius,
angle: sliceAngle * CGFloat(phaseY),
arcStartPointX: arcStartPointX,
arcStartPointY: arcStartPointY,
startAngle: startAngleOuter,
sweepAngle: sweepAngleOuter)
if minSpacedRadius < 0.0
{
if accountForSliceSpacing
{
var minSpacedRadius = calculateMinimumRadiusForSpacedSlice(
center: center,
radius: radius,
angle: sliceAngle * CGFloat(phaseY),
arcStartPointX: arcStartPointX,
arcStartPointY: arcStartPointY,
startAngle: startAngleOuter,
sweepAngle: sweepAngleOuter)
if minSpacedRadius < 0.0
{
minSpacedRadius = -minSpacedRadius
}
innerRadius = min(max(innerRadius, minSpacedRadius), radius)
}

let sliceSpaceAngleInner = visibleAngleCount == 1 || innerRadius == 0.0 ?
0.0 :
sliceSpace / innerRadius.DEG2RAD
let startAngleInner = rotationAngle + (angle + sliceSpaceAngleInner / 2.0) * CGFloat(phaseY)
var sweepAngleInner = (sliceAngle - sliceSpaceAngleInner) * CGFloat(phaseY)
if sweepAngleInner < 0.0
{
sweepAngleInner = 0.0
}
let endAngleInner = startAngleInner + sweepAngleInner
minSpacedRadius = -minSpacedRadius
}
innerRadius = min(max(innerRadius, minSpacedRadius), radius)
}

path.addLine(
to: CGPoint(
x: center.x + innerRadius * cos(endAngleInner.DEG2RAD),
y: center.y + innerRadius * sin(endAngleInner.DEG2RAD)))
let sliceSpaceAngleInner = visibleAngleCount == 1 || innerRadius == 0.0 ?
0.0 :
sliceSpace / innerRadius.DEG2RAD
let startAngleInner = rotationAngle + (angle + sliceSpaceAngleInner / 2.0) * CGFloat(phaseY)
var sweepAngleInner = (sliceAngle - sliceSpaceAngleInner) * CGFloat(phaseY)

This comment has been minimized.

Sign in to view

Copy link
@jjatie

jjatie Aug 17, 2020

Collaborator

let sweepAngleInner = max(0, (sliceAngle - sliceSpaceAngleInner) * CGFloat(phaseY))
if sweepAngleInner < 0.0
{
sweepAngleInner = 0.0
}
let endAngleInner = startAngleInner + sweepAngleInner

path.addRelativeArc(center: center, radius: innerRadius, startAngle: endAngleInner.DEG2RAD, delta: -sweepAngleInner.DEG2RAD)
}
else
{
if accountForSliceSpacing
{
let angleMiddle = startAngleOuter + sweepAngleOuter / 2.0

let sliceSpaceOffset =
calculateMinimumRadiusForSpacedSlice(
center: center,
radius: radius,
angle: sliceAngle * CGFloat(phaseY),
arcStartPointX: arcStartPointX,
arcStartPointY: arcStartPointY,
startAngle: startAngleOuter,
sweepAngle: sweepAngleOuter)

let arcEndPointX = center.x + sliceSpaceOffset * cos(angleMiddle.DEG2RAD)
let arcEndPointY = center.y + sliceSpaceOffset * sin(angleMiddle.DEG2RAD)

path.addLine(
to: CGPoint(
x: arcEndPointX,
y: arcEndPointY))
}
else
{
path.addLine(to: center)
}
}
path.addLine(
to: CGPoint(
x: center.x + innerRadius * cos(endAngleInner.DEG2RAD),
y: center.y + innerRadius * sin(endAngleInner.DEG2RAD)))

path.closeSubpath()
path.addRelativeArc(center: center, radius: innerRadius, startAngle: endAngleInner.DEG2RAD, delta: -sweepAngleInner.DEG2RAD)
}
else
{
if accountForSliceSpacing
{
let angleMiddle = startAngleOuter + sweepAngleOuter / 2.0

context.beginPath()
context.addPath(path)
context.fillPath(using: .evenOdd)
let sliceSpaceOffset =
calculateMinimumRadiusForSpacedSlice(
center: center,
radius: radius,
angle: sliceAngle * CGFloat(phaseY),
arcStartPointX: arcStartPointX,
arcStartPointY: arcStartPointY,
startAngle: startAngleOuter,
sweepAngle: sweepAngleOuter)

let axElement = createAccessibleElement(withIndex: j,
container: chart,
dataSet: dataSet)
{ (element) in
element.accessibilityFrame = path.boundingBoxOfPath
}
let arcEndPointX = center.x + sliceSpaceOffset * cos(angleMiddle.DEG2RAD)
let arcEndPointY = center.y + sliceSpaceOffset * sin(angleMiddle.DEG2RAD)

accessibleChartElements.append(axElement)
path.addLine(
to: CGPoint(
x: arcEndPointX,
y: arcEndPointY))
}
else
{
path.addLine(to: center)
}
}

path.closeSubpath()

context.beginPath()
context.addPath(path)
context.fillPath(using: .evenOdd)

let axElement = createAccessibleElement(withIndex: j,
container: chart,
dataSet: dataSet)
{ (element) in
element.accessibilityFrame = path.boundingBoxOfPath
}

angle += sliceAngle * CGFloat(phaseX)
accessibleChartElements.append(axElement)
}

// Post this notification to let VoiceOver account for the redrawn frames
Expand Down

0 comments on commit ae6fd3e

Please sign in to comment.