Fix GraphicsScene.itemsNearEvent and setClickRadius by bbc131 · Pull Request #2383 · pyqtgraph/pyqtgraph

bbc131 · 2022-08-02T13:02:44Z

There is an issue with the mapping of mouse cursor position to the items which shall get mouse events.
It's about the case, when the cursor position is not directly intersecting with the item, but is quite near to it.
For the InfiniteLine there exists some code, solving this, which I would say is rather a workaround. But actually, this should and can be solved within GraphicsScene.itemsNearEvent().
See also GraphicsScene.setClickRadius(), which doesn't have any effect currently.

MWE with description of resulting problems

import pyqtgraph as pg

app = pg.mkQApp()
plot = pg.PlotWidget()
plot.show()

plot.setXRange(min=-1.0, max=+1.0)

plot.scene().setClickRadius(20)  # Has no effect

region1 = pg.LinearRegionItem()
region1.setRegion((0.1, 0.13))
region1.setBrush(pg.mkBrush("blue"))
region1.setHoverBrush(pg.mkBrush("magenta"))
for line in region1.lines:
    line.setPen(pg.mkPen(color="yellow", width=2))
    line.setHoverPen(pg.mkPen("#00ff00", width=2))
plot.addItem(region1)

region2 = pg.LinearRegionItem()
region2.setRegion((0.5, 0.502))
region2.setBrush(pg.mkBrush("red"))
region2.setHoverBrush(pg.mkBrush("magenta"))
for line in region2.lines:
    line.setVisible(False)
plot.addItem(region2)

if __name__ == '__main__':
    pg.exec()

If you run the mwe, the blue region1 should have a width of around 10 pixel and the red region2 should appear with a width of 1 pixel. This works for me with the default window size and zoom.
In this case I see the following two problems:

Moving the blue region1 via mouse dragging is impossible, because it's too small. Either the left or the right line gets hovered (and dragged if clicked), but never the area in between. But, I would say this should be possible for an area which is roughly 6 px wide.
Moving the red region2 via mouse dragging is impossible neither works the hovering.

Reason

GraphicsScene.itemsNearEvent() returns only the items which directly intersect with the position of the event, e.g. the mouse cursor position in this case. See also the docstring, which describes the desired behaviour in my opinion.
Usually, this problem isn't observed, since for the InfiniteLine, there exists a workaround, which simply makes the bounding rect a 4 px wide line, see InfiniteLine.py line 304 and 310 (https://github.com/pyqtgraph/pyqtgraph/blob/8be2d6a88edc1e26b1f6f85c47a72c400db9a28b/pyqtgraph/graphicsItems/InfiniteLine.py#L304,L310).

Resolution

In the present PR, I removed the workaround in InfiniteLine.py and reworked GraphicsScene.itemsNearEvent() and reactivated commented code, such that the click radius is considered and the behaviour is as described in the docstring.

* Correctly consider the click radius when mapping position to items * Remove workaround for InfiniteLine * Adjust test

j9ac9k · 2022-08-05T05:11:09Z

Hi @bbc131

Thanks for the PR and thanks for tackling this issue.

Looks like something in the code diff might be triggering some kind of race condition that some of the tests are failing, it's been a while since I've seen this kind of random assortment of OS/bindings/python-versions failing... I haven't run the test suite locally, have you been able to replicate any kind of test failure on your system?

bbc131 · 2022-08-22T13:07:48Z

... have you been able to replicate any kind of test failure on your system?

I cannot reproduce this behaviour.
I'm using Win10, python 3.10, PySide2

j9ac9k · 2022-08-22T14:21:32Z

Looks like all the windows jobs are passing; I'll try and spend some time today attempting to reproduce this on macOS.

j9ac9k · 2022-08-22T14:43:17Z

Follow up, I can reproduce the issue on macOS with pyqt5 5.15.7 and python 3.9 (likely can on other environments too). I'll work on debugging this a bit since you can't reproduce @bbc131

j9ac9k · 2022-08-22T15:21:32Z

tests/graphicsItems/test_InfiniteLine.py

    px = pg.Point(-0.5, -1.0 / 3**0.5)
-    assert br.containsPoint(pos + 5 * px, QtCore.Qt.FillRule.OddEvenFill)
-    assert not br.containsPoint(pos + 7 * px, QtCore.Qt.FillRule.OddEvenFill)
+    assert br.containsPoint(pos + 2 * px, QtCore.Qt.FillRule.OddEvenFill)


