library: right align BPM, duration & bitrate values

[ronso0]

Right align BPM, duration & bitrate so big/small values can easily be spotted by length (number of digits).

Also adds 'BPM precision' to the Library preferences to set the number of decimals for the BPM column.

[ronso0]

@uklotzde's point is still somewhat valid, though it's usefull only if all tracks are in the same BPM range (digits) and I think this should be solved otherwise, not by shrinking the column

I'm still unsure though, because for the BPM column I prefer to cut off the less significant fractional digits to the right.

[daschuer]

This is clear improvident when enough space is available.

Unfortunately it feels like a regression when the eliding with "..." kicks in on the right, because this still assumes left alignment.

I have checked how it is done with Excel. It does not use ... eliding. It cuts off the decimal places without any hint and replaces whole values with ### if they does no longer fit. This feels like a good solution for us as well.

The issue is that we have no info about the available space when composing the string for display. So a similar solution is probably quite hard to implement.

Maybe we can force real decimal point alignment by making all strings equal length via padding it with a spaces of number and punctuation width

1234567890
     11111 <- with 5 leading EN QUAD

https://www.fileformat.info/info/unicode/char/2000/index.htm

1:12:44
   1:34 <- EN QUAD + PUNCTUATION SPACE+ EN QUAD

https://www.fileformat.info/info/unicode/char/2008/index.htm

What do you think?

[ronso0]

I tried exactly that but didn't find a suitable whitespace char, yet. I'll test with en quad.

[ronso0]

1234567890
     11111 <- with 5 leading EN QUAD

these are not aligned, so either num chars are not equal in width for var. width fonts, or en quad is not the right choice.
Maybe these are reverted to regular whitespace here in the web GUI. I'll test it.

Thing is, since users can select the library font (incl. monospace) it has to work for any font.

[ronso0]

IMO we should avoid on-the-fly font meteics calculation in every BPMDelegate, I fear this has a performance impact when scrilling the library.
Maybe some pre-calculation can happen in setFont()

edit okay, QItemDelegate::drawDisplay already does quite some font metrics stuff, so overriding that might work.

[daschuer]

IMO we should avoid on-the-fly font meteics calculation in every BPMDelegate, I fear this has a performance impact when scrilling the library.

There must be a metric already that generates the ... conditionally.

I am more concerned about our personal time we waste on that.

How would elide left work in your eyes?

Regarding the fonts, there is even no guarantee that the numbers have equal width. So IMHO a solution that follows the normal font principals should be fine.

[daschuer]

FIGURE SPACE is our friend https://www.fileformat.info/info/unicode/char/2007/index.htm

this is equivalent to the digit width of fonts with fixed-width digits

1234567890
     11111 <- with 5 leading FIGURE SPACE
     11111 <- with 5 leading EN QUAD
     11111 <- with 5 leading EN SPACE

[ronso0]

FIGURE SPACE is our friend

okay, I'll try that.

How would elide left work in your eyes?

It wouldn't, because the purpose of right elide is to shrink the column in order to hide the decimal(s).

[daschuer]

It wouldn't, because the purpose of right elide is to shrink the column in order to hide the decimal(s).

That was an idea during the discussion, but in the 2.4 branch we have only one decimal point and when the ... are coming they eat up more then one decimal.

I am afraid we can't have both, a dynamic precision + right alignment. For sake of consistency, I can imagine these solutions:

• Right align, elide left.
• Left align with padding spaces, elide right.
• Left align like In Excel (real dynamic precision)

As far as I remember reducing the BPM precision to 1 was a compromise, so everyone may effort the space for it. The ... elide indicates when the column is unusable small and the tooltips can be used.

[ronso0]

FIGURE SPACE is not suitable because it's fixed, i.e. not adjusted per font

space equal to tabular width of a font
this is equivalent to the digit width of fonts with fixed-width digits

IMO the discussion about avaliable space and elide is moot. The ellipsis character is min. as wide as a digit so that's not an argument for shrinking to save horizontal space because the ellipsis hides information and hence doesn't save space.

I'm more for setting the BPM precision in Pref > Library via a static var like we set the search debug timeout.
This + right align allows to size the column as desired while showing the entire number.
see second last commit.

[daschuer]

Since FIGURE SPACE is part of the font, it should be exactly the width of a digit. Unfortunately some font designer forget to adjust it and that's why it only works for well established fonts. Does it not work for the fonts normally used in Mixxx?

IMO the discussion about avaliable space and elide is moot.

Yes, that is my conclusion as well. The elide ... Is almost like the Excel ### because the value can no longer be interpreted reliable.

Since the Excel like dynamic precision solution seems to be hard to implement, I am OK with the preferences option.
But do we really need it? Can't we live with one decimal place? That works for me. I see no use case for more digits.

Can we than use the elide on the left than?

[daschuer]

This is by the way the original QItemDelegate::drawDisplay function we need to reimplement for dynamic precision:
https://github.com/qt/qtbase/blob/f7c8ff511c30dc4310a72b3da4b4a345efe1fba0/src/widgets/itemviews/qitemdelegate.cpp#L579
A challenge :-)

[daschuer]

Ich habe mal aus interesse ChatGPT gefragt. Ich bin verblüfft:

#include <QItemDelegate>
#include <QPainter>
#include <QStyleOptionViewItem>
#include <QLocale>

class DecimalDelegate : public QItemDelegate
{
public:
    void drawDisplay(QPainter* painter, const QStyleOptionViewItem& option, const QRect& rect, const QString& text) const override
    {
        // Retrieve the decimal separator for the current locale
        QLocale locale;
        QString decimalSeparator = locale.decimalPoint();

        // Calculate the number of decimal places based on the available width
        QFontMetrics fontMetrics(option.font);
        QString displayText = text;

        int textWidth = fontMetrics.horizontalAdvance(displayText);
        int maxWidth = rect.width();

        if (textWidth > maxWidth) {
            int ellipsisWidth = fontMetrics.horizontalAdvance("...");
            int numChars = displayText.length();
            int visibleWidth = maxWidth - ellipsisWidth;

            for (int i = numChars - 1; i >= 0; --i) {
                if (fontMetrics.horizontalAdvance(displayText.left(i) + "...") <= visibleWidth) {
                    if (displayText[i] == decimalSeparator) {
                        // Only decimal places are truncated, remove the ellipsis
                        displayText = displayText.left(i);
                    } else {
                        // Truncate the text and append ellipsis
                        displayText = displayText.left(i) + "...";
                    }
                    break;
                }
            }
        }

        // Draw the display text with the calculated decimal places
        QStyleOptionViewItem newOption = option;
        newOption.displayAlignment = Qt::AlignRight | Qt::AlignVCenter;
        newOption.text = displayText;

        QItemDelegate::drawDisplay(painter, newOption, rect, displayText);
    }
};

To use this delegate, you can set it on your QAbstractItemView using the setItemDelegate function:

QAbstractItemView* view = ...; // Your QAbstractItemView instance
DecimalDelegate* delegate = new DecimalDelegate;
view->setItemDelegate(delegate);

[ronso0]

Does it not work for the fonts normally used in Mixxx?

It works for Ubuntu font, but I tried others and it doesn't. So no option.

Since the Excel like dynamic precision solution seems to be hard to implement, I am OK with the preferences option.
But do we really need it? Can't we live with one decimal place? That works for me. I see no use case for more digits.

I don't really need decimals, I prefer the integer part only for less noise. The current elide solution obviously only works for track views with 100+ BPM.
IIRC users used the 2+ decimals during mix preparation to spot uneven BPM for checking where beat detection was wrong. That's why I chose 10 as upper limit.

The default is 1 decimal as it is now. So nothing will change for those who don't care.

[ronso0]

Interesting, I'll check ChatGPT's "idea".

[ronso0]

The current solution works fine for me.

• adjust the decimals in the preferences
• on next paint, the table is updated
• double-click the right-hand BPM column splitter to auto-adjust
• done.

[daschuer]

Ok, this is a good step forward.

Just the preferences are not aligned with a stretched window:

[ronso0]

Yes, I need to fix that. For the other spinbox labels as well, and the spinboxes won't have more than 4 digits, so maybe the size policy / alignment can be improved.

[ronso0]

Alright, this is finished.

Please double-check the v-alignment of the right aligned columns is 'center.
The pref page should be fine now, incl. the scroll fix I already applied to comboboxes in DlgPrefSound.

[daschuer]

LGTM, Thank you.

[m0dB]

After pulling 2.4 I get

AutoUic: /Users/m0dB/mixxx/src/preferences/dialog/dlgpreflibrarydlg.ui: Warning: Tab-stop assignment: 'bpmPrecisionSpinBox' is not a valid widget.

Could that be because of this PR?

[ronso0]

jup, seems I forgot to rename that.
I'll push a fix to 2.4 this night