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.

Sign up for GitHub

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

Minor changes to logic in ViewPortJob subclasses. #2992

Merged
merged 2 commits into from
Nov 23, 2017
Merged

Minor changes to logic in ViewPortJob subclasses. #2992

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb44fc5
Minor changes to logic in `ViewPortJob` subclasses.
jjatie Nov 13, 2017
d1c6d37
Removed TODOs
jjatie Nov 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions Source/Charts/Charts/BarLineChartViewBase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
}
else
{
let _ = viewPortHandler.refresh(newMatrix: viewPortHandler.touchMatrix, chart: self, invalidate: true)
viewPortHandler.refresh(newMatrix: viewPortHandler.touchMatrix, chart: self, invalidate: true)
}
}

Expand Down Expand Up @@ -651,7 +651,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD

matrix = _viewPortHandler.touchMatrix.concatenating(matrix)

let _ = _viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: true)
_viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: true)

if delegate !== nil
{
Expand Down Expand Up @@ -984,7 +984,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
let center = _viewPortHandler.contentCenter

let matrix = _viewPortHandler.zoomIn(x: center.x, y: -center.y)
let _ = _viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)
_viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)

// Range might have changed, which means that Y-axis labels could have changed in size, affecting Y-axis size. So we need to recalculate offsets.
calculateOffsets()
Expand All @@ -997,7 +997,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
let center = _viewPortHandler.contentCenter

let matrix = _viewPortHandler.zoomOut(x: center.x, y: -center.y)
let _ = _viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)
_viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)

// Range might have changed, which means that Y-axis labels could have changed in size, affecting Y-axis size. So we need to recalculate offsets.
calculateOffsets()
Expand All @@ -1008,7 +1008,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
@objc open func resetZoom()
{
let matrix = _viewPortHandler.resetZoom()
let _ = _viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)
_viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)

// Range might have changed, which means that Y-axis labels could have changed in size, affecting Y-axis size. So we need to recalculate offsets.
calculateOffsets()
Expand All @@ -1029,7 +1029,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
y: CGFloat)
{
let matrix = _viewPortHandler.zoom(scaleX: scaleX, scaleY: scaleY, x: x, y: -y)
let _ = _viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)
_viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)

// Range might have changed, which means that Y-axis labels could have changed in size, affecting Y-axis size. So we need to recalculate offsets.
calculateOffsets()
Expand Down Expand Up @@ -1080,7 +1080,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
scaleY: scaleY,
x: center.x,
y: -center.y)
let _ = viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)
viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)
}

/// Zooms by the specified scale factor to the specified values on the specified axis.
Expand Down Expand Up @@ -1170,7 +1170,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
@objc open func fitScreen()
{
let matrix = _viewPortHandler.fitScreen()
let _ = _viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)
_viewPortHandler.refresh(newMatrix: matrix, chart: self, invalidate: false)

calculateOffsets()
setNeedsDisplay()
Expand Down
29 changes: 13 additions & 16 deletions Source/Charts/Jobs/AnimatedViewPortJob.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,20 @@ open class AnimatedViewPortJob: ViewPortJob

@objc open func stop(finish: Bool)
{
if _displayLink != nil
guard _displayLink != nil else { return }
_displayLink.remove(from: RunLoop.main, forMode: RunLoopMode.commonModes)
_displayLink = nil

if finish
{
_displayLink.remove(from: RunLoop.main, forMode: RunLoopMode.commonModes)
_displayLink = nil

if finish
if phase != 1.0
{
if phase != 1.0
{
phase = 1.0
phase = 1.0

animationUpdate()
}

animationEnd()
phase = 1.0

animationUpdate()
}

animationEnd()
}
}

Expand Down Expand Up @@ -132,11 +129,11 @@ open class AnimatedViewPortJob: ViewPortJob

@objc internal func animationUpdate()
{
// Override this
fatalError("`animationUpdate()` must be overriden by subclasses")
}

@objc internal func animationEnd()
{
// Override this
fatalError("`animationEnd()` must be overriden by subclasses")
}
}
4 changes: 2 additions & 2 deletions Source/Charts/Jobs/AnimatedZoomViewJob.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ open class AnimatedZoomViewJob: AnimatedViewPortJob
let scaleY = yOrigin + (self.scaleY - yOrigin) * phase

var matrix = viewPortHandler.setZoom(scaleX: scaleX, scaleY: scaleY)
let _ = viewPortHandler.refresh(newMatrix: matrix, chart: view, invalidate: false)
viewPortHandler.refresh(newMatrix: matrix, chart: view, invalidate: false)

