RFC: Sane rectangle class

Regina Henschel rb.henschel at t-online.de
Fri Mar 20 17:37:24 UTC 2020


Hi Luboš,

Luboš Luňák schrieb am 19-Mar-20 um 16:13:
> 
>   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 :(.

These off-by-one problems occur earlier than in VCL. For example changes 
to maSnapRect when a shape is transformed by shear and rotation.

> 
>   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.

I see the problem not in tools::Rectangle itself, but in the fact, that 
it uses integer and not double. Using double makes width = right - left 
in all cases and would solve accuracy problems in manipulating shapes. 
It would be up to renderer to do a suitable conversion to integer.

We have already basegfx::B2DRange (or its alias B2DRectangle) and the 
struct RealRectangle2D in the API for using double. But I see no way but 
a big change to get the SdrObject-hierarchy to not use tools::Rectangle 
but basegfx::B2DRange.

[..]
> 
>   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

You can already use getWidth() instead of GetWidth(). A new kind of 
rectangle does not solve the problem, that you have to examine each use, 
whether including or excluding the edge is better. I think, only 
switching to double and using it a long as possible would really help to 
reduce off-by-one problems and increase accuracy.

Kind regards
Regina


More information about the LibreOffice mailing list