[Development] Comparing two reals in Qt code

[Samuel Rødal]

On 11/29/2012 07:38 PM, Sean Harmer wrote:
> On 29/11/2012 18:34, BRM wrote:
>>> From: Alan Alpert <416365416c at gmail.com>
>>> Sent: Thursday, November 29, 2012 11:52 AM
>>> Subject: Re: [Development] Comparing two reals in Qt code
>>>
>>> On Thu, Nov 29, 2012 at 6:53 AM, Sean Harmer <sean.harmer at kdab.com> wrote:
>>>>    On Thursday 29 November 2012 15:01:05 Dominik Holland wrote:
>>>>>    Hi all,
>>>>>
>>>>>    last month i had some performance problems on a QML animation running
>>> on
>>>>>    low end hardware (imx233).
>>>>>    I debugged that problem and it was caused because of rounding errors
>>>>>    during the comparison of two reals.
>>>>>
>>>>>    I fixed that problem by replacing the comparison by a qFuzzyCompare()
>>> in
>>>>>    the Qt4 declarative source code.
>>>>>    Afterwards i checked whether qt5 has the same problem... and it has
>>>>>    exactly the same problem.
>>>>>
>>>>>    Now i pushed my patch to gerrit
>>>>>    (https://codereview.qt-project.org/#change,40655) and looked again at
>>>>>    the code
>>>>>    for any other occurrences.
>>>>>
>>>>>    What i found is, that this happens in many places and it would be a
>>>>>    really huge effort to change the code to compare the qreals in the
>>> right
>>>>>    way.
>>>>>
>>>>>    Is this already a known problem and do you think that it is worth to
>>> try
>>>>>    to fix the code ?
>>>>    Also please be aware that qFuzzyCompare() is nto the right way to compare
>>> two
>>>>    arbitrary floating point values. If one of them is zero or close to zero
>>> the
>>>>    condition it tests for becomes impossible to satisfy.
>>>>
>>>>    Something along the lines of
>>>>
>>>>    fuzzyCompare(float p1, float p2)
>>>>    {
>>>>        if (qFuzzyIsNull(p1))
>>>>            return qFuzzyIsNull(p2);
>>>>        else if (qFuzzyIsNull(p2))
>>>>            return false;
>>>>        else
>>>>            return qFuzzyCompare(p1, p2);
>>>>    }
>>>>
>>>>    is slightly better but still not fantastic. Comparing two floats that we do
>>>>    not know a priori is actually very difficult
>>>>
>>>>    http://randomascii.wordpress.com/2012/02/25/comparing-floating-point-
>>>>    numbers-2012-edition/
>>>>
>>>>    I would love to see a better qFuzzyCompare() implementation in Qt but the
>>>>    existing one is used in so many places it would be a nightmare to introduce
>>>>    without unwanted side effects.
>>> What about a second qFuzzyCompare using an absolute instead of
>>> relative epsilon? Something like 1e-5 should give a decent answer for
>>> values between 1e5 and -1e5, which could be used for values like on
>>> screen positions.
>>>
>>> By having two functions for developers to choose from we'd mitigate
>>> the a priori knowledge problem, because some developers will know
>>> roughly which floats they're going to be testing and they can pick the
>>> right function for that.
>>>
>> How about allow the epsilon to be specified as an argument?
>> Then developers who need more resolution can get it, and it still mitigates the a priori issue
>> as it is (i) better documented, and (ii) documented in user code for what the developer requires
>> or thought they were getting.
>
> I have a work in progress patch somewhere that does similar to what you
> suggest with versions for absolute or relative comparisons using ideas
> from the link I posted earlier. I'll tidy it up and see if it's good
> enough to get into 5.1.

Yep, having something similar to AlmostEqualUlpsAndAbs() would be great. 
I've had some ideas of making qFuzzyCompare work that way in the past, 
but gave them up due to not wanting to risk subtly breaking a lot of 
existing code. Introducing a new function, with default but customizable 
arguments for the max differences, sounds like the way to go.

The following tasks might be relevant:
https://bugreports.qt-project.org/browse/QTBUG-26453
https://bugreports.qt-project.org/browse/QTBUG-16819

--
Samuel