From c06dd4818b53e1bf6f4872c00d7e643beff57178 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 17 Sep 2019 12:04:44 -0700 Subject: [PATCH] Support explicit stacking configuration in stacked area charts Summary: Ref T13279. Allow engines to choose how areas in a stacked area chart stack on top of one another. This could also be accomplished by using multiple stacked area datasets, but datasets would still need to know if they're stacking "up" or "down" so it's probably about the same at the end of the day. Test Plan: {F6865165} Subscribers: yelirekim Maniphest Tasks: T13279 Differential Revision: https://secure.phabricator.com/D20818 --- .../fact/chart/PhabricatorChartDataset.php | 7 - .../fact/chart/PhabricatorChartFunction.php | 11 +- .../chart/PhabricatorChartFunctionLabel.php | 11 + .../PhabricatorChartStackedAreaDataset.php | 284 +++++++++++------- .../PhabricatorProjectBurndownChartEngine.php | 68 ++++- 5 files changed, 247 insertions(+), 134 deletions(-) diff --git a/src/applications/fact/chart/PhabricatorChartDataset.php b/src/applications/fact/chart/PhabricatorChartDataset.php index a794834229..093a742077 100644 --- a/src/applications/fact/chart/PhabricatorChartDataset.php +++ b/src/applications/fact/chart/PhabricatorChartDataset.php @@ -59,13 +59,6 @@ abstract class PhabricatorChartDataset return $dataset; } - final public function toDictionary() { - return array( - 'type' => $this->getDatasetTypeKey(), - 'functions' => mpull($this->getFunctions(), 'toDictionary'), - ); - } - final public function getChartDisplayData( PhabricatorChartDataQuery $data_query) { return $this->newChartDisplayData($data_query); diff --git a/src/applications/fact/chart/PhabricatorChartFunction.php b/src/applications/fact/chart/PhabricatorChartFunction.php index ef5c5641bf..3ddcd6aec0 100644 --- a/src/applications/fact/chart/PhabricatorChartFunction.php +++ b/src/applications/fact/chart/PhabricatorChartFunction.php @@ -60,6 +60,10 @@ abstract class PhabricatorChartFunction return $this->functionLabel; } + final public function getKey() { + return $this->getFunctionLabel()->getKey(); + } + final public static function newFromDictionary(array $map) { PhutilTypeSpec::checkMap( $map, @@ -86,13 +90,6 @@ abstract class PhabricatorChartFunction return $function; } - public function toDictionary() { - return array( - 'function' => $this->getFunctionKey(), - 'arguments' => $this->getArgumentParser()->getRawArguments(), - ); - } - public function getSubfunctions() { $result = array(); $result[] = $this; diff --git a/src/applications/fact/chart/PhabricatorChartFunctionLabel.php b/src/applications/fact/chart/PhabricatorChartFunctionLabel.php index ad85c49b71..fa3f65aa67 100644 --- a/src/applications/fact/chart/PhabricatorChartFunctionLabel.php +++ b/src/applications/fact/chart/PhabricatorChartFunctionLabel.php @@ -3,11 +3,21 @@ final class PhabricatorChartFunctionLabel extends Phobject { + private $key; private $name; private $color; private $icon; private $fillColor; + public function setKey($key) { + $this->key = $key; + return $this; + } + + public function getKey() { + return $this->key; + } + public function setName($name) { $this->name = $name; return $this; @@ -46,6 +56,7 @@ final class PhabricatorChartFunctionLabel public function toWireFormat() { return array( + 'key' => $this->getKey(), 'name' => $this->getName(), 'color' => $this->getColor(), 'icon' => $this->getIcon(), diff --git a/src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php b/src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php index 4d30cd767b..2ba08ea1c9 100644 --- a/src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php +++ b/src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php @@ -5,124 +5,89 @@ final class PhabricatorChartStackedAreaDataset const DATASETKEY = 'stacked-area'; + private $stacks; + + public function setStacks(array $stacks) { + $this->stacks = $stacks; + return $this; + } + + public function getStacks() { + return $this->stacks; + } + protected function newChartDisplayData( PhabricatorChartDataQuery $data_query) { + $functions = $this->getFunctions(); + $functions = mpull($functions, null, 'getKey'); - $reversed_functions = array_reverse($functions, true); + $stacks = $this->getStacks(); - $function_points = array(); - foreach ($reversed_functions as $function_idx => $function) { - $function_points[$function_idx] = array(); - - $datapoints = $function->newDatapoints($data_query); - foreach ($datapoints as $point) { - $x_value = $point['x']; - $function_points[$function_idx][$x_value] = $point; - } + if (!$stacks) { + $stacks = array( + array_reverse(array_keys($functions), true), + ); } - $raw_points = $function_points; + $series = array(); + $raw_points = array(); - // 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. + foreach ($stacks as $stack) { + $stack_functions = array_select_keys($functions, $stack); - $must_define = array(); - foreach ($function_points as $function_idx => $points) { - foreach ($points as $x => $point) { - $must_define[$x] = $x; + $function_points = $this->getFunctionDatapoints( + $data_query, + $stack_functions); + + $stack_points = $function_points; + + $function_points = $this->getGeometry( + $data_query, + $function_points); + + $baseline = array(); + foreach ($function_points as $function_idx => $points) { + $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($stack_points[$function_idx][$x])) { + $stack_points[$function_idx][$x]['y1'] = $y1; + } + } + + $series[$function_idx] = $bounds; } + + $raw_points += $stack_points; } - ksort($must_define); - foreach ($reversed_functions as $function_idx => $function) { - $missing = array(); - foreach ($must_define as $x) { - if (!isset($function_points[$function_idx][$x])) { - $missing[$x] = true; - } - } + $series = array_select_keys($series, array_keys($functions)); + $series = array_values($series); - 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]); - } + $raw_points = array_select_keys($raw_points, array_keys($functions)); + $raw_points = array_values($raw_points); $range_min = null; $range_max = null; - $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; - } + foreach ($series as $geometry_list) { + foreach ($geometry_list as $geometry_item) { + $y0 = $geometry_item['y0']; + $y1 = $geometry_item['y1']; if ($range_min === null) { $range_min = $y0; @@ -134,12 +99,8 @@ final class PhabricatorChartStackedAreaDataset } $range_max = max($range_max, $y0, $y1); } - - $series[] = $bounds; } - $series = array_reverse($series); - // We're going to group multiple events into a single point if they have // X values that are very close to one another. // @@ -222,5 +183,118 @@ final class PhabricatorChartStackedAreaDataset ->setRange(new PhabricatorChartInterval($range_min, $range_max)); } + private function getAllXValuesAsMap( + PhabricatorChartDataQuery $data_query, + array $point_lists) { + + // 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(); + + $min = $data_query->getMinimumValue(); + $max = $data_query->getMaximumValue(); + $must_define[$max] = $max; + $must_define[$min] = $min; + + foreach ($point_lists as $point_list) { + foreach ($point_list as $x => $point) { + $must_define[$x] = $x; + } + } + + ksort($must_define); + + return $must_define; + } + + private function getFunctionDatapoints( + PhabricatorChartDataQuery $data_query, + array $functions) { + + assert_instances_of($functions, 'PhabricatorChartFunction'); + + $points = array(); + foreach ($functions as $idx => $function) { + $points[$idx] = array(); + + $datapoints = $function->newDatapoints($data_query); + foreach ($datapoints as $point) { + $x_value = $point['x']; + $points[$idx][$x_value] = $point; + } + } + + return $points; + } + + private function getGeometry( + PhabricatorChartDataQuery $data_query, + array $point_lists) { + + $must_define = $this->getAllXValuesAsMap($data_query, $point_lists); + + foreach ($point_lists as $idx => $points) { + + $missing = array(); + foreach ($must_define as $x) { + if (!isset($points[$x])) { + $missing[$x] = true; + } + } + + if (!$missing) { + continue; + } + + $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 = $points[$xmin]['y']; + } + + $point_lists[$idx][$x] = array( + 'x' => $x, + 'y' => $y, + ); + } + + ksort($point_lists[$idx]); + } + + return $point_lists; + } } diff --git a/src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php b/src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php index 2dbb2a6a16..8dda7cde16 100644 --- a/src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php +++ b/src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php @@ -29,6 +29,8 @@ final class PhabricatorProjectBurndownChartEngine } $functions = array(); + $stacks = array(); + if ($project_phids) { foreach ($project_phids as $project_phid) { $function = $this->newFunction( @@ -42,12 +44,31 @@ final class PhabricatorProjectBurndownChartEngine )); $function->getFunctionLabel() + ->setKey('moved-in') ->setName(pht('Tasks Moved Into Project')) ->setColor('rgba(128, 128, 200, 1)') ->setFillColor('rgba(128, 128, 200, 0.15)'); $functions[] = $function; + $function = $this->newFunction( + array( + 'accumulate', + array( + 'compose', + array('fact', 'tasks.open-count.status.project', $project_phid), + array('min', 0), + ), + )); + + $function->getFunctionLabel() + ->setKey('reopened') + ->setName(pht('Tasks Reopened')) + ->setColor('rgba(128, 128, 200, 1)') + ->setFillColor('rgba(128, 128, 200, 0.15)'); + + $functions[] = $function; + $function = $this->newFunction( array( 'accumulate', @@ -55,12 +76,31 @@ final class PhabricatorProjectBurndownChartEngine )); $function->getFunctionLabel() + ->setKey('created') ->setName(pht('Tasks Created')) ->setColor('rgba(0, 0, 200, 1)') ->setFillColor('rgba(0, 0, 200, 0.15)'); $functions[] = $function; + $function = $this->newFunction( + array( + 'accumulate', + array( + 'compose', + array('fact', 'tasks.open-count.status.project', $project_phid), + array('max', 0), + ), + )); + + $function->getFunctionLabel() + ->setKey('closed') + ->setName(pht('Tasks Closed')) + ->setColor('rgba(0, 200, 0, 1)') + ->setFillColor('rgba(0, 200, 0, 0.15)'); + + $functions[] = $function; + $function = $this->newFunction( array( 'accumulate', @@ -72,24 +112,15 @@ final class PhabricatorProjectBurndownChartEngine )); $function->getFunctionLabel() + ->setKey('moved-out') ->setName(pht('Tasks Moved Out of Project')) ->setColor('rgba(128, 200, 128, 1)') ->setFillColor('rgba(128, 200, 128, 0.15)'); $functions[] = $function; - $function = $this->newFunction( - array( - 'accumulate', - array('fact', 'tasks.open-count.status.project', $project_phid), - )); - - $function->getFunctionLabel() - ->setName(pht('Tasks Closed')) - ->setColor('rgba(0, 200, 0, 1)') - ->setFillColor('rgba(0, 200, 0, 0.15)'); - - $functions[] = $function; + $stacks[] = array('created', 'reopened', 'moved-in'); + $stacks[] = array('closed', 'moved-out'); } } else { $function = $this->newFunction( @@ -99,7 +130,8 @@ final class PhabricatorProjectBurndownChartEngine )); $function->getFunctionLabel() - ->setName(pht('Tasks Created')) + ->setKey('open') + ->setName(pht('Open Tasks')) ->setColor('rgba(0, 0, 200, 1)') ->setFillColor('rgba(0, 0, 200, 0.15)'); @@ -112,7 +144,8 @@ final class PhabricatorProjectBurndownChartEngine )); $function->getFunctionLabel() - ->setName(pht('Tasks Closed')) + ->setKey('closed') + ->setName(pht('Closed Tasks')) ->setColor('rgba(0, 200, 0, 1)') ->setFillColor('rgba(0, 200, 0, 0.15)'); @@ -121,9 +154,14 @@ final class PhabricatorProjectBurndownChartEngine $datasets = array(); - $datasets[] = id(new PhabricatorChartStackedAreaDataset()) + $dataset = id(new PhabricatorChartStackedAreaDataset()) ->setFunctions($functions); + if ($stacks) { + $dataset->setStacks($stacks); + } + + $datasets[] = $dataset; $chart->attachDatasets($datasets); }