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

Giuseppe D'Angelo giuseppe.dangelo at kdab.com
Wed Apr 20 18:21:31 CEST 2022


Hi,

On 20/04/2022 17:50, Tomi Pannila wrote:
> 
> thank you for the explanations. I guess Eirik kind of guessed right that
> I view QRect from $\mathbb{R}^2$ point of view.
> I'm a mathematician and this is the most natural point of view for me.
> For me, rectangles have edges=boundary which are measure zero.

I'm not really sure if this definition matches what QRect is (vs. 
QRectF). QRect is defined in terms of pixels on a grid, and this brings 
some counter-intuitive behaviors, like the fact that the rightmost 
border is not defined as "left + width".

> 
> 
> 
> To Lars
> 
>   >> With separate class one could have custom edge sizes as a bonus.
>   > That would be a rect with a border. But where do you stop then? One
> border width for all sizes of configurable for each side independently?
> In addition, you’d need more data to store that information making the
> class bigger.
>   > So far there hasn’t been a need to have this as a general purpose
> class in Qt.
> You are right, it would be a border instead an edge. Isn't the current
> implementation about edge also in some sense a border check of the
> "weird interior"?
> I don't know where one is expected to stop if one creates a class for
> rectangles with edges.
> It was a question related on separating rectangles with edges from
> ordinary rectangles.
> An idea to clarify the meaning and usage of rectangles.
> If one is serious on separating them, then the question "how far one
> would go" depends on "how edges are expected to be used" in Qt and
> outside Qt?

I may have missed this, but what is the desired change here? A different 
class for integer-based rectangles, where the coordinates are "between" 
pixels or so?

>   > Historical reasons means this has been the behavior for the last 25
> years, and a huge amount of code relies on this behavior. Changing it
> would lead to subtle breaks in lots of places for our users, breakage
> that is not easily
>   > found at compile time. So we don’t want to change it to not break
> lots of user code out there relying on it.
> I understand. I was hoping that nobody outside Qt would use the edges
> and hence the change would be plausible.
> I guess one could compile Qt with a warning logged if
> "rect.contains(point, true)" is called.
> This might give an idea how much it is actually used.

In general, the problem is that this is public API, has been like this 
for almost 20 years, and it's a very low level class used all over the 
place. This means, there *is* code somewhere that depends on the current 
behaviors.

https://www.hyrumslaw.com/

Fixing QRect's API quirks is something that comes up from time to time 
and every time (e.g. most recently 
https://codereview.qt-project.org/c/qt/qtbase/+/371251 ) and every time 
I'm *extremely* concerned about the regressions that any such change 
will cause in the wild.

Now, this specific feature seems to be completely unused, so if the 
wanted change is to simply drop it, it's much more acceptable (in the 
sense that users that DO use it are going to get compile errors).

My 2 c,

-- 
Giuseppe D'Angelo | giuseppe.dangelo at kdab.com | Senior Software Engineer
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
KDAB - The Qt, C++ and OpenGL Experts


More information about the Development mailing list