[Development] QRect::contains and undocumented(?) edge

[Eirik Aavitsland]

Hi Tomi,

I'm sure there are many things that could be improved in both doc and 
code here, but let me try to explain how this was intended at least

> could you please explain how your method "QRect::contains(const QRect 
> &r, bool proper)" at
> https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qrect.cpp?h=dev 

This becomes much easier to understand if one remembers QRect is 
designed to describe a pixel area. Its coordinates are pixel rows and 
columns. QRectF, on the other hand, is designed for (pseudo)cartesian 
coordinates.

So e.g. a QRect(0, 0, 3, 3) has top-left at pixel (0, 0), and covers the 
leftmost 3 pixels of the topmost 3 pixel rows. The contains() function 
then simply answers whether a given pixel is covered or not (true for 
0<=x<=2 && 0<=y<=2). The "edges" here simply means the left/rightmost 
pixel columns and top/bottom pixel rows covered (so "proper" contained 
would be only x=1 && y=1).


> I can guess that x1 x2, with x2 < x1, is "allowed" in QRect due to 
> easier manipulation of QRects in some cases (all ints) (references to 
> such cases?),

Yup, the api calls them non-normalized QRects. Useful in exceptional 
cases, probably, but the api is primarily tuned (and named) for normal 
rects.

> Earlier I was browsing qcosmeticstroke.cpp where clip check with a point 
> is multiple times checked(?) with
> "
> const QRect &cl = stroker->clip;
>      if (x < cl.x() || x > cl.right() || y < cl.y() || y > cl.bottom())
>          return;
> "
> https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/painting/qcosmeticstroker.cpp 
> 
> 
> I was expecting to see this kind of implementation, with negation, in 
> QRect::contains. I was also thinking if performance in rendering
> is so important that a call to QRect::contains cannot be made, and that 
> for some reason QRect::contains cannot be made inline.
> Perhaps there is a way to include an inline implementation of 
> QRect::contains to QRect.h which could be used here?

Looks like the implementation of contains() is indeed done to handle 
non-normalized rects also. For compat reasons one may not want to change 
that, but perhaps one could add a simpler variant (with a different 
name) of contains() that only handles normalized rects as inline in the 
header, yes.

> At
> https://doc-snapshots.qt.io/qt6-dev/qrect.html
> something is mentioned about "historical reasons". No further arguments 
> or links given. Perhaps this is the source of the confusion I have?
> Are these historical reasons permanent or do you plan to move away from 
> these historical reasons? What would go wrong if you abandon the 
> historical reasons?
> I'm aware that QRect has been illogical for a long time

Basically, it's about the challenges that appear when mixing QRects and 
QRectFs, either in use or in thinking, given that they use different 
coordinate systems. (the QRect above has right() == 2, since that is the 
rightmost pixel column covered. A QRectF(0, 0, 3, 3) otoh will have 
right()==3, as it is cartesian.) QRect only becomes "illogical" when 
thought of as a stunted QRectF.
I believe both kinds have their uses in a gui toolkit :)

cheers,
- Eirik Aa.