[Qt-interest] possible doc bug for QGraphicsItem::contains? (4.5.2)

[Ross Bencina]

Hi

I just spent a couple of hours working out why my optimised QGraphicsItem 
hit testing code isn't working. The documentation suggests that you can 
override QGraphicsItem::contains() to supply a faster hit testing method 
(for mouse hover, press etc). However it turns out that if QGraphicsScene 
has an associated QGraphicsView then QGraphicsItem::contains() is _never_ 
called for hit testing, instead the hit testing is always done with 
rectangle intersection on QGraphicsItem::shape() instead.

The relevant source references are:

qgraphicsscene.cpp:1042, where QGraphicsScenePrivate::itemsAtPosition() 
calls QGraphicsView::items(QPointF) to get the list of items

and

qgraphicsview.cpp:2282, where QGraphicsView::items is implemented in terms 
of QGraphicsView::items(QRectF)

so QGraphicsItem::contains()  only gets called if there is no QGraphicsView.

I'm not sure if this is a bug but it is certainly in contradiction of the 
documentation.

I recommend deprecating contains() or at least providing clear documentation 
along the lines of:

- Remove all references suggesting QGraphicsItem::contains() is always used 
for hit testing and replace with shape() is used for hit testing.
- QGraphicsItem::contains() should be documented as only providing an 
optimisation and should always return the same results as 
QGraphicsItem::shape().contains()
- Note that sometimes QGraphicsItem::shape() will be called for hit testing 
and other times QGraphicsItem::contains() and that the implementation should 
not assume one or the other.
- I think discussions of  prepareGeometryChange() should also mention that 
contains() needs to follow the same invariants as shape() and boundingRect() 
since Qt seems to cache results sometimes.

OR as a big alternative to the above, Qt should work as-documented.

thanks!

Ross.