Closed Bug 455138 Opened 11 years ago Closed 11 years ago

Bug 435293 landed failing tests

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.1b1

People

(Reporter: sgautherie, Assigned: kschwarz)

References

Details

Attachments

(1 file, 2 obsolete files)

http://hg.mozilla.org/mozilla-central/rev/db31a92daecb
[
rocallahan@mozilla.com
Sat Sep 13 04:28:36 2008 -0700
db31a92daecb	Robert O'Callahan — Marking failing tests random
]

to workaround
[
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/comm-central-macosx/build/mozilla/layout/reftests/transform/translatex-1b.html | 
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/comm-central-macosx/build/mozilla/layout/reftests/transform/translatey-1b.html | 
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/comm-central-macosx/build/mozilla/layout/reftests/transform/translate-1b.html | 
]
I landed additional
http://hg.mozilla.org/mozilla-central/rev/c833b8069d4d

to workaround
[
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/comm-central-linux/build/mozilla/layout/reftests/transform/percent-1d.html | 
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/comm-central-linux/build/mozilla/layout/reftests/transform/percent-1e.html | 
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/comm-central-linux/build/mozilla/layout/reftests/transform/percent-1f.html | 
]
http://hg.mozilla.org/mozilla-central/rev/7819481fc8d1
[
rocallahan@mozilla.com
Sat Sep 13 06:53:04 2008 -0700
7819481fc8d1	Robert O'Callahan — Mark failing tests with bug#
]
What Linux configuration are the tests running on?  I'd like to fix whatever's causing these bugs, but on my Linux system the tests are passing.
I don't know, I can't reproduce those failures on my Linux box either.
The translate failures should be easier to track down since they happened on both Windows and Mac. Unfortunately my Mac/Windows box is down at the moment.
All of these problems stem from rounding errors accumulating using floats, and unfortunately I don't think there's that much I can do to fix it.  I've found a few spots where I can curb error by constraining angular values to specific ranges internally, but overall I think that these sorts of errors are tricky to fix - some of the errors I've found are caused by discrepancies of less than 0.00001.  Long chains of transform computations are bound to lead to problems even when using doubles.  Any suggestions on what to do?  Should I scrap the reftests that are getting clobbered by rounding errors?
Attached patch Potential Patch #1 (obsolete) — Splinter Review
Fixes all of the failing reftests from the original patch by correcting rounding errors as much as possible.  Also patched the rotation tests, since the current version was rotating the elements out of the viewport and effectively rendering the test meaningless.  However, fixing the rotate tests ended up exposing another class of reftest failures stemming from subpixel (<0.00001 gfx units) rounding errors in the rotate tests, so I've marked them as random.  I'm not sure how to go about fixing those sorts of rounding errors, or if it's even possible to do so.  Suggestions appreciated.
Assignee: nobody → keith
Status: NEW → ASSIGNED
Attachment #339625 - Flags: superreview?(dbaron)
Attachment #339625 - Flags: review?(dbaron)
Attached patch Potential Patch #1 (correction) (obsolete) — Splinter Review
Whoops, left in some debugging printfs.  They've been removed in this patch.
Attachment #339625 - Attachment is obsolete: true
Attachment #339626 - Flags: superreview?(dbaron)
Attachment #339626 - Flags: review?(dbaron)
Attachment #339625 - Flags: superreview?(dbaron)
Attachment #339625 - Flags: review?(dbaron)
Attachment #339626 - Flags: superreview?(dbaron)
Attachment #339626 - Flags: superreview+
Attachment #339626 - Flags: review?(roc)
Attachment #339626 - Flags: review?(dbaron)
Attachment #339626 - Flags: review+
Comment on attachment 339626 [details] [diff] [review]
Potential Patch #1 (correction)

>+/* Helper function to constrain an angle in the range [0, 2pi) to a value
>+ * in the range [-pi, pi), which reduces accumulated floating point errors
>+ * from trigonometric functions by keeping the error terms small.
>+ */
>+static inline float ConstrainFloatValue(float aValue)
>+{
>+  return aValue >= kPi ? aValue - 2.0f * kPi : aValue;
>+}

Given that every caller of this calls fmod, maybe it would be good to have the fmod call actually be in this function?

>-    /* 360deg = 2pi rad, so deg = pi / 180 rad */
>+    /* 360deg = 2pi rad, so deg = pi / 180 rad. */

Seems nicer to have the deg and grad comments formatted consistently...

>+    /* Constrain to [0, 2*pi) */

This comment doesn't match the rest (or tho code).


r+sr=dbaron, although it would probably be good to have roc take a look as well.
Addresses r/sr comments from dbaron.  Only functional change is to push fmod into ConstrainFloatValue.
Attachment #339626 - Attachment is obsolete: true
Attachment #339679 - Flags: review?(roc)
Attachment #339626 - Flags: review?(roc)
Comment on attachment 339679 [details] [diff] [review]
Potential Patch #2
[Checkin: Comment 12]

Clever!
Attachment #339679 - Flags: review?(roc) → review+
Keywords: checkin-needed
Comment on attachment 339679 [details] [diff] [review]
Potential Patch #2
[Checkin: Comment 12]

http://hg.mozilla.org/mozilla-central/rev/6b03a4f27f8f
Attachment #339679 - Attachment description: Potential Patch #2 → Potential Patch #2 [Checkin: Comment 12]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.