From 5aa6c63e477a904e8a26befefd0728933619bc8a Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Mon, 28 Jan 2019 12:39:27 +0200 Subject: [PATCH 1/6] use same heigth for horizontal axes in step 4 thats used in steps 5/6 --- src/core/core.layouts.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/core/core.layouts.js b/src/core/core.layouts.js index d212731f7ef..098b8c66dce 100644 --- a/src/core/core.layouts.js +++ b/src/core/core.layouts.js @@ -171,7 +171,6 @@ module.exports = { // What we do to find the best sizing, we do the following // 1. Determine the minimum size of the chart area. // 2. Split the remaining width equally between each vertical axis - // 3. Split the remaining height equally between each horizontal axis // 4. Give each layout the maximum size it can be. The layout will return it's minimum size // 5. Adjust the sizes of each axis based on it's minimum reported size. // 6. Refit each axis @@ -183,13 +182,10 @@ module.exports = { var chartWidth = width - leftPadding - rightPadding; var chartHeight = height - topPadding - bottomPadding; var chartAreaWidth = chartWidth / 2; // min 50% - var chartAreaHeight = chartHeight / 2; // min 50% // Step 2 var verticalBoxWidth = (width - chartAreaWidth) / (leftBoxes.length + rightBoxes.length); - // Step 3 - var horizontalBoxHeight = (height - chartAreaHeight) / (topBoxes.length + bottomBoxes.length); // Step 4 var maxChartAreaWidth = chartWidth; @@ -201,7 +197,7 @@ module.exports = { var isHorizontal = box.isHorizontal(); if (isHorizontal) { - minSize = box.update(box.fullWidth ? chartWidth : maxChartAreaWidth, horizontalBoxHeight); + minSize = box.update(box.fullWidth ? chartWidth : maxChartAreaWidth, chartHeight / 2); maxChartAreaHeight -= minSize.height; } else { minSize = box.update(verticalBoxWidth, maxChartAreaHeight); From abe74a6df9b79dfbcdea8a060f475287f16c0bf9 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Mon, 28 Jan 2019 12:47:17 +0200 Subject: [PATCH 2/6] a little refactoring --- src/core/core.layouts.js | 164 ++++++++++++++++----------------------- 1 file changed, 66 insertions(+), 98 deletions(-) diff --git a/src/core/core.layouts.js b/src/core/core.layouts.js index 098b8c66dce..98844a84d8a 100644 --- a/src/core/core.layouts.js +++ b/src/core/core.layouts.js @@ -26,6 +26,33 @@ function sortByWeight(array, reverse) { }); } +function findMaxPadding(boxes) { + var maxPadding = {top: 0, left: 0, bottom: 0, right: 0}; + helpers.each(boxes, function(box) { + if (box.getPadding) { + var boxPadding = box.getPadding(); + maxPadding.left = Math.max(maxPadding.left, boxPadding.left); + maxPadding.right = Math.max(maxPadding.right, boxPadding.right); + maxPadding.top = Math.max(maxPadding.top, boxPadding.top); + maxPadding.bottom = Math.max(maxPadding.bottom, boxPadding.bottom); + } + }); + return maxPadding; +} + +function addHeightByPosition(boxes, size) { + helpers.each(boxes, function(box) { + size[box.position] += box.height; + }); +} + +function addWidthByPosition(boxes, size) { + helpers.each(boxes, function(box) { + size[box.position] += box.width; + }); +} + + defaults._set('global', { layout: { padding: { @@ -142,6 +169,10 @@ module.exports = { sortByWeight(topBoxes, true); sortByWeight(bottomBoxes, false); + var verticalBoxes = leftBoxes.concat(rightBoxes); + var horizontalBoxes = topBoxes.concat(bottomBoxes); + var outerBoxes = verticalBoxes.concat(horizontalBoxes); + // Essentially we now have any number of boxes on each of the 4 sides. // Our canvas looks like the following. // The areas L1 and L2 are the left axes. R1 is the right axis, T1 is the top axis and @@ -184,7 +215,7 @@ module.exports = { var chartAreaWidth = chartWidth / 2; // min 50% // Step 2 - var verticalBoxWidth = (width - chartAreaWidth) / (leftBoxes.length + rightBoxes.length); + var verticalBoxWidth = (width - chartAreaWidth) / verticalBoxes.length; // Step 4 @@ -211,37 +242,15 @@ module.exports = { }); } - helpers.each(leftBoxes.concat(rightBoxes, topBoxes, bottomBoxes), getMinimumBoxSize); + helpers.each(outerBoxes, getMinimumBoxSize); // If a horizontal box has padding, we move the left boxes over to avoid ugly charts (see issue #2478) - var maxHorizontalLeftPadding = 0; - var maxHorizontalRightPadding = 0; - var maxVerticalTopPadding = 0; - var maxVerticalBottomPadding = 0; - - helpers.each(topBoxes.concat(bottomBoxes), function(horizontalBox) { - if (horizontalBox.getPadding) { - var boxPadding = horizontalBox.getPadding(); - maxHorizontalLeftPadding = Math.max(maxHorizontalLeftPadding, boxPadding.left); - maxHorizontalRightPadding = Math.max(maxHorizontalRightPadding, boxPadding.right); - } - }); - - helpers.each(leftBoxes.concat(rightBoxes), function(verticalBox) { - if (verticalBox.getPadding) { - var boxPadding = verticalBox.getPadding(); - maxVerticalTopPadding = Math.max(maxVerticalTopPadding, boxPadding.top); - maxVerticalBottomPadding = Math.max(maxVerticalBottomPadding, boxPadding.bottom); - } - }); + var maxPadding = findMaxPadding(outerBoxes); // At this point, maxChartAreaHeight and maxChartAreaWidth are the size the chart area could // be if the axes are drawn at their minimum sizes. // Steps 5 & 6 - var totalLeftBoxesWidth = leftPadding; - var totalRightBoxesWidth = rightPadding; - var totalTopBoxesHeight = topPadding; - var totalBottomBoxesHeight = bottomPadding; + var outerBoxSizes = {top: topPadding, left: leftPadding, bottom: bottomPadding, right: rightPadding}; // Function to fit a box function fitBox(box) { @@ -250,10 +259,10 @@ module.exports = { }); if (minBoxSize) { - if (box.isHorizontal()) { + if (minBoxSize.horizontal) { var scaleMargin = { - left: Math.max(totalLeftBoxesWidth, maxHorizontalLeftPadding), - right: Math.max(totalRightBoxesWidth, maxHorizontalRightPadding), + left: Math.max(outerBoxSizes.left, maxPadding.left), + right: Math.max(outerBoxSizes.right, maxPadding.right), top: 0, bottom: 0 }; @@ -268,27 +277,12 @@ module.exports = { } // Update, and calculate the left and right margins for the horizontal boxes - helpers.each(leftBoxes.concat(rightBoxes), fitBox); - - helpers.each(leftBoxes, function(box) { - totalLeftBoxesWidth += box.width; - }); - - helpers.each(rightBoxes, function(box) { - totalRightBoxesWidth += box.width; - }); + helpers.each(verticalBoxes, fitBox); + addWidthByPosition(verticalBoxes, outerBoxSizes); // Set the Left and Right margins for the horizontal boxes - helpers.each(topBoxes.concat(bottomBoxes), fitBox); - - // Figure out how much margin is on the top and bottom of the vertical boxes - helpers.each(topBoxes, function(box) { - totalTopBoxesHeight += box.height; - }); - - helpers.each(bottomBoxes, function(box) { - totalBottomBoxesHeight += box.height; - }); + helpers.each(horizontalBoxes, fitBox); + addHeightByPosition(horizontalBoxes, outerBoxSizes); function finalFitVerticalBox(box) { var minBoxSize = helpers.findNextWhere(minBoxSizes, function(minSize) { @@ -298,8 +292,8 @@ module.exports = { var scaleMargin = { left: 0, right: 0, - top: totalTopBoxesHeight, - bottom: totalBottomBoxesHeight + top: outerBoxSizes.top, + bottom: outerBoxSizes.bottom }; if (minBoxSize) { @@ -308,60 +302,34 @@ module.exports = { } // Let the left layout know the final margin - helpers.each(leftBoxes.concat(rightBoxes), finalFitVerticalBox); + helpers.each(verticalBoxes, finalFitVerticalBox); // Recalculate because the size of each layout might have changed slightly due to the margins (label rotation for instance) - totalLeftBoxesWidth = leftPadding; - totalRightBoxesWidth = rightPadding; - totalTopBoxesHeight = topPadding; - totalBottomBoxesHeight = bottomPadding; - - helpers.each(leftBoxes, function(box) { - totalLeftBoxesWidth += box.width; - }); - - helpers.each(rightBoxes, function(box) { - totalRightBoxesWidth += box.width; - }); - - helpers.each(topBoxes, function(box) { - totalTopBoxesHeight += box.height; - }); - helpers.each(bottomBoxes, function(box) { - totalBottomBoxesHeight += box.height; - }); + outerBoxSizes = {top: topPadding, left: leftPadding, bottom: bottomPadding, right: rightPadding}; + addWidthByPosition(verticalBoxes, outerBoxSizes); + addHeightByPosition(horizontalBoxes, outerBoxSizes); // We may be adding some padding to account for rotated x axis labels - var leftPaddingAddition = Math.max(maxHorizontalLeftPadding - totalLeftBoxesWidth, 0); - totalLeftBoxesWidth += leftPaddingAddition; - totalRightBoxesWidth += Math.max(maxHorizontalRightPadding - totalRightBoxesWidth, 0); + var leftPaddingAddition = Math.max(maxPadding.left - outerBoxSizes.left, 0); + outerBoxSizes.left += leftPaddingAddition; + outerBoxSizes.right += Math.max(maxPadding.right - outerBoxSizes.right, 0); - var topPaddingAddition = Math.max(maxVerticalTopPadding - totalTopBoxesHeight, 0); - totalTopBoxesHeight += topPaddingAddition; - totalBottomBoxesHeight += Math.max(maxVerticalBottomPadding - totalBottomBoxesHeight, 0); + var topPaddingAddition = Math.max(maxPadding.top - outerBoxSizes.top, 0); + outerBoxSizes.top += topPaddingAddition; + outerBoxSizes.bottom += Math.max(maxPadding.bottom - outerBoxSizes.bottom, 0); // Figure out if our chart area changed. This would occur if the dataset layout label rotation // changed due to the application of the margins in step 6. Since we can only get bigger, this is safe to do // without calling `fit` again - var newMaxChartAreaHeight = height - totalTopBoxesHeight - totalBottomBoxesHeight; - var newMaxChartAreaWidth = width - totalLeftBoxesWidth - totalRightBoxesWidth; + var newMaxChartAreaHeight = height - outerBoxSizes.top - outerBoxSizes.bottom; + var newMaxChartAreaWidth = width - outerBoxSizes.left - outerBoxSizes.right; if (newMaxChartAreaWidth !== maxChartAreaWidth || newMaxChartAreaHeight !== maxChartAreaHeight) { - helpers.each(leftBoxes, function(box) { - box.height = newMaxChartAreaHeight; - }); - - helpers.each(rightBoxes, function(box) { + helpers.each(verticalBoxes, function(box) { box.height = newMaxChartAreaHeight; }); - helpers.each(topBoxes, function(box) { - if (!box.fullWidth) { - box.width = newMaxChartAreaWidth; - } - }); - - helpers.each(bottomBoxes, function(box) { + helpers.each(horizontalBoxes, function(box) { if (!box.fullWidth) { box.width = newMaxChartAreaWidth; } @@ -377,8 +345,8 @@ module.exports = { function placeBox(box) { if (box.isHorizontal()) { - box.left = box.fullWidth ? leftPadding : totalLeftBoxesWidth; - box.right = box.fullWidth ? width - rightPadding : totalLeftBoxesWidth + maxChartAreaWidth; + box.left = box.fullWidth ? leftPadding : outerBoxSizes.left; + box.right = box.fullWidth ? width - rightPadding : outerBoxSizes.left + maxChartAreaWidth; box.top = top; box.bottom = top + box.height; @@ -389,8 +357,8 @@ module.exports = { box.left = left; box.right = left + box.width; - box.top = totalTopBoxesHeight; - box.bottom = totalTopBoxesHeight + maxChartAreaHeight; + box.top = outerBoxSizes.top; + box.bottom = outerBoxSizes.top + maxChartAreaHeight; // Move to next point left = box.right; @@ -408,10 +376,10 @@ module.exports = { // Step 8 chart.chartArea = { - left: totalLeftBoxesWidth, - top: totalTopBoxesHeight, - right: totalLeftBoxesWidth + maxChartAreaWidth, - bottom: totalTopBoxesHeight + maxChartAreaHeight + left: outerBoxSizes.left, + top: outerBoxSizes.top, + right: outerBoxSizes.left + maxChartAreaWidth, + bottom: outerBoxSizes.top + maxChartAreaHeight }; // Step 9 From 53620452bfbf3d78742e4934a67fa089030ebf70 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 29 Jan 2019 13:18:27 +0200 Subject: [PATCH 3/6] renumbering, some minimization --- src/core/core.layouts.js | 52 +++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/src/core/core.layouts.js b/src/core/core.layouts.js index 98844a84d8a..8388a92e711 100644 --- a/src/core/core.layouts.js +++ b/src/core/core.layouts.js @@ -40,19 +40,12 @@ function findMaxPadding(boxes) { return maxPadding; } -function addHeightByPosition(boxes, size) { +function addSizeByPosition(boxes, size) { helpers.each(boxes, function(box) { - size[box.position] += box.height; + size[box.position] += box.isHorizontal() ? box.height : box.width; }); } -function addWidthByPosition(boxes, size) { - helpers.each(boxes, function(box) { - size[box.position] += box.width; - }); -} - - defaults._set('global', { layout: { padding: { @@ -202,12 +195,12 @@ module.exports = { // What we do to find the best sizing, we do the following // 1. Determine the minimum size of the chart area. // 2. Split the remaining width equally between each vertical axis - // 4. Give each layout the maximum size it can be. The layout will return it's minimum size - // 5. Adjust the sizes of each axis based on it's minimum reported size. - // 6. Refit each axis - // 7. Position each axis in the final location - // 8. Tell the chart the final location of the chart area - // 9. Tell any axes that overlay the chart area the positions of the chart area + // 3. Give each layout the maximum size it can be. The layout will return it's minimum size + // 4. Adjust the sizes of each axis based on it's minimum reported size. + // 5. Refit each axis + // 6. Position each axis in the final location + // 7. Tell the chart the final location of the chart area + // 8. Tell any axes that overlay the chart area the positions of the chart area // Step 1 var chartWidth = width - leftPadding - rightPadding; @@ -217,11 +210,12 @@ module.exports = { // Step 2 var verticalBoxWidth = (width - chartAreaWidth) / verticalBoxes.length; - - // Step 4 + // Step 3 var maxChartAreaWidth = chartWidth; var maxChartAreaHeight = chartHeight; + var outerBoxSizes = {top: topPadding, left: leftPadding, bottom: bottomPadding, right: rightPadding}; var minBoxSizes = []; + var maxPadding; function getMinimumBoxSize(box) { var minSize; @@ -237,7 +231,7 @@ module.exports = { minBoxSizes.push({ horizontal: isHorizontal, - minSize: minSize, + width: minSize.width, box: box, }); } @@ -245,12 +239,11 @@ module.exports = { helpers.each(outerBoxes, getMinimumBoxSize); // If a horizontal box has padding, we move the left boxes over to avoid ugly charts (see issue #2478) - var maxPadding = findMaxPadding(outerBoxes); + maxPadding = findMaxPadding(outerBoxes); // At this point, maxChartAreaHeight and maxChartAreaWidth are the size the chart area could // be if the axes are drawn at their minimum sizes. - // Steps 5 & 6 - var outerBoxSizes = {top: topPadding, left: leftPadding, bottom: bottomPadding, right: rightPadding}; + // Steps 4 & 5 // Function to fit a box function fitBox(box) { @@ -271,18 +264,18 @@ module.exports = { // on the margin. Sometimes they need to increase in size slightly box.update(box.fullWidth ? chartWidth : maxChartAreaWidth, chartHeight / 2, scaleMargin); } else { - box.update(minBoxSize.minSize.width, maxChartAreaHeight); + box.update(minBoxSize.width, maxChartAreaHeight); } } } // Update, and calculate the left and right margins for the horizontal boxes helpers.each(verticalBoxes, fitBox); - addWidthByPosition(verticalBoxes, outerBoxSizes); + addSizeByPosition(verticalBoxes, outerBoxSizes); // Set the Left and Right margins for the horizontal boxes helpers.each(horizontalBoxes, fitBox); - addHeightByPosition(horizontalBoxes, outerBoxSizes); + addSizeByPosition(horizontalBoxes, outerBoxSizes); function finalFitVerticalBox(box) { var minBoxSize = helpers.findNextWhere(minBoxSizes, function(minSize) { @@ -297,7 +290,7 @@ module.exports = { }; if (minBoxSize) { - box.update(minBoxSize.minSize.width, maxChartAreaHeight, scaleMargin); + box.update(minBoxSize.width, maxChartAreaHeight, scaleMargin); } } @@ -306,8 +299,7 @@ module.exports = { // Recalculate because the size of each layout might have changed slightly due to the margins (label rotation for instance) outerBoxSizes = {top: topPadding, left: leftPadding, bottom: bottomPadding, right: rightPadding}; - addWidthByPosition(verticalBoxes, outerBoxSizes); - addHeightByPosition(horizontalBoxes, outerBoxSizes); + addSizeByPosition(outerBoxes, outerBoxSizes); // We may be adding some padding to account for rotated x axis labels var leftPaddingAddition = Math.max(maxPadding.left - outerBoxSizes.left, 0); @@ -339,7 +331,7 @@ module.exports = { maxChartAreaWidth = newMaxChartAreaWidth; } - // Step 7 - Position the boxes + // Step 6 - Position the boxes var left = leftPadding + leftPaddingAddition; var top = topPadding + topPaddingAddition; @@ -374,7 +366,7 @@ module.exports = { helpers.each(rightBoxes, placeBox); helpers.each(bottomBoxes, placeBox); - // Step 8 + // Step 7 chart.chartArea = { left: outerBoxSizes.left, top: outerBoxSizes.top, @@ -382,7 +374,7 @@ module.exports = { bottom: outerBoxSizes.top + maxChartAreaHeight }; - // Step 9 + // Step 8 helpers.each(chartAreaBoxes, function(box) { box.left = chart.chartArea.left; box.top = chart.chartArea.top; From 0345c93440c9042c1f3c6b2d681d2096a3c382fd Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 29 Jan 2019 18:32:35 +0200 Subject: [PATCH 4/6] Better minification (and certainly performance) --- src/core/core.layouts.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/core/core.layouts.js b/src/core/core.layouts.js index 8388a92e711..2f0f314d7eb 100644 --- a/src/core/core.layouts.js +++ b/src/core/core.layouts.js @@ -27,17 +27,25 @@ function sortByWeight(array, reverse) { } function findMaxPadding(boxes) { - var maxPadding = {top: 0, left: 0, bottom: 0, right: 0}; + var top = 0; + var left = 0; + var bottom = 0; + var right = 0; helpers.each(boxes, function(box) { if (box.getPadding) { var boxPadding = box.getPadding(); - maxPadding.left = Math.max(maxPadding.left, boxPadding.left); - maxPadding.right = Math.max(maxPadding.right, boxPadding.right); - maxPadding.top = Math.max(maxPadding.top, boxPadding.top); - maxPadding.bottom = Math.max(maxPadding.bottom, boxPadding.bottom); + top = Math.max(top, boxPadding.top); + left = Math.max(left, boxPadding.left); + bottom = Math.max(bottom, boxPadding.bottom); + right = Math.max(right, boxPadding.right); } }); - return maxPadding; + return { + top: top, + left: left, + bottom: bottom, + right: right + }; } function addSizeByPosition(boxes, size) { From 075e61a7f68404fb8ea61922c9d248702a4262dc Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 29 Jan 2019 19:09:59 +0200 Subject: [PATCH 5/6] revert numbering, add a TODO in Step3 --- src/core/core.layouts.js | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/core/core.layouts.js b/src/core/core.layouts.js index 2f0f314d7eb..cfa137c96bc 100644 --- a/src/core/core.layouts.js +++ b/src/core/core.layouts.js @@ -203,12 +203,13 @@ module.exports = { // What we do to find the best sizing, we do the following // 1. Determine the minimum size of the chart area. // 2. Split the remaining width equally between each vertical axis - // 3. Give each layout the maximum size it can be. The layout will return it's minimum size - // 4. Adjust the sizes of each axis based on it's minimum reported size. - // 5. Refit each axis - // 6. Position each axis in the final location - // 7. Tell the chart the final location of the chart area - // 8. Tell any axes that overlay the chart area the positions of the chart area + // 3. Split the remaining height equally between each horizontal axis + // 4. Give each layout the maximum size it can be. The layout will return it's minimum size + // 5. Adjust the sizes of each axis based on it's minimum reported size. + // 6. Refit each axis + // 7. Position each axis in the final location + // 8. Tell the chart the final location of the chart area + // 9. Tell any axes that overlay the chart area the positions of the chart area // Step 1 var chartWidth = width - leftPadding - rightPadding; @@ -219,6 +220,10 @@ module.exports = { var verticalBoxWidth = (width - chartAreaWidth) / verticalBoxes.length; // Step 3 + // TODO re-limit horizontal axis height (this limit has affected only padding calculation since PR 1837) + // var horizontalBoxHeight = (height - chartAreaHeight) / (topBoxes.length + bottomBoxes.length); + + // Step 4 var maxChartAreaWidth = chartWidth; var maxChartAreaHeight = chartHeight; var outerBoxSizes = {top: topPadding, left: leftPadding, bottom: bottomPadding, right: rightPadding}; @@ -251,7 +256,7 @@ module.exports = { // At this point, maxChartAreaHeight and maxChartAreaWidth are the size the chart area could // be if the axes are drawn at their minimum sizes. - // Steps 4 & 5 + // Steps 5 & 6 // Function to fit a box function fitBox(box) { @@ -339,7 +344,7 @@ module.exports = { maxChartAreaWidth = newMaxChartAreaWidth; } - // Step 6 - Position the boxes + // Step 7 - Position the boxes var left = leftPadding + leftPaddingAddition; var top = topPadding + topPaddingAddition; @@ -374,7 +379,7 @@ module.exports = { helpers.each(rightBoxes, placeBox); helpers.each(bottomBoxes, placeBox); - // Step 7 + // Step 8 chart.chartArea = { left: outerBoxSizes.left, top: outerBoxSizes.top, @@ -382,7 +387,7 @@ module.exports = { bottom: outerBoxSizes.top + maxChartAreaHeight }; - // Step 8 + // Step 9 helpers.each(chartAreaBoxes, function(box) { box.left = chart.chartArea.left; box.top = chart.chartArea.top; From cd260d02c021b940729671c5c00225caaa97521c Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 29 Jan 2019 19:51:41 +0200 Subject: [PATCH 6/6] fix comment --- src/core/core.layouts.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/core.layouts.js b/src/core/core.layouts.js index cfa137c96bc..96887770b51 100644 --- a/src/core/core.layouts.js +++ b/src/core/core.layouts.js @@ -221,7 +221,7 @@ module.exports = { // Step 3 // TODO re-limit horizontal axis height (this limit has affected only padding calculation since PR 1837) - // var horizontalBoxHeight = (height - chartAreaHeight) / (topBoxes.length + bottomBoxes.length); + // var horizontalBoxHeight = (height - chartAreaHeight) / horizontalBoxes.length; // Step 4 var maxChartAreaWidth = chartWidth;