GeezerVison

@mgrojo @MKleusberg @lucydodo Hoping someone has time to review this?

Most of the testing has been happening in #2469, though we've been more "checking if the result is ok" rather than anyone reviewing the code so far. 😄

@justinclift Sorry. I want to do a review, but I don't have any display to test. 😢

@lucydodo No worries at all. 😄

As a data point, we've done testing on both macOS and Windows, so we know the result "works ok". But we need someone to look over the code itself and just make sure there aren't any obvious problems before we merge it. 😄

If this PR is accepted icons license must be updated with https://codefisher.org/pastel-svg/ one

That is an important point. Thanks for telling us.

What's the state of this pull request?

I tried to test it and I have a problem with it in my system. It seems to eat up all the available memory and never shows up. It shows these warnings:

qt.svg: link b hasn't been detected!
qt.svg: link b hasn't been detected!
qt.svg: link d hasn't been detected!
qt.svg: link d hasn't been detected!

Regarding the licensing of the icon set, the current set is "Creative Commons Attribution" and the new one is "Creative Commons Attribution NonCommercial Share Alike", which is more restrictive, although still usable. I'm not sure about the consequences, so I just mention it here.

Regarding the licensing of the icon set, the current set is "Creative Commons Attribution" and the new one is "Creative Commons Attribution NonCommercial Share Alike" ...

Oh, that's an important point! Personally, at first thought I don't think that change would have any real consequences for us. We're not a commercial project in any real sense (even with Patreon and similar). But we definitely can email the icon set author to ask, just to be sure. 😄

@mgrojo I compiled my SVG branch again on Windows and Linux with Qt 5.15 and everything looks fine.

On a related point, we're looking to get a HiDPI monitor for testing: #2570

@MKleusberg What should be done with this PR? 😄

Actually you really don't need a hidpi monitor for testing. (other than the program otherwise maybe not fitting on the monitor)
You only need QT_SCREEN_SCALE_FACTORS="1;1.5;2" which sets a list of scaling factors that is different for every monitor. (in this case 1 for the first, 1.5 for the second, and 2 for the third)
That way you can also very easily test how the program behaves when having to rescale while running and how it responds to fractional scaling

I've compiled again, and it runs without problems now, I don't know if it was an incorrect build or that the problem was before updating to Ubuntu 20.4.

I've used the environment variable, and it seems to work, although I don't know where I should pay attention.

In any case, I'm still concerned about the licensing issue. The icon-set license seems more restricted than ours, and the author requires a donation to get the SVG icons, and even then, they ask that you don't redistribute the whole icon set as SVG. Are we going to pay to get future icons? @sandman7920, have you paid to develop this PR?

@mgrojo Yes I have paid, also I can provide full iconset.

Is the license you're both talking about, the one here?

If so, it says (at this time anyway):

The Pastel SVG icon set Created by Michael Buckley is licensed under the

Creative Commons Attribution NonCommercial Share Alike 4.0 http://creativecommons.org/licenses/by-nc-sa/4.0/

However additionally I would kindly ask that:

1. For non commercial use a link back to the home page is very appreciated but in no way required.
2. If you put Pastel SVG package on another web host in modified form that you provide a link back to the
    Pastel SVG home page as part of the attribution. http://codefisher.org/pastel-svg/

For those that want to use the icons commercially, I ask that you first make a donation and then you may
use it under following license.

Creative Commons Attribution Share Alike 4.0 http://creativecommons.org/licenses/by-sa/4.0/ 

We can do the donation thing there too if it's useful. Our Patreon account generates reasonable funding for things like that. 😄

Is the license you're both talking about, the one here?

If so, it says (at this time anyway):

Yes, but it's also this:

Getting the SVG source files

All the of the above downloads only provide the PNG files that are generated from the SVG files that I created. If you would like to use the SVG files, I ask that you make a donation. Please see the donation page for more details. Once you have made the donation you will be able to download them from the Pastel SVG Downloads page.

We can do the donation thing there too if it's useful. Our Patreon account generates reasonable funding for things like that. 😄

Ok, then why not? It's better to have all the blessings if it's reasonable.

