Update the renderers with dedicated atomic rendering methods in order to make rendering overrides easier to implement. #4297

4np · 2020-02-25T15:15:06Z

Context

Often when implementing charting features, there is a requirement to implement certain UI changes. The more simple changes can often be accomplished by subclassing a particular renderer and overriding some of its methods, but more elaborate changes can be hard to accomplish.

While many methods are marked with open visibility and can theoretically be overloaded, in practice this is not as straightforward as most of those methods either call other methods or use variables that have private or internal visibility.

Consider the use case where you want to fill a bar chart with a pattern instead of a color. In order to do so, you would need to subclass the BarChartRenderer and override drawDataSet(context:, dataSet:, index:) (or drawHighlighted(context:, indices:) to update the highlight rendering).

There are a couple of challenges with this approach:

drawDataSet(context:, dataSet:, index:) performs quite some calculations you really do not want to replicate and want to keep 'in the charting framework' (separation of concerns).
these methods call a number of other methods that have internal or private visibility, which you cannot call from your subclassed renderer (for example prepareBuffer(dataSet:, index:) or BarLineScatterCandleBubbleRenderer.isInBoundsX(entry:, dataSet:)), which in turn requires you to duplicate those implementation as well.
similarly these methods use a number of variables that have internal or private visibility (e.g. _barShadowRectBuffer, _buffers, etcetera), again requiring unnecessary code duplications.
you want to focus on the rendering override, instead of re-implementing a lot of code in your subclassed renderer that, in essence, does not really matter for what you aim to accomplish.
because of all the code duplication from the base class, up-taking a new Charts version is cumbersome and requires diffing duplicate logic for changes.

Solution

This Pull Request lifts the rendering logic out of the methods into their own dedicated methods with open visibility. This way it becomes really easy to create a custom renderer that implements custom rendering behavior, without the need of having to duplicate complex logic or re-implement private / internal methods and/or variables. It allows you to focus on what matters, while keeping your subclassed renderer focussed and simple.

Example: Bar Chart Pattern Fill.

This is an example of creating a custom bar chart renderer that will fill bars with a circular pattern:

/// Custom bar chart renderer that fills bars with a circle based pattern.
class PatternFilledBarChartRenderer: BarChartRenderer {
    override func renderFill(with color: UIColor, for rect: CGRect, in context: CGContext,  dataSet: IBarChartDataSet) {
        let patternSize = CGSize(width: 10, height: 10)
        let patternCallback: CGPatternDrawPatternCallback = { (pointer, context) -> Void in
            context.addArc(center: CGPoint(x: 5, y: 5), radius: 5, startAngle: 0, endAngle: CGFloat(2.0 * .pi), clockwise: true)
            context.fillPath()
        }
        var callbacks = CGPatternCallbacks(version: 0, drawPattern: patternCallback, releaseInfo: nil)
        let pattern = CGPattern(info: nil,
                                bounds: CGRect(origin: CGPoint(x: 0, y: 0), size: patternSize),
                                matrix: .identity,
                                xStep: patternSize.width,
                                yStep: patternSize.height,
                                tiling: .constantSpacing,
                                isColored: false,
                                callbacks: &callbacks)
        let patternColorSpace = CGColorSpace(patternBaseSpace: CGColorSpaceCreateDeviceRGB())
        let colorComponents = UnsafeMutablePointer<CGFloat>.allocate(capacity: 4)
        color.getRed(&colorComponents[0], green: &colorComponents[1], blue: &colorComponents[2], alpha: &colorComponents[3])

        defer {
            colorComponents.deinitialize(count: 4)
            colorComponents.deallocate()
        }

        context.saveGState()
        context.setFillColorSpace(patternColorSpace!) //swiftlint:disable:this force_unwrapping
        context.setFillPattern(pattern!, colorComponents: colorComponents) //swiftlint:disable:this force_unwrapping
        context.fill(rect)
        context.restoreGState()
    }
}

Implementing this rendering override can now be done by overriding a single method in a straightforward manner, whereas before you needed to copy over pretty much the complete implementation of BarChartRenderer.swift (and more) while making small alterations to accomplish the same.

Screenshot:

Lastly

It would be greatly appreciated if this could make it into a newly drafted release in the not so distant future 🙏🏻 😉

… to make rendering overrides easier to implement.

codecov-io · 2020-02-25T16:07:34Z

Codecov Report

Merging #4297 into master will increase coverage by 0.02%.
The diff coverage is 48.61%.

@@            Coverage Diff             @@
##           master    #4297      +/-   ##
==========================================
+ Coverage    41.8%   41.83%   +0.02%     
==========================================
  Files         124      124              
  Lines       14117    14173      +56     
==========================================
+ Hits         5902     5929      +27     
- Misses       8215     8244      +29

Impacted Files	Coverage Δ
Source/Charts/Renderers/LineRadarRenderer.swift	`8% <ø> (+0.3%)`	⬆️
Source/Charts/Renderers/BubbleChartRenderer.swift	`0% <0%> (ø)`	⬆️
Source/Charts/Renderers/PieChartRenderer.swift	`69.59% <100%> (+0.3%)`	⬆️
Source/Charts/Renderers/LegendRenderer.swift	`60.04% <25%> (-1.23%)`	⬇️
Source/Charts/Renderers/BarChartRenderer.swift	`74.13% <48.71%> (-0.49%)`	⬇️
Source/Charts/Renderers/LineChartRenderer.swift	`66.82% <78.26%> (+0.71%)`	⬆️
...s/Data/Implementations/Standard/ChartDataSet.swift	`40.14% <0%> (+0.35%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 847e822...d3e82c3. Read the comment docs.

jjatie · 2020-02-26T14:38:42Z

I'm all for this, but there has been resistance to adding more function calls in the past. I believe the justification for this hasn't been valid since about Swift 3, but as a result @danielgindi will have to decide.

4np · 2020-02-26T15:00:40Z

Thank you for your feedback and thoughts @jjatie 👍

4np · 2020-12-15T10:29:36Z

@danielgindi @liuxuan30 @jjatie : do you have any thoughts about adding these override points for rendering logic?

# Conflicts: # Source/Charts/Renderers/BarChartRenderer.swift # Source/Charts/Renderers/LegendRenderer.swift # Source/Charts/Renderers/LineChartRenderer.swift # Source/Charts/Renderers/PieChartRenderer.swift

4np · 2020-12-15T12:30:24Z

@danielgindi @liuxuan30 : I updated the PR to the latest revision from master.

Perform these three steps to the demo application in order to reproduce the example in my screenshots below:

add the code below to the bottom of BarChartViewController.swift
change @IBOutlet var chartView: BarChartView! to @IBOutlet var chartView: PatternFilledBarChartView!
change the view's class from BarChartView to PatternFilledBarChartView in BarChartViewController.xib

/// Custom bar chart view that fills bars with a circular pattern.
class PatternFilledBarChartView: BarChartView {
    override init(frame: CGRect) {
        super.init(frame: frame)
        updateRenderer()
    }
    
    required init?(coder aDecoder: NSCoder) {
        super.init(coder: aDecoder)
        updateRenderer()
    }
    
    private func updateRenderer() {
        let chartAnimator = renderer?.animator ?? Animator()
        renderer = PatternFilledBarChartRenderer(dataProvider: self,
                                                 animator: chartAnimator,
                                                 viewPortHandler: viewPortHandler)
    }
}

/// Custom bar chart renderer that fills bars with a circular pattern.
class PatternFilledBarChartRenderer: BarChartRenderer {
    override func renderFill(with color: NSUIColor, for rect: CGRect, in context: CGContext, dataSet: BarChartDataSetProtocol, entry: BarChartDataEntry?) {
        let patternSize = CGSize(width: 10, height: 10)
        let patternCallback: CGPatternDrawPatternCallback = { (pointer, context) -> Void in
            context.addArc(center: CGPoint(x: 5, y: 5), radius: 5, startAngle: 0, endAngle: CGFloat(2.0 * .pi), clockwise: true)
            context.fillPath()
        }
        var callbacks = CGPatternCallbacks(version: 0, drawPattern: patternCallback, releaseInfo: nil)
        let pattern = CGPattern(info: nil,
                                bounds: CGRect(origin: CGPoint(x: 0, y: 0), size: patternSize),
                                matrix: .identity,
                                xStep: patternSize.width,
                                yStep: patternSize.height,
                                tiling: .constantSpacing,
                                isColored: false,
                                callbacks: &callbacks)
        let patternColorSpace = CGColorSpace(patternBaseSpace: CGColorSpaceCreateDeviceRGB())
        let colorComponents = UnsafeMutablePointer<CGFloat>.allocate(capacity: 4)
        color.getRed(&colorComponents[0], green: &colorComponents[1], blue: &colorComponents[2], alpha: &colorComponents[3])

        defer {
            colorComponents.deinitialize(count: 4)
            colorComponents.deallocate()
        }

        context.saveGState()
        context.setFillColorSpace(patternColorSpace!) //swiftlint:disable:this force_unwrapping
        context.setFillPattern(pattern!, colorComponents: colorComponents) //swiftlint:disable:this force_unwrapping
        context.fill(rect)
        context.restoreGState()
    }
}

Screenshots

4np · 2020-12-15T13:10:44Z

~~Added a fix for #3701 that will take the axis font into account when determining the size of the labels.~~

Reverted in 7e1f6a0 as it caused the tests to fail and it deviated from the focussed PR.

4np · 2020-12-15T13:28:23Z

@danielgindi @liuxuan30 @jjatie : just a quick look at open issues and PRs (see for example these ☝🏻), it looks like this PR will solve quite a number of them. Rather than extending the charting library with specific logic for very specific use cases, this PR allows people to create their custom renderers and override rendering to fit their specific needs.

This reverts commit a517e14.

…Border method.

bivant · 2020-12-15T19:58:27Z

The only issue I see here is a performance question. Does it change with this implementation and how much if yes. Other than that this is a perfect solution in many-many cases. I would say it should be done for all renderers (or should have such option) because recently had to modify xAxis one to match designer idea of the beauty :)
#3754

4np · 2020-12-16T07:47:12Z

@bivant : It shouldn't really, as it is only introducing dedicated open overridable render methods rather than doing the rendering inline nested inside private methods. It does not add additional complexity or computational overhead. It depends, of course, on what you do yourself inside a custom renderer, but it should make customizations much easier to accomplish. The example I gave above with the circular fill pattern is still very speedy and feels no different from the default implementation.

4np · 2020-12-28T10:13:25Z

@danielgindi @liuxuan30 @jjatie: anybody?

4np · 2021-01-29T07:40:00Z

@danielgindi @liuxuan30 @jjatie: bump...

scinfu · 2021-02-12T15:40:17Z

When this will be merged?

4np · 2021-02-22T17:09:38Z

@scinfu see #4569

liuxuan30 · 2021-02-26T01:13:52Z

@jjatie what you think for this PR now?

liuxuan30 · 2021-02-26T01:17:46Z

I had a quick look and it seems great, if we don't consider the android part; unfortunately we need to think about iOS and android interface again :(

jjatie · 2021-02-26T12:21:32Z

I think that as Charts is today, the implementation makes sense. Two notes:

It seems heavy to subclass the chart in the example. Why not just set the renderer on a BarChartView?
It feels strange to have to drill down the existing renderer to pull out its animator. Looking at our implementation, it looks like we can just pass in the animator directly from the chart which feels better.

If we agree that we should move forward with this, we need to audit for inline-ability. We also need to ensure all DataRenderer types are updated to reflect this model. It is also a good time to audit the naming of a lot of these methods and their parameters. (I would say render*() should be all renamed draw*() to be more in line with UIKit convention and to be more accurate)

4np · 2021-02-26T14:18:20Z

@liuxuan30 : In essence the public Charts API does not change, it merely adds ways to more easily customize rendering behavior by introducing a couple of focussed open methods. Perhaps MPAndroidCharts can take inspiration of this change and implement it in there as well? 🤷‍♂️ MPAndroidChart can still accomplish the same thing without having these focussed rendering methods, it will just be more work. It will also help you to get rid of a lot of the Issues and PRs that could be just be solved by overriding these renderers.

@jjatie : Yes true, you don't need to subclass BarChartView and just use the PatternFilledBarChartRenderer in the example, I just thought it would just be a good example class for people to understand. If the animator is available in the chart itself that would be even better, although I think it wasn't (publicly) available in the chart itself? Not sure though.

May I suggest that this perhaps does not merge to danielgindi:master but that we rather change the PR base to a feature branch you create off master? Then perhaps this PR can be merged to that feature branch allowing you to make the changes you want to make (for example render > draw or perhaps some of the method signatures) before it makes it to master? Or do you want me to make some changes on my end?

ps. @inlinable could be interesting performance wise, but I'm not sure that would work for such public overridable methods? Perhaps only when you're absolutely certain these remain stable.

liuxuan30 · 2021-03-14T05:13:15Z

@danielgindi please take a look at this PR

danielgindi · 2021-03-14T05:52:06Z

I think this is something that I would actually want to port Android too. Will try to dedicate some time soon

kelvinofula · 2021-03-17T12:03:22Z

Quick question, how do I apply the same logic to achieve a piechart with rounded corners? I've been really trying it for a long time without success. Any help will really be much appreciated. Thanks a lot.

liuxuan30 · 2021-03-22T03:40:10Z

@kelvinofula do not hijack.

@jjatie I think we reach an agreement with @danielgindi that, we could go ahead to make some iOS features and refactorings.

AppCodeZip · 2021-03-27T17:31:14Z

Recently I have been working on a project, to which I have added a chart library and display a beautiful rounded bar chart in swift. We can achieve this functionality easily. Please check this demo.

4np · 2021-03-27T18:06:29Z

@AppCodeZip : Do not hijack this Pull Request, your example is exactly opposite of what this PR is about.

liuxuan30 · 2021-07-07T01:49:37Z

@jjatie I'm sorry to say that, I got a layoff and find a new job now, so I would be super busy for the rest of the year, tons of pressure from corp projects.. I might be missing for a few months (and already)

If you think the PR is good to go, probably you want to go ahead without my review.

4np · 2021-07-07T18:27:50Z

Ouch @liuxuan30, I am sorry to hear that. I can imagine your mind is not with this repository and pull request. Best of luck with your job hunt. Perhaps iOS Dev Jobs might be useful to you?

liuxuan30 · 2021-07-09T02:08:32Z

@4np haha thank you. I think my new job is lovely and challenging, especialy there are 6 months trial period for me, so I have to spend all my time on that. I really wanted to spend more time on Charts but recently I guess I couldn't afford.

So I have to stay quiet for some time. I trust @jjatie and @danielgindi could take care of some PRs.

4np · 2022-07-25T06:51:47Z

@pmairoldi @liuxuan30 @jjatie @danielgindi

Could you please take another look at this PR? :) Right now we have to maintain a fork to make it easier to override rendering, but it also makes uptaking latest changes more difficult. It would be great to have this merge into the repository so we can get rid of the fork.

Like mentioned above, it might solve a lot of other issues or, at the very least, make it easier for other developers to implement rendering changes.

417-72KI · 2022-09-15T05:58:40Z

@4np @pmairoldi @liuxuan30 @jjatie @danielgindi
Any updates?

4np · 2022-11-11T07:49:38Z

@pmairoldi @liuxuan30 @jjatie @danielgindi

This PR has been open for close to 3 years. I would still like to see this PR getting merged though. Any updates?

4np · 2024-03-05T16:49:35Z

We just celebrated the 4th anniversary of this PR! 🥳🎊 As we have since migrated over to Swift Charts and it doesn't look like this PR will ever be merged, I am hereby closing it.

vadimbelyaev · 2024-03-05T18:32:57Z

Update the renderers with dedicated atomic rendering methods in order…

5435144

… to make rendering overrides easier to implement.

4np requested review from danielgindi, jjatie and liuxuan30 February 25, 2020 15:16

Changed UIColor in platform independant NSUIColor.

a4e6e8e

Added the chart data entry to the bar chart rendering overrides.

d3e82c3

Merge branch 'master' into fix/add-rendering-override-points

778fd6f

# Conflicts: # Source/Charts/Renderers/BarChartRenderer.swift # Source/Charts/Renderers/LegendRenderer.swift # Source/Charts/Renderers/LineChartRenderer.swift # Source/Charts/Renderers/PieChartRenderer.swift

4np added 2 commits December 15, 2020 14:02

Added missing in-line documentation.

0ff8d4e

Added a fix for ChartsOrg#3701

a517e14

4np added 4 commits December 15, 2020 15:21

Revert "Added a fix for ChartsOrg#3701"

7e1f6a0

This reverts commit a517e14.

No need for drawBorder as that is already being handled by the render…

a5f094d

…Border method.

Added rendering override methods for CandleStickChartRenderer.

271a5ee

Fixed some crash-prone force unwraps.

f83591b

4np mentioned this pull request Jan 29, 2021

Review / merge PR #4297 #4569

Open

This was referenced Jan 29, 2021

Rounding the corners of bars #1066

Open

Rounded corners for BarChartView #4552

Open

Rounded corners in each pie chart slice #4540

Open

Can we create pie chart like this :- #4500

Open

417-72KI mentioned this pull request Aug 29, 2022

Add rendering override points wealthnavi/Charts#2

Merged

pmairoldi self-assigned this Sep 15, 2022

pmairoldi added this to the 5.0.0 milestone Mar 11, 2023

pmairoldi modified the milestones: 5.0.0, Future Jun 7, 2023

4np closed this Mar 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update the renderers with dedicated atomic rendering methods in order to make rendering overrides easier to implement. #4297

Update the renderers with dedicated atomic rendering methods in order to make rendering overrides easier to implement. #4297

4np commented Feb 25, 2020 •

edited

Loading

codecov-io commented Feb 25, 2020 •

edited

Loading

jjatie commented Feb 26, 2020

4np commented Feb 26, 2020 •

edited

Loading

4np commented Dec 15, 2020 •

edited

Loading

4np commented Dec 15, 2020 •

edited

Loading

4np commented Dec 15, 2020 •

edited

Loading

4np commented Dec 15, 2020 •

edited

Loading

bivant commented Dec 15, 2020 •

edited

Loading

4np commented Dec 16, 2020 •

edited

Loading

4np commented Dec 28, 2020

4np commented Jan 29, 2021

scinfu commented Feb 12, 2021

4np commented Feb 22, 2021

liuxuan30 commented Feb 26, 2021

liuxuan30 commented Feb 26, 2021 •

edited

Loading

jjatie commented Feb 26, 2021

4np commented Feb 26, 2021 •

edited

Loading

liuxuan30 commented Mar 14, 2021

danielgindi commented Mar 14, 2021

kelvinofula commented Mar 17, 2021

liuxuan30 commented Mar 22, 2021

AppCodeZip commented Mar 27, 2021

4np commented Mar 27, 2021 •

edited

Loading

liuxuan30 commented Jul 7, 2021 •

edited

Loading

4np commented Jul 7, 2021 •

edited

Loading

liuxuan30 commented Jul 9, 2021

4np commented Jul 25, 2022 •

edited

Loading

417-72KI commented Sep 15, 2022

4np commented Nov 11, 2022 •

edited

Loading

4np commented Mar 5, 2024

vadimbelyaev commented Mar 5, 2024

Update the renderers with dedicated atomic rendering methods in order to make rendering overrides easier to implement. #4297

Update the renderers with dedicated atomic rendering methods in order to make rendering overrides easier to implement. #4297

Conversation

4np commented Feb 25, 2020 • edited Loading

Context

Solution

Example: Bar Chart Pattern Fill.

Screenshot:

Lastly

codecov-io commented Feb 25, 2020 • edited Loading

Codecov Report

jjatie commented Feb 26, 2020

4np commented Feb 26, 2020 • edited Loading

4np commented Dec 15, 2020 • edited Loading

4np commented Dec 15, 2020 • edited Loading

Screenshots

4np commented Dec 15, 2020 • edited Loading

4np commented Dec 15, 2020 • edited Loading

bivant commented Dec 15, 2020 • edited Loading

4np commented Dec 16, 2020 • edited Loading

4np commented Dec 28, 2020

4np commented Jan 29, 2021

scinfu commented Feb 12, 2021

4np commented Feb 22, 2021

liuxuan30 commented Feb 26, 2021

liuxuan30 commented Feb 26, 2021 • edited Loading

jjatie commented Feb 26, 2021

4np commented Feb 26, 2021 • edited Loading

liuxuan30 commented Mar 14, 2021

danielgindi commented Mar 14, 2021

kelvinofula commented Mar 17, 2021

liuxuan30 commented Mar 22, 2021

AppCodeZip commented Mar 27, 2021

4np commented Mar 27, 2021 • edited Loading

liuxuan30 commented Jul 7, 2021 • edited Loading

4np commented Jul 7, 2021 • edited Loading

liuxuan30 commented Jul 9, 2021

4np commented Jul 25, 2022 • edited Loading

417-72KI commented Sep 15, 2022

4np commented Nov 11, 2022 • edited Loading

4np commented Mar 5, 2024

vadimbelyaev commented Mar 5, 2024

4np commented Feb 25, 2020 •

edited

Loading

codecov-io commented Feb 25, 2020 •

edited

Loading

4np commented Feb 26, 2020 •

edited

Loading

4np commented Dec 15, 2020 •

edited

Loading

4np commented Dec 15, 2020 •

edited

Loading

4np commented Dec 15, 2020 •

edited

Loading

4np commented Dec 15, 2020 •

edited

Loading

bivant commented Dec 15, 2020 •

edited

Loading

4np commented Dec 16, 2020 •

edited

Loading

liuxuan30 commented Feb 26, 2021 •

edited

Loading

4np commented Feb 26, 2021 •

edited

Loading

4np commented Mar 27, 2021 •

edited

Loading

liuxuan30 commented Jul 7, 2021 •

edited

Loading

4np commented Jul 7, 2021 •

edited

Loading

4np commented Jul 25, 2022 •

edited

Loading

4np commented Nov 11, 2022 •

edited

Loading