1009825 - FuzzyEquals* functions are unsuitable for typical page coordinate values

Status: NEW

(Core :: MFBT, defect)

Description

[Botond Ballo [:botond]]

For example, when zoomed out and scrolled to the end of the page on http://people.mozilla.org/~bballo/grid.html, I see:

  GetCompositionEnd() = 2001.00012
  GetPageEnd()        = 2001

This is frustrating my attempt in bug 998025 to assert that the two are FuzzyEqual when we are in overscroll, and I'd like to fix it.

Comment 1

[Botond Ballo [:botond]]

On second thought, the above difference is acceptable rounding error for a 'float'. My problem was that I was using FuzzyEqualsAdditive for comparing them, which is unsuitable for values in the thousands (at least with the default epsilon).

So I tried using FuzzyEqualsMultiplicative() instead, but then that failed in a case where one value was 0 and another around 1e-6 - again acceptable rounding error for a 'float', but FuzzyEqualsMultiplicative() does not work if one of its arguments is zero.

So, neither FuzzyEqualsAdditive() nor FuzzyEqualsMultiplicative() handle both of the following pairs of numbers:

  1)    2001 and 2011.00012
  2)       0 and 1e-6

Such numbers are typical for page coordinate values, with the rounding error being introduced while accounting for zooming. It would be great to have a FuzzyEquals function that handled such values.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

bjacob, any thoughts on this?

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Sounds like we should have a phone or live conversation about this, as these questions generally depend a lot on the particulars of your application-specific constraints.

Some general ideas that we would have to think about:
 - Don't "try" things --- know exactly what behavior you want, and stick to it.
 - In particular, it seems that page coordinates should have the same absolute precision at the bottom of the page as they have at the top of the page. This suggests that you might really want additive comparisons, not multiplicative comparisons.
 - Know what precision you need. Float gives you 7 decimal digits of precision. If pages can be as tall as 1e4 pixels, that means that you get about 1e-3 pixel precision. Is that enough for your needs? If yes, live with the small error that you're observing above. If not, then you must switch to another representating. Besides doubles (expensive on some mobile devices), you might find that fixed-point is a reasonable representation for use cases where you only want to perform additive comparisons anyway.
 - Asserting on conditions based on floating point arithmetic is most of the time a cause of intermittent assertion failures, because it's hard to prove that such conditions are always true. Either prove that in your case, or give up on asserting.

Comment 4

[Botond Ballo [:botond]]

(In reply to Benoit Jacob [:bjacob] from comment #3)
> Know what precision you need. Float gives you 7 decimal digits of
> precision. If pages can be as tall as 1e4 pixels, that means that you get
> about 1e-3 pixel precision. Is that enough for your needs? If yes, live with
> the small error that you're observing above.

Yes, I can live with the level of error I'm observing.

Does that mean I should be using FuzzyEqualsAdditive with an epsilon on 1e-3?

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

From the limited view that I have into your problem at this point, it does look like you want FuzzyEqualsAdditive with whatever epsilon matches your notion of "small enough difference that it doesn't matter".

That's only the first of the questions that you have to answer, though. The next question, typically, is: what do you do with this imprecision? Should you snap values, so that afterwards the  actual stored value what it is interpreted as?

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

missing *is* :

"so that afterwards the  actual stored value *is* what it is interpreted as? "

Comment 7

[Ivan Tsay (:ITsay)]

Hi Botond,

Do we need this to be fixed before the feature landing in bug 998025? If so, I will mark this as feature-b2g=2.0. Otherwise, we just take this as the bug fixing for 2.0 after feature landing deadline (June 6).

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

Looks like this needinfo was intended for Botond, not me.

Comment 9

[Botond Ballo [:botond]]

It looks like we have settled on an acceptable solution in the short term, which is to use FuzzyEqualsAdditive with an epsilon on 1e-2.

I'd still like to think about what Benoit said in comment 5 and determine if any follow-up action is necessary before closing this bug. I'm removing bug 998025's dependency on this.