When I change the 2 to a 1, this check passes on my machine; not sure if that is the appropriate test tho.

bbc131 · 2022-08-23T06:01:55Z

Regarding the test case which fails here at the 3rd assert:

    # test bounding rect for oblique line
    br = oline.mapToScene(oline.boundingRect())
    pos = oline.mapToScene(pg.Point(2, 0))
    assert br.containsPoint(pos, QtCore.Qt.FillRule.OddEvenFill)
    px = pg.Point(-0.5, -1.0 / 3**0.5)
    assert br.containsPoint(pos + 2 * px, QtCore.Qt.FillRule.OddEvenFill)
    assert not br.containsPoint(pos + 3 * px, QtCore.Qt.FillRule.OddEvenFill)

I think, that my choice of the values 2 and 3 is not correct, but I do not understand this testcase. The values before (5 and 7) made also no sense to me, so I chose this values by trial and error.
My guess is, that the vector px shall be orthogonal to oline, so that one can test how wide the bounding rect is. But with an angle of 30° and an x = 0.5, I get y = 3**0.5 / 2 instead of 1.0 / 3**0.5.
I also couldn't manage to draw this points in the plot, to see if my guess is correct, maybe you know how to do this.

j9ac9k · 2022-08-23T15:57:42Z

I'm inclined to merge this, I'd like @ixjlyons to give a sanity check as he has worked w/ weird boundingRect issues of InfiniteLine before.

ixjlyons · 2022-09-04T01:18:30Z

This looks good to me.

I'm not really alarmed by the InfiniteLine test changes - there's some weirdness around InfiniteLine when angle % 90 != 0 that I never quite figured out (#1878). I can't say I completely follow that portion of the test either, but I think (2, 0) in item coordinates is aligned with the line, not orthogonal to it. It does appear to be checking at least in part the padding of the bounding rect, though, and maybe it doesn't make sense to do that anymore anyway. The bounding rect checks on the vertical and horizontal lines are simpler checks of points either on the lines or far from them.

j9ac9k · 2022-09-04T03:00:38Z

Thanks for reviewing this @ixjlyons and thanks for the submission @bbc131 Sorry it took me so long to get this merged.

* Fix GraphicsScene.itemsNearEvent and setClickRadius * Correctly consider the click radius when mapping position to items * Remove workaround for InfiniteLine * Adjust test * Try to fix test_InfiniteLine

Fix GraphicsScene.itemsNearEvent and setClickRadius

376f1b2

* Correctly consider the click radius when mapping position to items * Remove workaround for InfiniteLine * Adjust test

j9ac9k reviewed Aug 22, 2022

View reviewed changes

Try to fix test_InfiniteLine

aeaee5e

j9ac9k merged commit ab5ec85 into pyqtgraph:master Sep 4, 2022

ixjlyons mentioned this pull request Sep 4, 2022

Fix InfiniteLine bounding rect calculation #2407

Merged

ixjlyons mentioned this pull request Sep 16, 2022

movable InfiniteLine grabbing does not respect the bounding rect #2416

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix GraphicsScene.itemsNearEvent and setClickRadius#2383

Fix GraphicsScene.itemsNearEvent and setClickRadius#2383
j9ac9k merged 2 commits intopyqtgraph:masterfrom
bbc131:itemsNearEvent

bbc131 commented Aug 2, 2022

Uh oh!

j9ac9k commented Aug 5, 2022

Uh oh!

bbc131 commented Aug 22, 2022

Uh oh!

j9ac9k commented Aug 22, 2022

Uh oh!

j9ac9k commented Aug 22, 2022

Uh oh!

j9ac9k Aug 22, 2022

Uh oh!

bbc131 commented Aug 23, 2022 •

edited

Loading

Uh oh!

j9ac9k commented Aug 23, 2022

Uh oh!

ixjlyons commented Sep 4, 2022

Uh oh!

j9ac9k commented Sep 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

bbc131 commented Aug 2, 2022

MWE with description of resulting problems

Reason

Resolution

Uh oh!

j9ac9k commented Aug 5, 2022

Uh oh!

bbc131 commented Aug 22, 2022

Uh oh!

j9ac9k commented Aug 22, 2022

Uh oh!

j9ac9k commented Aug 22, 2022

Uh oh!

j9ac9k Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

bbc131 commented Aug 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

j9ac9k commented Aug 23, 2022

Uh oh!

ixjlyons commented Sep 4, 2022

Uh oh!

j9ac9k commented Sep 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bbc131 commented Aug 23, 2022 •

edited

Loading