New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Table definition high DPI scaling incorrect #1184

Open
shravan2x opened this Issue Oct 22, 2017 · 10 comments

Comments

Projects
None yet
5 participants
@shravan2x
Copy link

shravan2x commented Oct 22, 2017

Details for the issue

The table definition form has a bad high DPI scaling on 4k Windows 10.

image

I'm opening this issue because:

  • DB4S is crashing
  • DB4S has a bug
  • DB4S needs a feature
  • DB4S has another problem

I'm using DB4S on:

  • Windows: ( version: Windows 10 Build 16299.19 )
  • Linux: ( distro: ___ )
  • Mac OS: ( version: ___ )
  • Other: ___

I'm using DB4S version:

  • 3.10.1
  • 3.10.0
  • 3.9.1
  • Other: ___

I have also:

@karim

This comment has been minimized.

Copy link
Member

karim commented Oct 22, 2017

Can you add a new environment variable to your Windows 10? Set QT_AUTO_SCREEN_SCALE_FACTOR to 1 and see if it fixed your problem.

More information can be found at http://doc.qt.io/qt-5/highdpi.html#high-dpi-support-in-qt.

To set a new environment variable in Windows 10 you can check the accepted answer here.

@shravan2x

This comment has been minimized.

Copy link
Author

shravan2x commented Oct 23, 2017

This slightly improves it, but the issue still partially exists as seen in the "Not Null" field and now the table size appears to be too large, as shown by the small scrollbars.

image

Further, it would be nice to have DB Browser for SQLite set the settings internally since an environment variable will affect all running applications that use Qt.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 23, 2017

Ironically, I can't tell what's wrong when I look at those screenshots. 🙁 Likely because I'm not using high DPI monitors. 😄

With the environment variable setting, yeah that's just a diagnostic/workaround step to see if that had an effect first. 😄

It sounds like it has, so setting it internally might be worth doing at some point. But it doesn't seem like it's a complete working solution yet. We probably need to do some further investigation to figure out how to fix the problem before we decide what programming changes need to be done. 😄

@shravan2x

This comment has been minimized.

Copy link
Author

shravan2x commented Oct 23, 2017

@justinclift The first screenshot had a bad spacing between the "N-P-A-I" columns, and the second screenshot had larger scrollbars than necessary.

IMO, users are generally okay with fixing those themselves, but the problem with that is that the changes aren't stored and loaded when the form is brought up again. The same also happens with other dialogs like "Open Database" one, which does not store the last loaded location. That is a useful feature for when people often only work with a single file.

These are rather small improvements and I just made the issue to track the problem itself, I don't expect it to be fixed anytime soon.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 23, 2017

The same also happens with other dialogs like "Open Database" one, which does not store the last loaded location.

As a useful tip, that behaviour is changeable in the Preferences dialog. 😄 We used to get requests for various different (conflicting) behaviours, so we just ended up adding a few different approaches and allowing the user to pick whichever one seems best for them. In theory (!) you should find a setting that suits in the list already. 😄

@shravan2x

This comment has been minimized.

Copy link
Author

shravan2x commented Oct 24, 2017

Good to know, I just enabled it =)

@karim

This comment has been minimized.

Copy link
Member

karim commented Oct 24, 2017

As @justinclift already said, the environment variable was just a check to find the cause of the problem. It looks like Qt handles scaling differently on different operating systems.

I believe this can be done internally like you suggested, but we have to find out what need to be changed first. This can be as simple as setting QGuiApplication::setAttribute(Qt::AA_EnableHighDpiScaling); on the control, but I don't have any experience with Qt unfortunately. Also, I don't know if this will affect other things.

High DPI support in Qt was introduced in version 5.6 and reverting back to 5.5 should fix this I guess, but it is something we don't want to do, since it is no longer supported. We could upgrade to Qt 5.8, which should fix some of the bugs that were introduced in version 5.6, but it will break Win XP build. :)

The second screenshot you posted looks fine. It is exactly the same as mine, but I'm not using a high DPI monitor. The "Not null" field might be confusing since all the other fields are using abbreviations, like PK for Primary Key. I guess we could change "Not null" to NN to fix that.

@shravan2x

This comment has been minimized.

Copy link
Author

shravan2x commented Oct 24, 2017

The "Not null" field might be confusing since all the other fields are using abbreviations, like PK for Primary Key. I guess we could change "Not null" to NN to fix that.

This actually sounds like a fair plan. Why is the scrollbar so small though? i.e. it appears the table element is longer than it should be.

but it will break Win XP build. :)

Win XP? Is that a kind of fruit?

@chrisjlocke

This comment has been minimized.

Copy link
Contributor

chrisjlocke commented Oct 24, 2017

but it will break Win XP build

XP users have a version that works. Development and the fixing of bugs shouldn't be hampered by supporting an OS that should have been shot 13 years ago.

MKleusberg added a commit that referenced this issue Oct 29, 2017

Rename "Not null" column to "NN" in Edit Table dialog
The "Not null" text didn't fit in the column width anyway and this makes
it more consistent with the other constraint columns.

See issue #1184.
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Oct 29, 2017

A minor update here: at least the "Not null" text will be replaced by "NN" in the next build 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment