Re: RFC: Sane rectangle class

Hi,

On Thu, Mar 19, 2020 at 4:14 PM Luboš Luňák <l.lunak@collabora.com> wrote:

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

That's why I'm trying to (or mainly was trying to because lack of time)
build a set of rendering tests in the vcl/backendtest/* so we can define
what should be the expected rendered output for inputs through a
OutputDevice to catch things like this and harmonise the backends to a
common expectation. But as long as I'm the only one writing new (automated)
tests for this and other just expect test to exist, we will progress slowly
to get VCL to a "sane" state.

Another thing that is sensitive to this change is the SVM - serialized
metafile format, where any change to a type could break the format by
mistake. For that I have written vcl/qa/cppunit/svm/svmtest.cxx but it
isn't an exhaustive test either. Actually it is one where you become
frustrated, because you see how we ruined the MetaFile and we can't fix it
any more because it became part of the file format.

  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.

My long term goal for this is to have enough test coverage to start
re-factoring OutputDevice to use floating point types from basegfx. After
that start to refactor tools:Rectangle to have a sensible implementation
for places where it makes sense to use it (mainly for non-pixel units).

I fear a bit introducing a new implementation as that will just mean one
more implementation that we will have to deal long run, but I don't think
it would make things much worse as they are now - at elast we will know
what was audited to have a sane behaviour and what not.

BTW. there is SwRect in writer too, which is a half-open implementation (or
so it says in the documentation of the class).

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

Regards, Tomaž