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

[FIX] ScatterplotGraph: Use mapToView instead of mapRectFromParent #3571

Merged
merged 2 commits into from
Feb 15, 2019

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Feb 2, 2019

Issue

Fixes #2789.

Description of changes
  • To show selection rectangle, (pyqtgraph's) ViewBox.updateScaleBox is overloaded to use self.mapToView instead of self.childgroup.mapRectFromParent. This may not be equivalent if there are multiple viewboxes in the same view or something similarly complicated, but we don't have this.
  • To fix selection, (our) mouseDragEvent is modified in the same way
  • An unrelated fix: select_by_rectangle assumed that the top left corner is above and to the left of the bottom right. This was true only if the user dragged in this direction. select_by_rectangle now sorts the coordinates.

The PR doesn't pass the diff coverage threshold, but I won't write any tests because they would involve simulating user interaction with mouse, yet at the end there wouldn't be anything to actually test.

Includes
  • Code changes

@codecov
Copy link

codecov bot commented Feb 2, 2019

Codecov Report

Merging #3571 into master will decrease coverage by 0.01%.
The diff coverage is 13.33%.

@@            Coverage Diff             @@
##           master    #3571      +/-   ##
==========================================
- Coverage   83.97%   83.95%   -0.02%     
==========================================
  Files         370      370              
  Lines       66944    67005      +61     
==========================================
+ Hits        56213    56257      +44     
- Misses      10731    10748      +17

@janezd janezd force-pushed the scatterplot-selection-large-scale branch from de1fb96 to 28666d7 Compare February 2, 2019 16:52
@janezd janezd changed the title ScatterplotGraph: Use mapToView instead of mapRectFromParent [FIX] ScatterplotGraph: Use mapToView instead of mapRectFromParent Feb 2, 2019
Copy link
Contributor

@VesnaT VesnaT left a comment

Choose a reason for hiding this comment

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

The selection now works, but zoom does not. The mouseDragEvent should probably be modified for zooming as well.

@janezd
Copy link
Contributor Author

janezd commented Feb 11, 2019

@VesnaT, done. Please check (assuming CI tests pass).

Interestingly, pyqtgraph calls mapToView when showing the rectangle, which is why it was shown correctly while dragging, but then it calls the broken mapRectFromParent at ev.finish. This is why it sufficed to reimplement only a part of mouseDragEvent.

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.

Scatter Plot: selection is not working with very high feature values
2 participants