Skip to content

Fix qt5 mouse #8440

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

Merged
merged 6 commits into from
Apr 24, 2017
Merged

Fix qt5 mouse #8440

merged 6 commits into from
Apr 24, 2017

Conversation

tacaswell
Copy link
Member

attn @efiring

This is the last thing for 2.0.1 I believe.

@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 7, 2017
@tacaswell tacaswell added this to the 2.0.1 (next bug fix release) milestone Apr 7, 2017
@tacaswell tacaswell changed the base branch from master to v2.0.x April 7, 2017 03:42
@tacaswell tacaswell closed this Apr 7, 2017
@tacaswell tacaswell reopened this Apr 7, 2017
@QuLogic
Copy link
Member

QuLogic commented Apr 7, 2017

So this basically undoes #8144, but that's supposedly fixing a bug. Is there some difference in Qt5 versions being used or something?

@efiring
Copy link
Member

efiring commented Apr 7, 2017

This is what I am using when seeing the bug:

(test) efiring@manini2:~/currents/programs/pycurrents/scripts$ conda list pyqt
# packages in environment at /Users/efiring/anaconda/envs/test:
#
pyqt                      5.6.0                    py35_0

@QuLogic
Copy link
Member

QuLogic commented Apr 7, 2017

What about qt?

@efiring
Copy link
Member

efiring commented Apr 7, 2017

(test) efiring@manini2:~/currents/programs/pycurrents/scripts$ conda list | grep -i qt
ipython-qtconsole         4.0.1                    py35_0
pyqt                      5.6.0                    py35_0
qt                        5.6.0                         0
qtawesome                 0.3.3                    py35_0    conda-forge
qtconsole                 4.2.1                    py35_1
qtpy                      1.1.2                    py35_0

@tacaswell
Copy link
Member Author

@QuLogic I think the bug was in @astrofrog 's example code. Internally we need to use the the pixels the renderer thinks it has not the pixels qt thinks it has.

@tacaswell
Copy link
Member Author

attn @astrofrog Can you test this out with both qt5 and osx?

@tacaswell
Copy link
Member Author

I still suspect that there are a bunch of un-needed scaling factors in there (as we scale up one place and down in the other), but it is not clear exactly which pairs of them need to be pulled out.

@tacaswell
Copy link
Member Author

And I am working on a "high-dpi" style sheet.

@astrofrog
Copy link
Contributor

@tacaswell - can you mark me as a reviewer so that I don't lose track of this, and I'll review/test it on Monday

@tacaswell tacaswell requested a review from astrofrog April 8, 2017 23:40
@tacaswell tacaswell force-pushed the fix_qt5_mouse branch 2 times, most recently from 9e34497 to 5e869de Compare April 10, 2017 20:06
@efiring
Copy link
Member

efiring commented Apr 12, 2017

I deleted a comment about this hanging my terminal; it seems to have been caused by an ipython version update.

@efiring
Copy link
Member

efiring commented Apr 17, 2017

@tacaswell, your suggestion of adding back the block that was deleted at the end of cedaaae works fine; I think that with that change, this is ready for 2.0.1.

Do not over-size the icons
Set the figure size back to the current value to push the dpi (and
hence rendered size) change through.
If `_original_dpi` is tacked on to the figure instance by a backend
canvas use that for 'figure' dpi instead of the dpi scaled for the
screen.
@astrofrog
Copy link
Contributor

Sorry I dropped the ball on reviewing this, I won't have time to check it in the short term but I don't want to delay 2.0.1, so this is fine by me.

@tacaswell
Copy link
Member Author

@efiring Can you test this set of commits (I force-pushed).

There is one more thing I want to add (but might not be worth holding PR over) is to connect up the signal Qt provides when the dpi ratio changes.

@efiring
Copy link
Member

efiring commented Apr 18, 2017

In my quick check, it looks good and works perfectly. I tested saving to png via the gui, manual resizing, get_size_inches and set_size_inches, cursor readout, and zoom to rect.

@efiring efiring merged commit 7d9036c into matplotlib:v2.0.x Apr 24, 2017
@tacaswell tacaswell deleted the fix_qt5_mouse branch April 24, 2017 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants