From 81456db5594e989d98b62df04a53ebe420900cea Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 3 May 2019 12:26:35 -0700 Subject: [PATCH] Roughly support stacked area charts Summary: Ref T13279. This adds support for: - Datasets can have types, like "stacked area". - Datasets can have multiple functions. - Charts can store dataset types and datasets with multiple functions. - Adds a "stacked area" dataset. - Makes D3 actually draw a stacked area chart. Lots of rough edges here still, but the result looks slightly more like it's supposed to look. D3 can do some of this logic itself, like adding up the area stacks on top of one another with `d3.stack()`. I'm doing it in PHP instead because I think it's a bit easier to debug, and it gives us more options for things like caching or "export to CSV" or "export to API" or rendering a data table under the chart or whatever. Test Plan: {F6427780} Reviewers: amckinley Reviewed By: amckinley Subscribers: yelirekim Maniphest Tasks: T13279 Differential Revision: https://secure.phabricator.com/D20498 --- resources/celerity/map.php | 12 +- src/__phutil_library_map__.php | 2 + .../PhabricatorAccumulateChartFunction.php | 5 +- .../fact/chart/PhabricatorChartDataset.php | 69 ++++++-- .../fact/chart/PhabricatorChartFunction.php | 31 +++- .../PhabricatorChartStackedAreaDataset.php | 149 ++++++++++++++++++ .../PhabricatorChartRenderingEngine.php | 40 ++--- .../PhabricatorProjectBurndownChartEngine.php | 44 +++--- webroot/rsrc/js/application/fact/Chart.js | 131 ++++++++------- 9 files changed, 343 insertions(+), 140 deletions(-) create mode 100644 src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index b23ffca37c..a2bc989ea6 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -389,7 +389,7 @@ return array( 'rsrc/js/application/diffusion/behavior-pull-lastmodified.js' => 'c715c123', 'rsrc/js/application/doorkeeper/behavior-doorkeeper-tag.js' => '6a85bc5a', 'rsrc/js/application/drydock/drydock-live-operation-status.js' => '47a0728b', - 'rsrc/js/application/fact/Chart.js' => 'fcb0c07d', + 'rsrc/js/application/fact/Chart.js' => 'a3516cea', 'rsrc/js/application/files/behavior-document-engine.js' => '243d6c22', 'rsrc/js/application/files/behavior-icon-composer.js' => '38a6cedb', 'rsrc/js/application/files/behavior-launch-icon-composer.js' => 'a17b84f1', @@ -696,7 +696,7 @@ return array( 'javelin-behavior-user-menu' => '60cd9241', 'javelin-behavior-view-placeholder' => 'a9942052', 'javelin-behavior-workflow' => '9623adc1', - 'javelin-chart' => 'fcb0c07d', + 'javelin-chart' => 'a3516cea', 'javelin-color' => '78f811c9', 'javelin-cookie' => '05d290ef', 'javelin-diffusion-locate-file-source' => '94243d89', @@ -1767,6 +1767,10 @@ return array( 'javelin-workflow', 'phabricator-draggable-list', ), + 'a3516cea' => array( + 'phui-chart-css', + 'd3', + ), 'a4356cde' => array( 'javelin-install', 'javelin-dom', @@ -2180,10 +2184,6 @@ return array( 'fa74cc35' => array( 'phui-oi-list-view-css', ), - 'fcb0c07d' => array( - 'phui-chart-css', - 'd3', - ), 'fdc13e4e' => array( 'javelin-install', ), diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4b32787a36..21cd7c90d9 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2669,6 +2669,7 @@ phutil_register_library_map(array( 'PhabricatorChartFunctionArgument' => 'applications/fact/chart/PhabricatorChartFunctionArgument.php', 'PhabricatorChartFunctionArgumentParser' => 'applications/fact/chart/PhabricatorChartFunctionArgumentParser.php', 'PhabricatorChartRenderingEngine' => 'applications/fact/engine/PhabricatorChartRenderingEngine.php', + 'PhabricatorChartStackedAreaDataset' => 'applications/fact/chart/PhabricatorChartStackedAreaDataset.php', 'PhabricatorChatLogApplication' => 'applications/chatlog/application/PhabricatorChatLogApplication.php', 'PhabricatorChatLogChannel' => 'applications/chatlog/storage/PhabricatorChatLogChannel.php', 'PhabricatorChatLogChannelListController' => 'applications/chatlog/controller/PhabricatorChatLogChannelListController.php', @@ -8683,6 +8684,7 @@ phutil_register_library_map(array( 'PhabricatorChartFunctionArgument' => 'Phobject', 'PhabricatorChartFunctionArgumentParser' => 'Phobject', 'PhabricatorChartRenderingEngine' => 'Phobject', + 'PhabricatorChartStackedAreaDataset' => 'PhabricatorChartDataset', 'PhabricatorChatLogApplication' => 'PhabricatorApplication', 'PhabricatorChatLogChannel' => array( 'PhabricatorChatLogDAO', diff --git a/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php b/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php index 074219504c..6ffbb85da9 100644 --- a/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php +++ b/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php @@ -35,8 +35,9 @@ final class PhabricatorAccumulateChartFunction $datasource_xv = $datasource->newInputValues($empty_query); if (!$datasource_xv) { - // TODO: Maybe this should just be an error? - $datasource_xv = $xv; + // When the datasource has no datapoints, we can't evaluate the function + // anywhere. + return array_fill(0, count($xv), null); } $yv = $datasource->evaluateFunction($datasource_xv); diff --git a/src/applications/fact/chart/PhabricatorChartDataset.php b/src/applications/fact/chart/PhabricatorChartDataset.php index 48355c3b36..df3984f9ce 100644 --- a/src/applications/fact/chart/PhabricatorChartDataset.php +++ b/src/applications/fact/chart/PhabricatorChartDataset.php @@ -1,42 +1,77 @@ function; + final public function getDatasetTypeKey() { + return $this->getPhobjectClassConstant('DATASETKEY', 32); } - public function setFunction(PhabricatorComposeChartFunction $function) { - $this->function = $function; + final public function getFunctions() { + return $this->functions; + } + + final public function setFunctions(array $functions) { + assert_instances_of($functions, 'PhabricatorComposeChartFunction'); + + $this->functions = $functions; + return $this; } - public static function newFromDictionary(array $map) { + final public static function getAllDatasetTypes() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getDatasetTypeKey') + ->execute(); + } + + final public static function newFromDictionary(array $map) { PhutilTypeSpec::checkMap( $map, array( - 'function' => 'list', + 'type' => 'string', + 'functions' => 'list', )); - $dataset = new self(); + $types = self::getAllDatasetTypes(); - $dataset->function = id(new PhabricatorComposeChartFunction()) - ->setArguments(array($map['function'])); + $dataset_type = $map['type']; + if (!isset($types[$dataset_type])) { + throw new Exception( + pht( + 'Trying to construct a dataset of type "%s", but this type is '. + 'unknown. Supported types are: %s.', + $dataset_type, + implode(', ', array_keys($types)))); + } + + $dataset = id(clone $types[$dataset_type]); + + $functions = array(); + foreach ($map['functions'] as $map) { + $functions[] = PhabricatorChartFunction::newFromDictionary($map); + } + $dataset->setFunctions($functions); return $dataset; } - public function toDictionary() { - // Since we wrap the raw value in a "compose(...)", when deserializing, - // we need to unwrap it when serializing. - $function_raw = head($this->getFunction()->toDictionary()); - + final public function toDictionary() { return array( - 'function' => $function_raw, + 'type' => $this->getDatasetTypeKey(), + 'functions' => mpull($this->getFunctions(), 'toDictionary'), ); } + final public function getWireFormat(PhabricatorChartDataQuery $data_query) { + return $this->newWireFormat($data_query); + } + + abstract protected function newWireFormat( + PhabricatorChartDataQuery $data_query); + + } diff --git a/src/applications/fact/chart/PhabricatorChartFunction.php b/src/applications/fact/chart/PhabricatorChartFunction.php index b4a66645ad..5b610ae0fa 100644 --- a/src/applications/fact/chart/PhabricatorChartFunction.php +++ b/src/applications/fact/chart/PhabricatorChartFunction.php @@ -43,8 +43,37 @@ abstract class PhabricatorChartFunction return $this; } + final public static function newFromDictionary(array $map) { + PhutilTypeSpec::checkMap( + $map, + array( + 'function' => 'string', + 'arguments' => 'list', + )); + + $functions = self::getAllFunctions(); + + $function_name = $map['function']; + if (!isset($functions[$function_name])) { + throw new Exception( + pht( + 'Attempting to build function "%s" from dictionary, but that '. + 'function is unknown. Known functions are: %s.', + $function_name, + implode(', ', array_keys($functions)))); + } + + $function = id(clone $functions[$function_name]) + ->setArguments($map['arguments']); + + return $function; + } + public function toDictionary() { - return $this->getArgumentParser()->getRawArguments(); + return array( + 'function' => $this->getFunctionKey(), + 'arguments' => $this->getArgumentParser()->getRawArguments(), + ); } public function getSubfunctions() { diff --git a/src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php b/src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php new file mode 100644 index 0000000000..1683e6e268 --- /dev/null +++ b/src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php @@ -0,0 +1,149 @@ +getFunctions(); + + $function_points = array(); + foreach ($functions as $function_idx => $function) { + $function_points[$function_idx] = array(); + + $datapoints = $function->newDatapoints($data_query); + foreach ($datapoints as $point) { + $x = $point['x']; + $function_points[$function_idx][$x] = $point; + } + } + + $raw_points = $function_points; + + // We need to define every function we're drawing at every point where + // any of the functions we're drawing are defined. If we don't, we'll + // end up with weird gaps or overlaps between adjacent areas, and won't + // know how much we need to lift each point above the baseline when + // stacking the functions on top of one another. + + $must_define = array(); + foreach ($function_points as $function_idx => $points) { + foreach ($points as $x => $point) { + $must_define[$x] = $x; + } + } + ksort($must_define); + + foreach ($functions as $function_idx => $function) { + $missing = array(); + foreach ($must_define as $x) { + if (!isset($function_points[$function_idx][$x])) { + $missing[$x] = true; + } + } + + if (!$missing) { + continue; + } + + $points = $function_points[$function_idx]; + + $values = array_keys($points); + $cursor = -1; + $length = count($values); + + foreach ($missing as $x => $ignored) { + // Move the cursor forward until we find the last point before "x" + // which is defined. + while ($cursor + 1 < $length && $values[$cursor + 1] < $x) { + $cursor++; + } + + // If this new point is to the left of all defined points, we'll + // assume the value is 0. If the point is to the right of all defined + // points, we assume the value is the same as the last known value. + + // If it's between two defined points, we average them. + + if ($cursor < 0) { + $y = 0; + } else if ($cursor + 1 < $length) { + $xmin = $values[$cursor]; + $xmax = $values[$cursor + 1]; + + $ymin = $points[$xmin]['y']; + $ymax = $points[$xmax]['y']; + + // Fill in the missing point by creating a linear interpolation + // between the two adjacent points. + $distance = ($x - $xmin) / ($xmax - $xmin); + $y = $ymin + (($ymax - $ymin) * $distance); + } else { + $xmin = $values[$cursor]; + $y = $function_points[$function_idx][$xmin]['y']; + } + + $function_points[$function_idx][$x] = array( + 'x' => $x, + 'y' => $y, + ); + } + + ksort($function_points[$function_idx]); + } + + $series = array(); + $baseline = array(); + foreach ($function_points as $function_idx => $points) { + $below = idx($function_points, $function_idx - 1); + + $bounds = array(); + foreach ($points as $x => $point) { + if (!isset($baseline[$x])) { + $baseline[$x] = 0; + } + + $y0 = $baseline[$x]; + $baseline[$x] += $point['y']; + $y1 = $baseline[$x]; + + $bounds[] = array( + 'x' => $x, + 'y0' => $y0, + 'y1' => $y1, + ); + + if (isset($raw_points[$function_idx][$x])) { + $raw_points[$function_idx][$x]['y1'] = $y1; + } + } + + $series[] = $bounds; + } + + $events = array(); + foreach ($raw_points as $function_idx => $points) { + $event_list = array(); + foreach ($points as $point) { + $event_list[] = $point; + } + $events[] = $event_list; + } + + $result = array( + 'type' => $this->getDatasetTypeKey(), + 'data' => $series, + 'events' => $events, + 'color' => array( + 'blue', + 'cyan', + 'green', + ), + ); + + return $result; + } + + +} diff --git a/src/applications/fact/engine/PhabricatorChartRenderingEngine.php b/src/applications/fact/engine/PhabricatorChartRenderingEngine.php index 7916e35704..1b77d2403f 100644 --- a/src/applications/fact/engine/PhabricatorChartRenderingEngine.php +++ b/src/applications/fact/engine/PhabricatorChartRenderingEngine.php @@ -119,7 +119,9 @@ final class PhabricatorChartRenderingEngine $functions = array(); foreach ($datasets as $dataset) { - $functions[] = $dataset->getFunction(); + foreach ($dataset->getFunctions() as $function) { + $functions[] = $function; + } } $subfunctions = array(); @@ -144,39 +146,17 @@ final class PhabricatorChartRenderingEngine ->setMaximumValue($domain_max) ->setLimit(2000); - $datasets = array(); - foreach ($functions as $function) { - $points = $function->newDatapoints($data_query); - - $x = array(); - $y = array(); - - foreach ($points as $point) { - $x[] = $point['x']; - $y[] = $point['y']; - } - - $datasets[] = array( - 'x' => $x, - 'y' => $y, - 'color' => '#ff00ff', - ); - } - - - $y_min = 0; - $y_max = 0; + $wire_datasets = array(); foreach ($datasets as $dataset) { - if (!$dataset['y']) { - continue; - } - - $y_min = min($y_min, min($dataset['y'])); - $y_max = max($y_max, max($dataset['y'])); + $wire_datasets[] = $dataset->getWireFormat($data_query); } + // TODO: Figure these out from the datasets again. + $y_min = -2; + $y_max = 20; + $chart_data = array( - 'datasets' => $datasets, + 'datasets' => $wire_datasets, 'xMin' => $domain_min, 'xMax' => $domain_max, 'yMin' => $y_min, diff --git a/src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php b/src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php index 4b4a99ecf4..fd4a872bbd 100644 --- a/src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php +++ b/src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php @@ -30,37 +30,33 @@ final class PhabricatorProjectBurndownChartEngine if ($project_phids) { foreach ($project_phids as $project_phid) { $argvs[] = array( - 'sum', - array( - 'accumulate', - array('fact', 'tasks.open-count.create.project', $project_phid), - ), - array( - 'accumulate', - array('fact', 'tasks.open-count.status.project', $project_phid), - ), - array( - 'accumulate', - array('fact', 'tasks.open-count.assign.project', $project_phid), - ), + 'accumulate', + array('fact', 'tasks.open-count.create.project', $project_phid), + ); + $argvs[] = array( + 'accumulate', + array('fact', 'tasks.open-count.status.project', $project_phid), + ); + $argvs[] = array( + 'accumulate', + array('fact', 'tasks.open-count.assign.project', $project_phid), ); } } else { - $argvs[] = array( - 'sum', - array('accumulate', array('fact', 'tasks.open-count.create')), - array('accumulate', array('fact', 'tasks.open-count.status')), - ); + $argvs[] = array('accumulate', array('fact', 'tasks.open-count.create')); + $argvs[] = array('accumulate', array('fact', 'tasks.open-count.status')); + } + + $functions = array(); + foreach ($argvs as $argv) { + $functions[] = id(new PhabricatorComposeChartFunction()) + ->setArguments(array($argv)); } $datasets = array(); - foreach ($argvs as $argv) { - $function = id(new PhabricatorComposeChartFunction()) - ->setArguments(array($argv)); - $datasets[] = id(new PhabricatorChartDataset()) - ->setFunction($function); - } + $datasets[] = id(new PhabricatorChartStackedAreaDataset()) + ->setFunctions($functions); $chart = id(new PhabricatorFactChart()) ->setDatasets($datasets); diff --git a/webroot/rsrc/js/application/fact/Chart.js b/webroot/rsrc/js/application/fact/Chart.js index ceb3b2ad00..66a9b98d70 100644 --- a/webroot/rsrc/js/application/fact/Chart.js +++ b/webroot/rsrc/js/application/fact/Chart.js @@ -26,6 +26,10 @@ JX.install('Chart', { } var hardpoint = this._rootNode; + + // Remove the old chart (if one exists) before drawing the new chart. + JX.DOM.setContent(hardpoint, []); + var viewport = JX.Vector.getDim(hardpoint); var config = this._data; @@ -48,22 +52,14 @@ JX.install('Chart', { size.width = size.frameWidth - padding.left - padding.right; size.height = size.frameHeight - padding.top - padding.bottom; - var x = d3.time.scale() + var x = d3.scaleTime() .range([0, size.width]); - var y = d3.scale.linear() + var y = d3.scaleLinear() .range([size.height, 0]); - var xAxis = d3.svg.axis() - .scale(x) - .orient('bottom'); - - var yAxis = d3.svg.axis() - .scale(y) - .orient('left'); - - // Remove the old chart (if one exists) before drawing the new chart. - JX.DOM.setContent(hardpoint, []); + var xAxis = d3.axisBottom(x); + var yAxis = d3.axisLeft(y); var svg = d3.select('#' + hardpoint.id).append('svg') .attr('width', size.frameWidth) @@ -80,11 +76,7 @@ JX.install('Chart', { .attr('width', size.width) .attr('height', size.height); - function as_date(value) { - return new Date(value * 1000); - } - - x.domain([as_date(config.xMin), as_date(config.xMax)]); + x.domain([this._newDate(config.xMin), this._newDate(config.xMax)]); y.domain([config.yMin, config.yMax]); var div = d3.select('body') @@ -95,50 +87,11 @@ JX.install('Chart', { for (var idx = 0; idx < config.datasets.length; idx++) { var dataset = config.datasets[idx]; - var line = d3.svg.line() - .x(function(d) { return x(d.xvalue); }) - .y(function(d) { return y(d.yvalue); }); - - var data = []; - for (var ii = 0; ii < dataset.x.length; ii++) { - data.push( - { - xvalue: as_date(dataset.x[ii]), - yvalue: dataset.y[ii] - }); + switch (dataset.type) { + case 'stacked-area': + this._newStackedArea(g, dataset, x, y, div); + break; } - - g.append('path') - .datum(data) - .attr('class', 'line') - .style('stroke', dataset.color) - .attr('d', line); - - g.selectAll('dot') - .data(data) - .enter() - .append('circle') - .attr('class', 'point') - .attr('r', 3) - .attr('cx', function(d) { return x(d.xvalue); }) - .attr('cy', function(d) { return y(d.yvalue); }) - .on('mouseover', function(d) { - var d_y = d.xvalue.getFullYear(); - - // NOTE: Javascript months are zero-based. See PHI1017. - var d_m = d.xvalue.getMonth() + 1; - - var d_d = d.xvalue.getDate(); - - div - .html(d_y + '-' + d_m + '-' + d_d + ': ' + d.yvalue) - .style('opacity', 0.9) - .style('left', (d3.event.pageX - 60) + 'px') - .style('top', (d3.event.pageY - 38) + 'px'); - }) - .on('mouseout', function() { - div.style('opacity', 0); - }); } g.append('g') @@ -150,7 +103,65 @@ JX.install('Chart', { .attr('class', 'y axis') .attr('transform', css_function('translate', 0, 0)) .call(yAxis); + }, + + _newStackedArea: function(g, dataset, x, y, div) { + var to_date = JX.bind(this, this._newDate); + + var area = d3.area() + .x(function(d) { return x(to_date(d.x)); }) + .y0(function(d) { return y(d.y0); }) + .y1(function(d) { return y(d.y1); }); + + var line = d3.line() + .x(function(d) { return x(to_date(d.x)); }) + .y(function(d) { return y(d.y1); }); + + for (var ii = 0; ii < dataset.data.length; ii++) { + g.append('path') + .style('fill', dataset.color[ii % dataset.color.length]) + .style('opacity', '0.15') + .attr('d', area(dataset.data[ii])); + + g.append('path') + .attr('class', 'line') + .attr('d', line(dataset.data[ii])); + + g.selectAll('dot') + .data(dataset.events[ii]) + .enter() + .append('circle') + .attr('class', 'point') + .attr('r', 3) + .attr('cx', function(d) { return x(to_date(d.x)); }) + .attr('cy', function(d) { return y(d.y1); }) + .on('mouseover', function(d) { + var dd = to_date(d.x); + + var d_y = dd.getFullYear(); + + // NOTE: Javascript months are zero-based. See PHI1017. + var d_m = dd.getMonth() + 1; + + var d_d = dd.getDate(); + + div + .html(d_y + '-' + d_m + '-' + d_d + ': ' + d.y1) + .style('opacity', 0.9) + .style('left', (d3.event.pageX - 60) + 'px') + .style('top', (d3.event.pageY - 38) + 'px'); + }) + .on('mouseout', function() { + div.style('opacity', 0); + }); + + } + }, + + _newDate: function(epoch) { + return new Date(epoch * 1000); } + } });