Skip to content

Commit

Permalink
improvements in barRect height calculation (#3650)
Browse files Browse the repository at this point in the history 
* fixed barRectCaculation

* fixed offset calculation

* Fix the  mess caused by the setting the min&min value of the y-axis by error :
Just simply swap their values

* Fix the mess caused by the setting the min&min value of the y-axis by error

* Revert "Fix the mess caused by the setting the min&min value of the y-axis by error"

This reverts commit 526a73a.

* Fix the mess caused by the setting the min&min value of the y-axis by error

* update offset calculation

* update code style

* update code style

* update offset calculation

* keep barRect calculation untouched

try to simply the calculation. keep barRect calculation untouched

* After the correction of min and max ,  they should be assigned back to  _axisMinimum and _axisMaximum

* add demo for bar chart unit test

* update unit test

* revert last commit

* update unit test

* make sure max is greater than min &
turnoff record mode for barchartest

* add new UT and fix some issues.

1. add more bar chart UT
2. code style fix
3. manually add chart.notifyDataSetChanged() - some old UT and new ones forget to call this method, leading the test images to be wrong.

Notice:
some test images diff shows slight pixel shift, not sure why, but both old and new image seems drawing correctly

* update tvOS images

* update tolerance to 1% & more swift-y

* update tvOS images, removing "Description Label", (not rendered anymore)

changing tolerance will trigger "Description Label" detection

* update iOS test images, "Description Label" no longer rendered.

* fixed offset calculation in some cases.
moved those codes into the for loop. because the offset of each bar may be different.
(detail in comments)

* Update Source/Charts/Renderers/BarChartRenderer.swift

Co-Authored-By: potato04 <shiww@outlook.com>
  • Loading branch information
@potato04 @jjatie
potato04 authored and jjatie committed Nov 1, 2018
1 parent 69e433a commit c01a765
Show file tree
Hide file tree
Showing 172 changed files with 371 additions and 45 deletions.
17 changes: 17 additions & 0 deletions Source/Charts/Components/YAxis.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,23 @@ open class YAxis: AxisBase
var min = _customAxisMin ? _axisMinimum : dataMin
var max = _customAxisMax ? _axisMaximum : dataMax

// Make sure max is greater than min
// Discussion: https://github.com/danielgindi/Charts/pull/3650#discussion_r221409991
if min > max
{
switch(_customAxisMax, _customAxisMin)
{
case(true, true):
(min, max) = (max, min)
case(true, false):
min = max < 0 ? max * 1.5 : max * 0.5
case(false, true):
max = min < 0 ? min * 0.5 : min * 1.5
case(false, false):
break
}
}

// temporary range (before calculations)
let range = abs(max - min)

Expand Down
100 changes: 79 additions & 21 deletions Source/Charts/Renderers/BarChartRenderer.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ open class BarChartRenderer: BarLineScatterCandleBubbleRenderer
var barRect = CGRect()
var x: Double
var y: Double


for i in stride(from: 0, to: min(Int(ceil(Double(dataSet.entryCount) * animator.phaseX)), dataSet.entryCount), by: 1)
{
Expand All @@ -128,9 +129,84 @@ open class BarChartRenderer: BarLineScatterCandleBubbleRenderer
var bottom = isInverted
? (y >= 0.0 ? CGFloat(y) : 0)
: (y <= 0.0 ? CGFloat(y) : 0)

/* When drawing each bar, the renderer actually draws each bar from 0 to the required value.
* This drawn bar is then clipped to the visible chart rect in BarLineChartViewBase's draw(rect:) using clipDataToContent.
* While this works fine when calculating the bar rects for drawing, it causes the accessibilityFrames to be oversized in some cases.
* This offset attempts to undo that unnecessary drawing when calculating barRects
*
* +---------------------------------------------------------------+---------------------------------------------------------------+
* | Situation 1: (!inverted && y >= 0) | Situation 3: (inverted && y >= 0) |
* | | |
* | y -> +--+ <- top | 0 -> ---+--+---+--+------ <- top |
* | |//| } topOffset = y - max | | | |//| } topOffset = min |
* | max -> +---------+--+----+ <- top - topOffset | min -> +--+--+---+--+----+ <- top + topOffset |
* | | +--+ |//| | | | | | |//| | |
* | | | | |//| | | | +--+ |//| | |
* | | | | |//| | | | |//| | |
* | min -> +--+--+---+--+----+ <- bottom + bottomOffset | max -> +---------+--+----+ <- bottom - bottomOffset |
* | | | |//| } bottomOffset = min | |//| } bottomOffset = y - max |
* | 0 -> ---+--+---+--+----- <- bottom | y -> +--+ <- bottom |
* | | |
* +---------------------------------------------------------------+---------------------------------------------------------------+
* | Situation 2: (!inverted && y < 0) | Situation 4: (inverted && y < 0) |
* | | |
* | 0 -> ---+--+---+--+----- <- top | y -> +--+ <- top |
* | | | |//| } topOffset = -max | |//| } topOffset = min - y |
* | max -> +--+--+---+--+----+ <- top - topOffset | min -> +---------+--+----+ <- top + topOffset |
* | | | | |//| | | | +--+ |//| | |
* | | +--+ |//| | | | | | |//| | |
* | | |//| | | | | | |//| | |
* | min -> +---------+--+----+ <- bottom + bottomOffset | max -> +--+--+---+--+----+ <- bottom - bottomOffset |
* | |//| } bottomOffset = min - y | | | |//| } bottomOffset = -max |
* | y -> +--+ <- bottom | 0 -> ---+--+---+--+------- <- bottom |
* | | |
* +---------------------------------------------------------------+---------------------------------------------------------------+
*/
var topOffset: CGFloat = 0.0
var bottomOffset: CGFloat = 0.0
if let offsetView = dataProvider as? BarChartView
{
let offsetAxis = offsetView.getAxis(dataSet.axisDependency)
if y >= 0
{
// situation 1
if offsetAxis.axisMaximum < y
{
topOffset = CGFloat(y - offsetAxis.axisMaximum)
}
if offsetAxis.axisMinimum > 0
{
bottomOffset = CGFloat(offsetAxis.axisMinimum)
}
}
else // y < 0
{
//situation 2
if offsetAxis.axisMaximum < 0
{
topOffset = CGFloat(offsetAxis.axisMaximum * -1)
}
if offsetAxis.axisMinimum > y
{
bottomOffset = CGFloat(offsetAxis.axisMinimum - y)
}
}
if isInverted
{
// situation 3 and 4
// exchange topOffset/bottomOffset based on 1 and 2
// see diagram above
(topOffset, bottomOffset) = (bottomOffset, topOffset)
}
}
//apply offset
top = isInverted ? top + topOffset : top - topOffset
bottom = isInverted ? bottom - bottomOffset : bottom + bottomOffset

// multiply the height of the rect with the phase
if top > 0
// explicitly add 0 + topOffset to indicate this is changed after adding accessibility support (#3650, #3520)
if top > 0 + topOffset
{
top *= CGFloat(phaseY)
}
Expand All @@ -139,28 +215,10 @@ open class BarChartRenderer: BarLineScatterCandleBubbleRenderer
bottom *= CGFloat(phaseY)
}

// When drawing with an auto calculated y-axis minimum, the renderer actually draws each bar from 0
// to the required value. This drawn bar is then clipped to the visible chart rect in BarLineChartViewBase's draw(rect:) using clipDataToContent.
// While this works fine when calculating the bar rects for drawing, it causes the accessibilityFrames to be oversized in some cases.
// This offset attempts to undo that unnecessary drawing when calculating barRects, particularly when not using custom axis minima.
// This allows the minimum to still be visually non zero, but the rects are only drawn where necessary.
// This offset calculation also avoids cases where there are positive/negative values mixed, since those won't need this offset.
var offset: CGFloat = 0.0
if let offsetView = dataProvider as? BarChartView {

let offsetAxis = offsetView.leftAxis.isEnabled ? offsetView.leftAxis : offsetView.rightAxis

if barData.yMin.sign != barData.yMax.sign { offset = 0.0 }
else if !offsetAxis._customAxisMin {
offset = CGFloat(offsetAxis.axisMinimum)
}
}

barRect.origin.x = left
barRect.size.width = right - left
barRect.origin.y = top
barRect.size.height = bottom == top ? 0 : bottom - top + offset

barRect.size.width = right - left
barRect.size.height = bottom - top
buffer.rects[bufferIndex] = barRect
bufferIndex += 1
}
Expand Down
Loading

0 comments on commit c01a765

Please sign in to comment.