Clean up automargin pipeline a bit

[alexcjohnson]

@etpinard I based this branch off multicategory (including commits from this morning) because we were both working in axis drawing code.

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults - somehow it never caused explicit problems before, but with big enough changes in Plotly.react, ie #3255, it does.

It turned out the biggest challenge with this was rangesliders. The main thing I did to fix this was to combine rangeslider and axis automargins to happen together - note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

One baseline image, range_slider_rangemode, changed in this PR b2a4b76 - just a little bit, but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline. On this branch there's no such jump. Not quite sure what in this PR fixed it 😅 but it's fixed.

[alexcjohnson]

moved this into plots.linkSubplots, which is both a more obvious place for it and happens earlier (in supplyDefaults) so _mainSubplot is available sooner.

[etpinard]

Is moving findMainSubplot from lsInner to supplyDefaults necessary? I suspect this will make supplyDefaults for large splom traces significantly slower.

[alexcjohnson]

Something or other failed with it in lsInner, though I don't recall what. I'll take a look at 🐎 for large sploms, I'm sure there's a shortcut we can take there.

[etpinard]

I'm sure there's a shortcut we can take there.

Yeah, I would be nice to leave findMainSubplot in supplyDefaults and add a fullLayout._hasOnlyLargeSploms condition to bypass the loop-over-subplot-for-all-axes loops.

[alexcjohnson]

Large sploms usually avoid the subplot loop there - the first try based on _anchorAxis works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Anyway I added an extra collection of counteraxes and subplots per axis in f55e769, which means we don't have to search the full subplot list anymore here (and in fact Axes.getSubplots is completely unused within plotly.js, but we still have it in streambed ATM). So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

[etpinard]

works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Ha right, I remember making that happen 3 or 4 splom-perf PRs ago.

So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

Beautiful 🐎

[etpinard]

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults

This is fantastic 🎉

but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline.

Yeah, I agree. The current baseline appears wrong. 👌

[alexcjohnson]

note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

Working on this has uncovered some more bugs - will address those in a separate bugfix PR.

[etpinard]

💃 let's get this in #3300

[alexcjohnson]

@etpinard I based this branch off multicategory (including commits from this morning) because we were both working in axis drawing code.

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults - somehow it never caused explicit problems before, but with big enough changes in Plotly.react, ie #3255, it does.

It turned out the biggest challenge with this was rangesliders. The main thing I did to fix this was to combine rangeslider and axis automargins to happen together - note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

One baseline image, range_slider_rangemode, changed in this PR b2a4b76 - just a little bit, but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline. On this branch there's no such jump. Not quite sure what in this PR fixed it 😅 but it's fixed.

[alexcjohnson]

moved this into plots.linkSubplots, which is both a more obvious place for it and happens earlier (in supplyDefaults) so _mainSubplot is available sooner.

[etpinard]

Is moving findMainSubplot from lsInner to supplyDefaults necessary? I suspect this will make supplyDefaults for large splom traces significantly slower.

[alexcjohnson]

Something or other failed with it in lsInner, though I don't recall what. I'll take a look at 🐎 for large sploms, I'm sure there's a shortcut we can take there.

[etpinard]

I'm sure there's a shortcut we can take there.

Yeah, I would be nice to leave findMainSubplot in supplyDefaults and add a fullLayout._hasOnlyLargeSploms condition to bypass the loop-over-subplot-for-all-axes loops.

[alexcjohnson]

Large sploms usually avoid the subplot loop there - the first try based on _anchorAxis works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Anyway I added an extra collection of counteraxes and subplots per axis in f55e769, which means we don't have to search the full subplot list anymore here (and in fact Axes.getSubplots is completely unused within plotly.js, but we still have it in streambed ATM). So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

[etpinard]

works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Ha right, I remember making that happen 3 or 4 splom-perf PRs ago.

So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

Beautiful 🐎

[etpinard]

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults

This is fantastic 🎉

but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline.

Yeah, I agree. The current baseline appears wrong. 👌

[alexcjohnson]

note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

Working on this has uncovered some more bugs - will address those in a separate bugfix PR.

[etpinard]

💃 let's get this in #3300

[alexcjohnson]

@etpinard I based this branch off multicategory (including commits from this morning) because we were both working in axis drawing code.

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults - somehow it never caused explicit problems before, but with big enough changes in Plotly.react, ie #3255, it does.

It turned out the biggest challenge with this was rangesliders. The main thing I did to fix this was to combine rangeslider and axis automargins to happen together - note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

One baseline image, range_slider_rangemode, changed in this PR b2a4b76 - just a little bit, but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline. On this branch there's no such jump. Not quite sure what in this PR fixed it 😅 but it's fixed.

[alexcjohnson]

moved this into plots.linkSubplots, which is both a more obvious place for it and happens earlier (in supplyDefaults) so _mainSubplot is available sooner.

[etpinard]

Is moving findMainSubplot from lsInner to supplyDefaults necessary? I suspect this will make supplyDefaults for large splom traces significantly slower.

[alexcjohnson]

Something or other failed with it in lsInner, though I don't recall what. I'll take a look at 🐎 for large sploms, I'm sure there's a shortcut we can take there.

[etpinard]

I'm sure there's a shortcut we can take there.

Yeah, I would be nice to leave findMainSubplot in supplyDefaults and add a fullLayout._hasOnlyLargeSploms condition to bypass the loop-over-subplot-for-all-axes loops.

[alexcjohnson]

Large sploms usually avoid the subplot loop there - the first try based on _anchorAxis works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Anyway I added an extra collection of counteraxes and subplots per axis in f55e769, which means we don't have to search the full subplot list anymore here (and in fact Axes.getSubplots is completely unused within plotly.js, but we still have it in streambed ATM). So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

[etpinard]

works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Ha right, I remember making that happen 3 or 4 splom-perf PRs ago.

So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

Beautiful 🐎

[etpinard]

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults

This is fantastic 🎉

but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline.

Yeah, I agree. The current baseline appears wrong. 👌

[alexcjohnson]

note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

Working on this has uncovered some more bugs - will address those in a separate bugfix PR.

[etpinard]

💃 let's get this in #3300

[alexcjohnson]

@etpinard I based this branch off multicategory (including commits from this morning) because we were both working in axis drawing code.

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults - somehow it never caused explicit problems before, but with big enough changes in Plotly.react, ie #3255, it does.

It turned out the biggest challenge with this was rangesliders. The main thing I did to fix this was to combine rangeslider and axis automargins to happen together - note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

One baseline image, range_slider_rangemode, changed in this PR b2a4b76 - just a little bit, but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline. On this branch there's no such jump. Not quite sure what in this PR fixed it 😅 but it's fixed.

[alexcjohnson]

moved this into plots.linkSubplots, which is both a more obvious place for it and happens earlier (in supplyDefaults) so _mainSubplot is available sooner.

[etpinard]

Is moving findMainSubplot from lsInner to supplyDefaults necessary? I suspect this will make supplyDefaults for large splom traces significantly slower.

[alexcjohnson]

Something or other failed with it in lsInner, though I don't recall what. I'll take a look at 🐎 for large sploms, I'm sure there's a shortcut we can take there.

[etpinard]

I'm sure there's a shortcut we can take there.

Yeah, I would be nice to leave findMainSubplot in supplyDefaults and add a fullLayout._hasOnlyLargeSploms condition to bypass the loop-over-subplot-for-all-axes loops.

[alexcjohnson]

Large sploms usually avoid the subplot loop there - the first try based on _anchorAxis works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Anyway I added an extra collection of counteraxes and subplots per axis in f55e769, which means we don't have to search the full subplot list anymore here (and in fact Axes.getSubplots is completely unused within plotly.js, but we still have it in streambed ATM). So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

[etpinard]

works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Ha right, I remember making that happen 3 or 4 splom-perf PRs ago.

So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

Beautiful 🐎

[etpinard]

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults

This is fantastic 🎉

but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline.

Yeah, I agree. The current baseline appears wrong. 👌

[alexcjohnson]

note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

Working on this has uncovered some more bugs - will address those in a separate bugfix PR.

[etpinard]

💃 let's get this in #3300

[alexcjohnson]

@etpinard I based this branch off multicategory (including commits from this morning) because we were both working in axis drawing code.

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults - somehow it never caused explicit problems before, but with big enough changes in Plotly.react, ie #3255, it does.

It turned out the biggest challenge with this was rangesliders. The main thing I did to fix this was to combine rangeslider and axis automargins to happen together - note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

One baseline image, range_slider_rangemode, changed in this PR b2a4b76 - just a little bit, but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline. On this branch there's no such jump. Not quite sure what in this PR fixed it 😅 but it's fixed.

[alexcjohnson]

moved this into plots.linkSubplots, which is both a more obvious place for it and happens earlier (in supplyDefaults) so _mainSubplot is available sooner.

[etpinard]

Is moving findMainSubplot from lsInner to supplyDefaults necessary? I suspect this will make supplyDefaults for large splom traces significantly slower.

[alexcjohnson]

Something or other failed with it in lsInner, though I don't recall what. I'll take a look at 🐎 for large sploms, I'm sure there's a shortcut we can take there.

[etpinard]

I'm sure there's a shortcut we can take there.

Yeah, I would be nice to leave findMainSubplot in supplyDefaults and add a fullLayout._hasOnlyLargeSploms condition to bypass the loop-over-subplot-for-all-axes loops.

[alexcjohnson]

Large sploms usually avoid the subplot loop there - the first try based on _anchorAxis works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Anyway I added an extra collection of counteraxes and subplots per axis in f55e769, which means we don't have to search the full subplot list anymore here (and in fact Axes.getSubplots is completely unused within plotly.js, but we still have it in streambed ATM). So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

[etpinard]

works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Ha right, I remember making that happen 3 or 4 splom-perf PRs ago.

So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

Beautiful 🐎

[etpinard]

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults

This is fantastic 🎉

but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline.

Yeah, I agree. The current baseline appears wrong. 👌

[alexcjohnson]

note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

Working on this has uncovered some more bugs - will address those in a separate bugfix PR.

[etpinard]

💃 let's get this in #3300

[alexcjohnson]

@etpinard I based this branch off multicategory (including commits from this morning) because we were both working in axis drawing code.

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults - somehow it never caused explicit problems before, but with big enough changes in Plotly.react, ie #3255, it does.

It turned out the biggest challenge with this was rangesliders. The main thing I did to fix this was to combine rangeslider and axis automargins to happen together - note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

One baseline image, range_slider_rangemode, changed in this PR b2a4b76 - just a little bit, but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline. On this branch there's no such jump. Not quite sure what in this PR fixed it 😅 but it's fixed.

[alexcjohnson]

moved this into plots.linkSubplots, which is both a more obvious place for it and happens earlier (in supplyDefaults) so _mainSubplot is available sooner.

[etpinard]

Is moving findMainSubplot from lsInner to supplyDefaults necessary? I suspect this will make supplyDefaults for large splom traces significantly slower.

[alexcjohnson]

Something or other failed with it in lsInner, though I don't recall what. I'll take a look at 🐎 for large sploms, I'm sure there's a shortcut we can take there.

[etpinard]

I'm sure there's a shortcut we can take there.

Yeah, I would be nice to leave findMainSubplot in supplyDefaults and add a fullLayout._hasOnlyLargeSploms condition to bypass the loop-over-subplot-for-all-axes loops.

[alexcjohnson]

Large sploms usually avoid the subplot loop there - the first try based on _anchorAxis works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Anyway I added an extra collection of counteraxes and subplots per axis in f55e769, which means we don't have to search the full subplot list anymore here (and in fact Axes.getSubplots is completely unused within plotly.js, but we still have it in streambed ATM). So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

[etpinard]

works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Ha right, I remember making that happen 3 or 4 splom-perf PRs ago.

So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

Beautiful 🐎

[etpinard]

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults

This is fantastic 🎉

but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline.

Yeah, I agree. The current baseline appears wrong. 👌

[alexcjohnson]

note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

Working on this has uncovered some more bugs - will address those in a separate bugfix PR.

[etpinard]

💃 let's get this in #3300

[alexcjohnson]

@etpinard I based this branch off multicategory (including commits from this morning) because we were both working in axis drawing code.

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults - somehow it never caused explicit problems before, but with big enough changes in Plotly.react, ie #3255, it does.

It turned out the biggest challenge with this was rangesliders. The main thing I did to fix this was to combine rangeslider and axis automargins to happen together - note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

One baseline image, range_slider_rangemode, changed in this PR b2a4b76 - just a little bit, but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline. On this branch there's no such jump. Not quite sure what in this PR fixed it 😅 but it's fixed.

[alexcjohnson]

moved this into plots.linkSubplots, which is both a more obvious place for it and happens earlier (in supplyDefaults) so _mainSubplot is available sooner.

[etpinard]

Is moving findMainSubplot from lsInner to supplyDefaults necessary? I suspect this will make supplyDefaults for large splom traces significantly slower.

[alexcjohnson]

Something or other failed with it in lsInner, though I don't recall what. I'll take a look at 🐎 for large sploms, I'm sure there's a shortcut we can take there.

[etpinard]

I'm sure there's a shortcut we can take there.

Yeah, I would be nice to leave findMainSubplot in supplyDefaults and add a fullLayout._hasOnlyLargeSploms condition to bypass the loop-over-subplot-for-all-axes loops.

[alexcjohnson]

Large sploms usually avoid the subplot loop there - the first try based on _anchorAxis works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Anyway I added an extra collection of counteraxes and subplots per axis in f55e769, which means we don't have to search the full subplot list anymore here (and in fact Axes.getSubplots is completely unused within plotly.js, but we still have it in streambed ATM). So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

[etpinard]

works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Ha right, I remember making that happen 3 or 4 splom-perf PRs ago.

So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

Beautiful 🐎

[etpinard]

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults

This is fantastic 🎉

but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline.

Yeah, I agree. The current baseline appears wrong. 👌

[alexcjohnson]

note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

Working on this has uncovered some more bugs - will address those in a separate bugfix PR.

[etpinard]

💃 let's get this in #3300

[alexcjohnson]

@etpinard I based this branch off multicategory (including commits from this morning) because we were both working in axis drawing code.

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults - somehow it never caused explicit problems before, but with big enough changes in Plotly.react, ie #3255, it does.

It turned out the biggest challenge with this was rangesliders. The main thing I did to fix this was to combine rangeslider and axis automargins to happen together - note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

One baseline image, range_slider_rangemode, changed in this PR b2a4b76 - just a little bit, but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline. On this branch there's no such jump. Not quite sure what in this PR fixed it 😅 but it's fixed.

[alexcjohnson]

moved this into plots.linkSubplots, which is both a more obvious place for it and happens earlier (in supplyDefaults) so _mainSubplot is available sooner.

[etpinard]

Is moving findMainSubplot from lsInner to supplyDefaults necessary? I suspect this will make supplyDefaults for large splom traces significantly slower.

[alexcjohnson]

Something or other failed with it in lsInner, though I don't recall what. I'll take a look at 🐎 for large sploms, I'm sure there's a shortcut we can take there.

[etpinard]

I'm sure there's a shortcut we can take there.

Yeah, I would be nice to leave findMainSubplot in supplyDefaults and add a fullLayout._hasOnlyLargeSploms condition to bypass the loop-over-subplot-for-all-axes loops.

[alexcjohnson]

Large sploms usually avoid the subplot loop there - the first try based on _anchorAxis works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Anyway I added an extra collection of counteraxes and subplots per axis in f55e769, which means we don't have to search the full subplot list anymore here (and in fact Axes.getSubplots is completely unused within plotly.js, but we still have it in streambed ATM). So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

[etpinard]

works unless the labels are unanchored - upper half with lower labels, or both halves with no diagonal.

Ha right, I remember making that happen 3 or 4 splom-perf PRs ago.

So even when splom does go through this loop, it now only takes 0.3ms at 20 dimensions - ~2ms at 50 dimensions if it's O(n^2).

Beautiful 🐎

[etpinard]

The main goal was to get plots.doAutoMargin out of supplyDefaults - because that can lead to a full redraw of the plot if margins have changed, which is completely inappropriate for supplyDefaults

This is fantastic 🎉

but I believe it was in fact wrong before, as you can see by interacting with this mock on master: start dragging the range and the box expands vertically a little bit, to match what's in the new baseline.

Yeah, I agree. The current baseline appears wrong. 👌

[alexcjohnson]

note though that in principle both can contribute independently to the margins, if you have a rangeslider (on the bottom) and a top axis with long labels (oh right, I meant to add a test for that case - I'll add that)

Working on this has uncovered some more bugs - will address those in a separate bugfix PR.

[etpinard]

💃 let's get this in #3300