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.

Sign up for GitHub

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

Jump to bottom

Add option to set/unset dpi scaling for screens that are not high resolution #3551

Merged
merged 4 commits into from
Oct 24, 2016
Merged

Add option to set/unset dpi scaling for screens that are not high resolution #3551

merged 4 commits into from
Oct 24, 2016

Conversation

mariacamilarg
Copy link
Contributor

Fixes #3489

@mariacamilarg mariacamilarg added this to the v3.1 milestone Oct 18, 2016
@mariacamilarg mariacamilarg self-assigned this Oct 18, 2016
@mariacamilarg
Copy link
Contributor Author

This creates a new checkbox in preferences to enable/disable high DPI scaling. The default is disabled.

pr

ccordoba12
ccordoba12 reviewed Oct 18, 2016
View reviewed changes
spyder/app/mainwindow.py
#==============================================================================
from spyder.config.main import CONF
if CONF.get('main', 'high_dpi_scaling'):
has_high_dpi_scaling=True
Copy link
Member

@ccordoba12 ccordoba12 Oct 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Please name this variable as high_dpi_scaling

ccordoba12
ccordoba12 reviewed Oct 18, 2016
View reviewed changes
spyder/app/mainwindow.py
QCoreApplication.setAttribute(Qt.AA_EnableHighDpiScaling, True)

QCoreApplication.setAttribute(Qt.AA_EnableHighDpiScaling, has_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.

Remove this blank addition :-)

ccordoba12
ccordoba12 reviewed Oct 18, 2016
View reviewed changes
spyder/app/mainwindow.py
@@ -1171,7 +1177,7 @@ def add_xydoc(text, pathlist):
for child in self.menuBar().children():
if isinstance(child, QMenu) and child != self.help_menu:
child.setTearOffEnabled(True)

Copy link
Member

Choose a reason for hiding this comment

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

Also remove this blank

Copy link
Member

Choose a reason for hiding this comment

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

@mariacamilaremolinagutierrez, you forgot to remove this blank :-)

@ccordoba12
Copy link
Member

Great job!! This is exactly what I've done to fix this issue ;-)

I left some minor (and very simple) comments to solve :-)

@ccordoba12
Copy link
Member

@mariacamilaremolinagutierrez, please address my comments as soon as possible. I want to include this in 3.0.2 :-)

@ccordoba12 ccordoba12 modified the milestones: v3.0.2, v3.1 Oct 20, 2016
goanpeca
goanpeca reviewed Oct 20, 2016
View reviewed changes
spyder/app/mainwindow.py
@@ -261,6 +266,7 @@ def __init__(self, options=None):

qapp = QApplication.instance()
if PYQT5:
# Enabling scpaling for high dpi
Copy link
Member

Choose a reason for hiding this comment

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

Typo scaling vs. scpaling

spyder/app/mainwindow.py
@@ -261,6 +266,7 @@ def __init__(self, options=None):

qapp = QApplication.instance()
if PYQT5:
# Enabling scpaling for high dpi
qapp.setAttribute(Qt.AA_UseHighDpiPixmaps)
Copy link
Member

Choose a reason for hiding this comment

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

@ccordoba12 should we also affect this setting with the new option that was added, or it is really not needed?

@SylvainCorlay ?

Copy link
Member

Choose a reason for hiding this comment

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

I think the critical one is HighDpiScaling. No one has reported problems about this one :-)

Copy link
Member

Choose a reason for hiding this comment

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

ok

@ccordoba12 ccordoba12 changed the title this fixed the dpi auto scaling for screens that are not high resolution Add option to set/unset dpi scaling for screens that are not high resolution Oct 20, 2016
@goanpeca
Copy link
Member

goanpeca commented Oct 20, 2016 •
edited
Loading

@mariacamilaremolinagutierrez we normally work on forks of the repo, not on the repo itself.

https://github.com/spyder-ide/spyder/wiki/Dev%3A-Github-Workflow#Developers

@ccordoba12

@mariacamilarg
Copy link
Contributor Author

@goanpeca Hi Gonzalo, yes I knew about this after. I apologize, for the next one I'll fork the repo :)

ccordoba12
ccordoba12 reviewed Oct 21, 2016
View reviewed changes
spyder/app/mainwindow.py
if CONF.get('main', 'high_dpi_scaling'):
high_dpi_scaling=True
else:
high_dpi_scaling=False
Copy link
Member

Choose a reason for hiding this comment

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

@mariacamilaremolinagutierrez, please add spaces around equals, like this

high_dpi_scaling = True

Copy link
Member

Choose a reason for hiding this comment

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

ciocheck coming soon to the rescue!

@ccordoba12
Copy link
Member

@mariacamilaremolinagutierrez, please address my latest comments so I can merge this one :-)

@mariacamilarg
Copy link
Contributor Author

I don't understand why only that change make a test fail. I'm working on another issue, but as soon as I finish it I'll figure this out.

image

I don't fully understand the tests it is failing, but they are:

spyder/widgets/sourcecode/tests/test_autocolon.py::test_auto_colon_even_if_colon_inside_quotes xfail
spyder/widgets/sourcecode/tests/test_autocolon.py::test_no_auto_colon_in_listcomp_over_three_lines xfail
spyder/widgets/sourcecode/tests/test_autocolon.py::test_auto_colon_in_two_if_statements_on_one_line xfail
spyder/widgets/sourcecode/tests/test_autoindent.py::test_simple_def xfail
spyder/widgets/sourcecode/tests/test_autoindent.py::test_def_with_unindented_comment xfail
spyder/widgets/sourcecode/tests/test_autoindent.py::test_open_parenthesis xfail

@ccordoba12
Copy link
Member

Thanks @mariacamilaremolinagutierrez! Merging now :-)

@ccordoba12 ccordoba12 merged commit f09e70c into 3.x Oct 24, 2016
@ccordoba12 ccordoba12 deleted the fixes_issue_3489 branch October 24, 2016 00:25
ccordoba12 added a commit that referenced this pull request Oct 24, 2016
@ccordoba12
Merge from 3.x: PR #3551
f40b5af 
Fixes #3489
@jitseniesen
Copy link
Member

I don't fully understand the tests it is failing, but they are:

@mariacamilaremolinagutierrez In case you haven't figured it out yet: xfail indicates that it is a test that we expect to fail, and the test does indeed fail. So it's not a bad thing. We use it when we write tests for bugs that we haven't yet fixed. See http://doc.pytest.org/en/latest/skipping.html for more info.

@mariacamilarg
Copy link
Contributor Author

Thank you for the explanation @jitseniesen :)

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

@goanpeca goanpeca goanpeca left review comments

@ccordoba12 ccordoba12 ccordoba12 left review comments

Assignees

@mariacamilarg mariacamilarg

Labels
type:Bug User Interface (UI)
Projects
None yet
Milestone
v3.0.2
Development

Successfully merging this pull request may close these issues.
4 participants
@mariacamilarg @ccordoba12 @goanpeca @jitseniesen