Ok, then why not? It's better to have all the blessings if it's reasonable.

Cool. Done. 😄

@codefisher Hopefully that's helpful. 😄

Please merge this sooner, if you can. Current set of icons makes the experience of using DB Browser on a hi-DPI screen quite unpleasant. You don't need to pay for that specific iconset, any SVG iconset will be a great improvement over the current low-res one.

Ok, let's merge this now. In fact, we've already made a donation to the author, so there is no doubt about the rights. If someone provides a valid alternative icon-set, we can analyse that proposal then, but after some searching I was unable to find a vectorial icon-set as good and publicly available as the original Silk icon-set.

Thanks!

There is some strange bug on windows with the system styles (windows and windowsvista).
When styleproxy is used and QDockWidget has a custom stylesheet applied to the widget title, close-button and float-button are rendered huge.

void TableBrowserDock::setFocusStyle(bool on)
{
    // Highlight title bar when dock widget is active
    if(on)
        setStyleSheet("QDockWidget::title {background:palette(highlight);}");
    else
        setStyleSheet(QString());
}

I have created a new series of patches with a workaround for windows and some other tweaks.

Linux has no such issue. Mac and Windows 7 are not tested.

@mgrojo should I make a new PR or more testing is needed?

Nightly

New

I have tested Windows 7 and Linux with -style Windows, the problem exists on all platforms with Windows-style applied.

If QProxyStyle is used and QDockWidget has a custom stylesheet on the title element bug exists.

void TableBrowserDock::setFocusStyle(bool on)
{
    // Highlight title bar when dock widget is active
    if(on)
        setStyleSheet("QDockWidget::title {background:palette(highlight);}");
    else
        setStyleSheet(QString());
}

workaround

class DB4SProxyStyle final : public QProxyStyle {
public:
    DB4SProxyStyle(int toolBarIconSize, qreal dpi, QStyle *style)
        : QProxyStyle(style), m_toolBarIconSize(toolBarIconSize * dpi / 96)
    {
#ifdef Q_OS_WIN
        m_smallIconSize = 11 * dpi / 96; // WINDOWS THEME BUG

        QStyleOption so(QStyleOption::Version, QStyleOption::SO_MenuItem);
        m_menuItemIconSize = style->pixelMetric(PM_SmallIconSize, &so);
#endif
    }

    int pixelMetric(QStyle::PixelMetric metric,
                    const QStyleOption *option = nullptr,
                    const QWidget *widget = nullptr) const override
    {
        if (metric == PM_ToolBarIconSize) {
            return m_toolBarIconSize;
        }
#ifdef Q_OS_WIN
        else if (metric == PM_SmallIconSize) {
            // set maximum icon size for SmallIcon

            // Set default MenuIcon size
            if (option && option->type == QStyleOption::SO_MenuItem) {
                return m_menuItemIconSize;
            }

            // if we don't return size before QDockWidgets are created,
            // any of the widgets with custom stylesheet are rendered with huge float,close buttons
            // see void TableBrowserDock::setFocusStyle
            return m_smallIconSize;
        }
#endif
        return QProxyStyle::pixelMetric(metric, option, widget);
    }

private:
    int m_toolBarIconSize;
#ifdef Q_OS_WIN
    int m_menuItemIconSize;
    int m_smallIconSize;
#endif
};

Also, in new patches toolBarIconSize calculation is based on screen->logicalDotsPerInch()

New QDockWidget::style

void TableBrowserDock::setFocusStyle(bool on)
{
    // Highlight title bar when dock widget is active
    if(on)
        setStyleSheet(QStringLiteral(
            "QDockWidget::title {"
                "background:palette(AlternateBase);"
                "text-align: center;"
                "border-bottom: 2px solid palette(highlight);"
            "}"));
    else
        setStyleSheet(QStringLiteral(
            "QDockWidget::title {"
                "text-align: center;"
            "}"));
}

Windows

KDE Breeze

KDE DB4S Dark Style

KDE DB4S Light Style

Is the active docked tab highlighted over the inactive ones?

Yes it's highlighted