Skip to content
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

PR: Add an option to set a custom HiDPI scale factor #4454

Merged
merged 16 commits into from
Jun 1, 2017

Conversation

rlaverde
Copy link
Member

@rlaverde rlaverde commented May 10, 2017

Fixes: #4316

This is working, but It'll fail when setting variables that could be interpreted as int or floats (2, 2.5 but not 2;, 2.5;)

It'll be necessary to backport some changes from master (introduced by the multiple edgeline configuration)

  • The support for regex in config lineedit options. dc5d4a8
  • The change for preserving the type of preferences 0c9c97c and 5b26fe8

hidpi resolutpion

@rlaverde rlaverde added this to the v3.2 milestone May 10, 2017
@rlaverde rlaverde self-assigned this May 10, 2017
@rlaverde rlaverde requested review from ccordoba12 and goanpeca May 10, 2017 16:55
@goanpeca
Copy link
Member

goanpeca commented May 10, 2017

@ccordoba12 @rlaverde why dont use a single option + a QDoubleSpinBox? The default entry would be 1.0, why do we need an extra entry???? Also I would change to Enable high dpi scaling

Also the QDoubleSpinBox might return floats all the time?

screen shot 2017-05-10 at 12 14 33 pm

@rlaverde
Copy link
Member Author

Using a QDoubleSpinBox won't be necessary to backport the changes, but it won't allow configuration for multiple screens.

And I think that the differentiation between auto-scaling and custom-scalling is necessary (3 options)

@ccordoba12
Copy link
Member

ccordoba12 commented May 10, 2017

It'll be necessary to backport some changes from master (introduced by the multiple edgeline configuration)

Ok, please backport the corresponding changes. They don't seem too disruptive, so we can have them in 3.2.

@rlaverde
Copy link
Member Author

Ok, please backport the corresponding changes

Done 😄

'high_dpi_scaling': False,
'high_dpi_custom_scale_factor': False,
'high_dpi_custom_scale_factors': '2;',
Copy link
Member

@ccordoba12 ccordoba12 May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put here a float number, e.g. 1.5 or 2.5, so that people understand that they can adjust the scale factor to anything they want.

Also remove the ; at the end (which doesn't make sense for a single screen).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A float number could cause glitches, as said in the second question of this blog

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but that's something for users to try. The point is showing them that setting float factors is a possibility (and users on issue #4316 already made use of that possibility).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is the colon really necessary? I'd prefer to remove it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it for "2.5", the ";" was necessary before backporting the changes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, great!! Then 2.5 (without colon) it is :-)

tip=_("Set this for high DPI displays"))

custom_scaling_radio = self.create_radiobutton(
_("Set a custom high DPI scaling"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tooltip needs to be more descriptive, i.e. we need to explain here that users can introduce non-integer scale factors and that they can also set factors for different screens, separated by colons.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add a link to Qt documentation that explains this, like http://doc.qt.io/qt-5/highdpi.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to do that in a tooltip ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the link in a label

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!!

@ccordoba12
Copy link
Member

I don't see here any adjustment for the contents of the Help plugin. @rlaverde, is that necessary or not?

@rlaverde
Copy link
Member Author

I added more information of how to set this settings.

spectacle d26270

I don't see here any adjustment for the contents of the Help plugin. @rlaverde, is that necessary or not?

I forget about this, I'll try to do it, but I don't know how to test it

@ccordoba12
Copy link
Member

I forget about this, I'll try to do it, but I don't know how to test it

Just set a custom scale factor and verify that the text in the Help plugin also scales. If not, you'll have to search for a solution on the web.

@rlaverde rlaverde force-pushed the hidpi-custom-scale-factor branch from 1c1dd88 to 92c833f Compare May 16, 2017 20:55
@ccordoba12
Copy link
Member

@rlaverde, please ping me if/when you solve the problem with Help.

@rlaverde
Copy link
Member Author

rlaverde commented May 16, 2017

@ccordoba12 I'm testing and that seems to be an issue with QWebEngineView (also happens with spyder-reports), the QT wiki advice to set Qt.AA_EnableHighDpiScaling to True, but doesn't mention QT_SCREEN_SCALE_FACTORS

I tested with a minimal QWebEngineView and It doesn't scale

@ccordoba12
Copy link
Member

What if we try to scale things using CSS or something like that?

@rlaverde
Copy link
Member Author

What if we try to scale things using CSS or something like that?

Maybe that will work, generate a CSS based in high_dpi_custom_scale_factors

@ccordoba12
Copy link
Member

Yes, exactly!

@rlaverde
Copy link
Member Author

@ccordoba12 I think I found the error with Help plugin, font was being assigned sending a point size to method setFontSize() that receive a pixel size.

@ccordoba12
Copy link
Member

Nice!! Could you post a new screenshot with a custom scale factor?

Also, changing screen resolution requires a restart, so please be sure to demand a restart for the newly introduced options here.

@rlaverde
Copy link
Member Author

Nice!! Could you post a new screenshot with a custom scale factor?

spectacle cs6068

@goanpeca
Copy link
Member

I will be test this in the afternoon, going out now.

@goanpeca goanpeca assigned goanpeca and unassigned rlaverde May 17, 2017
@ccordoba12
Copy link
Member

Ok, things look really good so far. The only thing missing is requiring a restart, and that would be it :-)

@rlaverde
Copy link
Member Author

The only thing missing is requiring a restart, and that would be it :-)

Done, although I don't find a easy way to manage the preferences as a group, and set the restart for each preference.

@ccordoba12
Copy link
Member

I'll leave @goanpeca the final word for this one.

@rlaverde rlaverde force-pushed the hidpi-custom-scale-factor branch from ed92f34 to ef8b639 Compare June 1, 2017 15:18
@ccordoba12 ccordoba12 changed the title PR: Add an option for a custom hidpi scale factor PR: Add an option to set a custom HiDpi scale factor Jun 1, 2017
@ccordoba12 ccordoba12 merged commit 5cfda77 into spyder-ide:3.x Jun 1, 2017
ccordoba12 added a commit that referenced this pull request Jun 1, 2017
@rlaverde rlaverde deleted the hidpi-custom-scale-factor branch June 1, 2017 16:16
@ccordoba12 ccordoba12 changed the title PR: Add an option to set a custom HiDpi scale factor PR: Add an option to set a custom HiDPI scale factor Jun 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants