RFC: Sane rectangle class

Luboš Luňák l.lunak at collabora.com
Thu Mar 19 15:13:50 UTC 2020


 Hello,

 yes, this is about the tools::Rectangle nightmare of an API (in case you 
don't know, it's this [1] ). I'm hunting an off-by-one error somewhere in 
VCL, and it's hard to find it when I can't even tell which parts of the code 
are right or wrong :(.

 This has been already discussed a couple of times, e.g. in the [2] thread, 
and apparently there's no reasonable way to fix tools::Rectangle. Which means 
we basically have two choices, 1) live with it, suck it up, and have code 
full of workarounds where you can't be sure what's right, or 2) use something 
else. Since I'm fed up with 1), I suggest we try 2). Note that it doesn't 
mean doing one big change, I rather propose that we get a replacement for 
tools::Rectangle and slowly transition to it. That means that when writing 
new code we'll have sane API to use, some code will hopefully get rewritten 
over time, and old code that everybody fears to touch can stay the way it is.

 Just to get you an idea what others are doing:
- Qt has messed up in a similar way, but at least it's documented and 
consistent [3]. They basically suggest to use widthxheight and not use 
bottom/right.
- GDK/Gtk/Cairo sidestep this by having a plain rect that contains 
x,y,width,height.
- Skia uses open-interval top/left x bottom/right, it is consistent and API 
differentiates sizes and coordinates, there's e.g. MakeXYWH() and MakeTLRB() 
[4]
- we have also B2IRectangle in basegfx, which is closed interval, it has magic 
value for empty rectangle, I find the API overengineered and confusing for 
something as simple as a rectangle, and 'git grep B2IRectangle | wc -l' 
giving 84 is not particularly encouraging either (compared to 9189 for 
tools::Rectangle)

 So, yeah, I'm proposing a new standard Rectangle class (and I know xkcd, and 
I'm still serious). My idea is roughly that there will be some 
tools::NewRectangle (or whatever usable name), it will be more or less like 
tools::Rectangle, but it'll make things clear, for example:
- internal representation will be whatever sane thing will work, e.g. 
x,y,width,height , and it won't matter for the API
- empty rectangle is simply width == 0 || height == 0
- no (int, int, int, int) ctor
- we can try without bottom and right functions, or we can define what they 
mean and be consistent about it (no idea, no preference)
- there will be things like FromOpenRectangle() to allow converting from/to 
tools::Rectangle, making it hopefully easier to gradually move over


 Comments, proposals, preferences? I'm willing to do the work of writing the 
code.

[1] 
https://cgit.freedesktop.org/libreoffice/core/tree/include/tools/gen.hxx#n364
[2] https://lists.freedesktop.org/archives/libreoffice/2019-May/082683.html
[3] https://doc.qt.io/qt-5/qrect.html#coordinates
[4] https://api.skia.org/structSkRect.html#aef01ba00dd9ab4489fed11413fb2cdc7

-- 
 Luboš Luňák
 l.lunak at collabora.com


More information about the LibreOffice mailing list