FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix qt5 mouse by tacaswell · Pull Request #8440 · matplotlib/matplotlib · GitHub

Repository navigation

Fix qt5 mouse - #8440

Merged
efiring merged 6 commits into
matplotlib:v2.0.xfrom
tacaswell:fix_qt5_mouse
Apr 24, 2017
Merged

efiring merged 6 commits into
matplotlib:v2.0.xfrom
tacaswell:fix_qt5_mouse

Conversation

Copy link
Copy Markdown
Member

attn @efiring

This is the last thing for 2.0.1 I believe.

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

QuLogic commented Apr 7, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

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

efiring commented Apr 7, 2017

Copy link
Copy Markdown
Member

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 commented Apr 7, 2017

Copy link
Copy Markdown
Member

What about qt?

efiring commented Apr 7, 2017

Copy link
Copy Markdown
Member
(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

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
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 requested a review from astrofrog April 8, 2017 23:40
tacaswell force-pushed the fix_qt5_mouse branch 2 times, most recently from 9e34497 to 5e869de Compare April 10, 2017 20:06

efiring commented Apr 12, 2017

Copy link
Copy Markdown
Member

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

efiring commented Apr 17, 2017

Copy link
Copy Markdown
Member

@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.

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
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 commented Apr 18, 2017

Copy link
Copy Markdown
Member

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 merged commit 7d9036c into matplotlib:v2.0.x Apr 24, 2017
tacaswell deleted the fix_qt5_mouse branch April 24, 2017 19:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL