Closed Bug 1390046 Opened 4 years ago Closed 4 years ago

stylo: test_transform_limits.html fails with expected "matrix(1, 0, 0, 1, 3.40282e+38, 0)" but got "matrix(1, 0, 0, 1, 1.78957e+7, 0)"

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: birtles, Assigned: daisuke)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Running locally I get:

INFO TEST-UNEXPECTED-FAIL | dom/animation/test/mozilla/test_transform_limits.html | Test that the parameter of transform translate is clamped - Test that the parameter of transform translate is clamped: assert_equals: expected "matrix(1, 0, 0, 1, 3.40282e+38, 0)" but got "matrix(1, 0, 0, 1, 1.78957e+7, 0)"

Some sort of precision issue?
Yeah, servo parses translate() as AppUnit (Au) value which is represented by i32. We can separate the test case as a crash test, I think.
Assignee: nobody → dakatsuka
Status: NEW → ASSIGNED
I think we could probably keep this as a mochitest but perhaps just check that the translateX value is sufficiently large (and, I guess, not Infinity).

Alternatively, we could hard-code in the max we expect:

- const max_float = 3.40282e+38;
+ const MAX_TRANSLATE_COMPONENT = isServo ? 1.78957e+7 : 3.40282e+38;

But I guess we'll want to change the parsing of translate() in future to use higher precision?
(In reply to Brian Birtles (:birtles) from comment #2)
> I think we could probably keep this as a mochitest but perhaps just check
> that the translateX value is sufficiently large (and, I guess, not Infinity).
> 
> Alternatively, we could hard-code in the max we expect:
> 
> - const max_float = 3.40282e+38;
> + const MAX_TRANSLATE_COMPONENT = isServo ? 1.78957e+7 : 3.40282e+38;

I'd prefer this even if it might be changed in future, because we can know the exact limit value in this test case.
(In reply to Daisuke Akatsuka (:daisuke) from comment #4)
> try:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=93fb8a0286f424b7af05eb7ca322322026bf0262

You don't need to speficy linux64-haz, you need to specify --artifact instead.
Comment on attachment 8898149 [details]
Bug 1390046: Fix test fail.

https://reviewboard.mozilla.org/r/169506/#review174780

::: commit-message-5c79f:1
(Diff revision 1)
> +Bug 1390046: Fix test fail. r?hiro

You should try to write descriprive commit message. :)

'Use max integer value for the translate limit value on stylo' or something.

::: dom/animation/test/mozilla/file_transform_limits.html:11
(Diff revision 1)
>  'use strict';
>  
>  // We clamp +infinity or -inifinity value in floating point to
>  // maximum floating point value or -maximum floating point value.
> -const max_float = 3.40282e+38;
> +const MAX_FLOAT = 3.40282e+38;
> +const MAX_TRANSLATE_COMPONENT = isStyledByServo() ? "1.78957e+7" : MAX_FLOAT;

If we add a const value for this max int, it would be more clear what the value is.
Attachment #8898149 - Flags: review?(hikezoe) → review+
Okay, thanks Hiro!
Comment on attachment 8898149 [details]
Bug 1390046: Fix test fail.

https://reviewboard.mozilla.org/r/169506/#review174784

::: commit-message-5c79f:1
(Diff revision 2)
> +Bug 1390046: Fix test fail. r?hiro
> +
> +Use max integer value for the translate limit value on stylo.

I meant that using this 'Use ..' in the first place, and it would be nice to have the reason here.
https://hg.mozilla.org/mozilla-central/rev/1aaa19e64c89
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Attachment #8899049 - Flags: review+
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ab92c4aa737b
Enable test_transform_limits.html on stylo. r=jdm
wao, thank you very much, Hiro, Josh!
You need to log in before you can comment on or make changes to this bug.