Improved auto skipper

[benmccann]

There's a ticks.autoSkip feature (docs), which plots only a subset of ticks and labels to avoid overlapping labels.

We should make the following improvements to the auto skipper:

• Make it aware of major ticks. It should first attempt to include all major ticks. It should then attempt to include minor ticks in between. This should fix Time scale only aligns single point #4600
  • This is easier if we allow buildTicks to return ticks outside the min/max range. Then we can remove ticks outside the min/max range after calling buildTicks. This is very similar to how the time scale's generate returns ticks outside the min/max range and buildTicks removes ticks outside the range. But if we move the auto-pruning logic to happen in the auto-skipper instead of buildTicks then it'd make sense to move the min/max enforcement as well
  • Attempt to split major ticks into sub-divisions that make sense similar to time scale's interval. Can have different division for integer and time scales. These divisions could possibly be auto-generated by doing a prime factorization and then finding all ways to divide factors into two groups. E.g. the prime factorization of 24 is 2*2*2*3. You could choose the following subsets of factors: 2, 3, 2*2, 3*2, 2*2*2, 3*2*2, 3*2*2*2. A manual divisions table might look something like like:

    // key is number of minor units between major units
    // value is number of minor units to include between major units
    divisions: {
      "1000": [1000, 500, 200, 100, 50, 25, 20, 10, 5, 4, 2],
      "60": [60, 30, 12, 6, 4, 3, 2],
      "24": [24, 12, 8, 6, 4, 3, 2],
    }

• Use the auto skipper in the time scale
  • Remove autoSkip: false from the time scale (source)
  • source: 'auto' should generate a tick at every minor unit. generate should not attempt to do anything smart
  • generate should not generate more than 1 tick per pixel

[benmccann]

One thing to note is that we're looking for the longest text several places. E.g. in the time scale we have the following:

me._labelFormat = determineLabelFormat(me._timestamps.data);

determineLabelFormat is looking for the longest formatted piece of data, but this may not be optimal because formatting every timestamp is expensive and we're calculating it based on points that might not be shown since not all data points are ticks / labels.

We also use the longest text a few places in core.scale.js including calculateTickRotation, fit, and _autoSkip. Perhaps we can make the time scale use or populate core.scale's me.longestLabelWidth variable.

[nagix]

#5961 addresses the last issue by caching the longest label width for those methods.

[benmccann]

Yeah, I'm not sure all the sub-classes use it yet though. E.g. timescale has its own implementation

[nagix]

While determineLabelFormat is looking for the longest format, helpers.longestText that is called from several places in core.scale.js is measuring the pixel width of every label, which I guess is more expensive. It may be possible to optimize it for auto skipper as well.

calculateTickRotation, fit and _autoSkip are common among all the scales, so the optimization in helpers.longestText would be applied to any scale type.

[nagix]

Ok, I understand that it could be also expensive to convert timestamps to moment objects (or other time library objects) in determineLabelFormat.

[benmccann]

I'll send a fix to make determineLabelFormat more performant. I was thinking that it was trying to figure out how many ticks could fit (like _autoSkip), but that's not true. It's actually used only for the tooltip. I think I was getting it a bit confused with the time scale's generate and getLabelCapacity

[andig]

Does this PR still make sense? Seems the underlying issue is already fixed?