Skip to content
Closed
Changes from all commits
Commits
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
54 changes: 40 additions & 14 deletions src/controllers/controller.polarArea.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,32 @@ module.exports = function(Chart) {
});
},

getStartAngle: function(customAngles, dataSetData, metaData, datasetStartAngle, circumference, index) {

// If there is NaN data before us, we need to calculate the starting angle correctly.
// We could be way more efficient here, but its unlikely that the polar area chart will have a lot of data

var previousAngle = 0;
var visibleCount = 0;
var startAngle = 0;
var i = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be just var i since it's initialized to 0 below


for (i = 0; i < index; ++i) {
Copy link
Member

@simonbrunel simonbrunel Oct 1, 2017

Choose a reason for hiding this comment

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

Do we really need to iterate every time for each element? can't we use angle at index-1 to calculate the angle of the element at index

Choose a reason for hiding this comment

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

I think we have to iterate since the array (or function/number) indicate the angle for one item and not it's starting angle. So to get the starting angle of the current element we need to calculate it from the start angle of the chart. Or would I have missed something?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that's right.

Copy link
Member

Choose a reason for hiding this comment

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

What I mean is that we could compute a _starts and _angles private cache arrays in update() that contains all start angles in which case only one iteration is needed, something like:

update: function(...) {
    // ...
    var start = options.startAngle || 0;
    for (i = 0; i < count; ++i) {
        this._starts[i] = start;
        angle = [compute slice angle]; // if dataset invisible, angle == 0
        this._angles[i] = angle;
        start += angle;
    }
    // ...
    helpers.each(meta.data, function(arc, index) {
        me.updateElement(arc, index, reset);
    });
}

updateElement: function(...) {
   // ...
   var startAngle = this._starts[index];
   var endAngle = startAngle + this._angles[index];
   // ...
}

It seems to simplify a lot the computation / logic.

Choose a reason for hiding this comment

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

I hadn't thought about that but yes it could save some performance.

if (customAngles !== undefined) {
previousAngle += customAngles[visibleCount];
}
if (!isNaN(dataSetData[i]) && !metaData[i].hidden) {
++visibleCount;
}
}
if (customAngles === undefined) {
startAngle = datasetStartAngle + (circumference * visibleCount);
} else {
startAngle = datasetStartAngle + previousAngle;
}

return startAngle;
},
updateElement: function(arc, index, reset) {
var me = this;
var chart = me.chart;
Expand All @@ -146,25 +172,18 @@ module.exports = function(Chart) {
var animationOpts = opts.animation;
var scale = chart.scale;
var labels = chart.data.labels;
var customAngles = opts.customAngles;
var meta = me.getMeta();

var circumference = me.calculateCircumference(dataset.data[index]);
var circumference = me.calculateCircumference(dataset.data[index], index);
var centerX = scale.xCenter;
var centerY = scale.yCenter;

// If there is NaN data before us, we need to calculate the starting angle correctly.
// We could be way more efficient here, but its unlikely that the polar area chart will have a lot of data
var visibleCount = 0;
var meta = me.getMeta();
for (var i = 0; i < index; ++i) {
if (!isNaN(dataset.data[i]) && !meta.data[i].hidden) {
++visibleCount;
}
}

// var negHalfPI = -0.5 * Math.PI;
var datasetStartAngle = opts.startAngle;
var distance = arc.hidden ? 0 : scale.getDistanceFromCenterForValue(dataset.data[index]);
var startAngle = datasetStartAngle + (circumference * visibleCount);

var startAngle = me.getStartAngle(customAngles, dataset.data, meta.data, datasetStartAngle, circumference, index);
var endAngle = startAngle + (arc.hidden ? 0 : circumference);

var resetRadius = animationOpts.animateScale ? 0 : scale.getDistanceFromCenterForValue(dataset.data[index]);
Expand Down Expand Up @@ -211,10 +230,17 @@ module.exports = function(Chart) {
return count;
},

calculateCircumference: function(value) {
calculateCircumference: function(value, index) {
var me = this;
var chart = me.chart;
var opts = chart.options;
var customAngles = opts.customAngles;
var count = this.getMeta().count;
if (count > 0 && !isNaN(value)) {
return (2 * Math.PI) / count;
if (customAngles === undefined) {
return (2 * Math.PI) / count;
}
return count === 1 ? 2 * Math.PI : customAngles[index];
}
return 0;
}
Expand Down