On 29/11/2012 18:34, BRM wrote: >> From: Alan Alpert <[email protected]> >> 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 <[email protected]> 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. Cheers, Sean -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