let valsInView = CGFloat(yAxis?.axisRange ?? 0.0) / viewPortHandler.scaleY
let xsInView = CGFloat(xAxisRange) / viewPortHandler.scaleX
Expand All @@ -85,7 +85,7 @@ open class AnimatedZoomViewJob: AnimatedViewPortJob
transformer.pointValueToPixel(&pt)

matrix = viewPortHandler.translate(pt: pt)
let _ = viewPortHandler.refresh(newMatrix: matrix, chart: view, invalidate: true)
viewPortHandler.refresh(newMatrix: matrix, chart: view, invalidate: true)
}

internal override func animationEnd()
Expand Down
17 changes: 1 addition & 16 deletions Source/Charts/Jobs/MoveViewJob.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,7 @@ import CoreGraphics

@objc(MoveChartViewJob)
open class MoveViewJob: ViewPortJob
{
public override init(
viewPortHandler: ViewPortHandler,
xValue: Double,
yValue: Double,
transformer: Transformer,
view: ChartViewBase)
{
super.init(
viewPortHandler: viewPortHandler,
xValue: xValue,
yValue: yValue,
transformer: transformer,
view: view)
}

{
open override func doJob()
{
guard
Expand Down
8 changes: 4 additions & 4 deletions Source/Charts/Jobs/ViewPortJob.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ import CoreGraphics
open class ViewPortJob: NSObject
{
@objc internal var point: CGPoint = CGPoint()
@objc internal weak var viewPortHandler: ViewPortHandler?
@objc internal weak var viewPortHandler: ViewPortHandler? // TODO: Can this be unowned?
Copy link
Member

Choose a reason for hiding this comment

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

before merge, do we need to finish TODO? I am willing to check if we can use unowned when I got time, but seems weak is totally fine here. What's your concern using weak?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

weak does work here, but I think what is actually intended is unowned. In terms of function, nothing would change, but it does clean up a lot of guard statements if we can use unowned I feel like that is a separate PR and one that requires a bit more digging. I'm not sure what the policy is on TODOs

Copy link
Member

Choose a reason for hiding this comment

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

I don't know the policy neither. but if it is another PR, could you remove the comment? I will merge later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@objc internal var xValue: Double = 0.0
@objc internal var yValue: Double = 0.0
@objc internal weak var transformer: Transformer?
@objc internal weak var view: ChartViewBase?
@objc internal weak var transformer: Transformer? // TODO: Can this be unowned?
@objc internal weak var view: ChartViewBase? // TODO: Can this be unowned?

@objc public init(
viewPortHandler: ViewPortHandler,
Expand All @@ -41,6 +41,6 @@ open class ViewPortJob: NSObject

@objc open func doJob()
{
// Override this
fatalError("`doJob()` must be overridden by subclasses")
}
}
4 changes: 2 additions & 2 deletions Source/Charts/Jobs/ZoomViewJob.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ open class ZoomViewJob: ViewPortJob
else { return }

var matrix = viewPortHandler.setZoom(scaleX: scaleX, scaleY: scaleY)
let _ = viewPortHandler.refresh(newMatrix: matrix, chart: view, invalidate: false)
viewPortHandler.refresh(newMatrix: matrix, chart: view, invalidate: false)

let yValsInView = (view as! BarLineChartViewBase).getAxis(axisDependency).axisRange / Double(viewPortHandler.scaleY)
let xValsInView = (view as! BarLineChartViewBase).xAxis.axisRange / Double(viewPortHandler.scaleX)
Expand All @@ -67,7 +67,7 @@ open class ZoomViewJob: ViewPortJob
transformer.pointValueToPixel(&pt)

matrix = viewPortHandler.translate(pt: pt)
let _ = viewPortHandler.refresh(newMatrix: matrix, chart: view, invalidate: false)
viewPortHandler.refresh(newMatrix: matrix, chart: view, invalidate: false)

(view as! BarLineChartViewBase).calculateOffsets()
view.setNeedsDisplay()
Expand Down
5 changes: 2 additions & 3 deletions Source/Charts/Utils/ViewPortHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,11 @@ open class ViewPortHandler: NSObject
let translateY = pt.y - offsetTop

let matrix = _touchMatrix.concatenating(CGAffineTransform(translationX: -translateX, y: -translateY))

let _ = refresh(newMatrix: matrix, chart: chart, invalidate: true)
refresh(newMatrix: matrix, chart: chart, invalidate: true)
}

/// call this method to refresh the graph with a given matrix
@objc open func refresh(newMatrix: CGAffineTransform, chart: ChartViewBase, invalidate: Bool) -> CGAffineTransform
@objc @discardableResult open func refresh(newMatrix: CGAffineTransform, chart: ChartViewBase, invalidate: Bool) -> CGAffineTransform
{
_touchMatrix = newMatrix

Expand Down