Closed Bug 1393605 Opened 7 years ago Closed 7 years ago

stylo: panicked at 'determinant should now be 1 or -1'

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed

People

(Reporter: truber, Assigned: boris)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase)

Attachments

(3 files, 1 obsolete file)

Attached file testcase.html (deleted) —
The attached testcase causes a panic in m-c rev 20170824-892c8916ba32 with stylo enabled by pref. thread '<unnamed>' panicked at 'determinant should now be 1 or -1', /home/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style-4d3f3573eb5f1581/out/properties.rs:90865 stack backtrace: 0: 0x7f5328b423d3 - std::sys::imp::backtrace::tracing::imp::unwind_backtrace::hcab99e0793da62c7 1: 0x7f5328b3d6f6 - std::sys_common::backtrace::_print::hbfe5b0c7e79c0711 2: 0x7f5328b4fa6a - std::panicking::default_hook::{{closure}}::h9ba2c6973907a2be 3: 0x7f5328b4f66b - std::panicking::default_hook::he4d55e2dd21c3cca 4: 0x7f5328b4feba - std::panicking::rust_panic_with_hook::ha138c05cd33ad44d 5: 0x7f5328738ffa - std::panicking::begin_panic::h3893082380049d75 6: 0x7f532884250e - style::properties::animated_properties::decompose_2d_matrix::hf348b1f062a35f9d [Parent 29953] WARNING: stylo: No docshell yet, assuming Gecko style system: file /home/worker/workspace/build/src/dom/base/nsDocument.cpp, line 13446 7: 0x7f532856a6c5 - style::properties::animated_properties::<impl style::values::animated::Animate for style::properties::longhands::transform::computed_value::ComputedMatrix>::animate::hdfafca52f4cd648d 8: 0x7f53283bb94a - Servo_MatrixTransform_Operate 9: 0x7f5326a0daf4 - void nsStyleTransformMatrix::ProcessMatrixOperator<nsStyleTransformMatrix::Interpolate>(mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits>&, nsCSSValue::Array const*, mozilla::GeckoStyleContext*, nsPresContext*, mozilla::RuleNodeCacheConditions&, nsStyleTransformMatrix::TransformReferenceBox&, bool*) 10: 0x7f5326a0d60f - nsStyleTransformMatrix::ReadTransforms(nsCSSValueList const*, nsStyleContext*, nsPresContext*, mozilla::RuleNodeCacheConditions&, nsStyleTransformMatrix::TransformReferenceBox&, float, bool*) 11: 0x7f5326c94aa6 - nsDisplayTransform::GetResultingTransformMatrixInternal(nsDisplayTransform::FrameTransformProperties const&, nsPoint const&, float, unsigned int, nsRect const*) 12: 0x7f5326c94f2e - nsDisplayTransform::GetResultingTransformMatrix(nsIFrame const*, nsPoint const&, float, unsigned int, nsRect const*) 13: 0x7f5326c95408 - nsDisplayTransform::TransformRect(nsRect const&, nsIFrame const*, nsRect const*) 14: 0x7f5326b127ac - nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize, nsSize*, nsStyleDisplay const*) 15: 0x7f5326b12a61 - nsIFrame::FinishAndStoreOverflow(mozilla::ReflowOutput*, nsStyleDisplay const*) 16: 0x7f5326b2279f - nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) 17: 0x7f5326b0e25d - nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) 18: 0x7f5326b235c5 - nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) 19: 0x7f5326b0e25d - nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*)
Flags: in-testsuite?
Flags: needinfo?(boris.chiou)
The actual determinant here is 0.00028317794
(In reply to Manish Goregaokar [:manishearth] from comment #2) > The actual determinant here is 0.00028317794 Thanks, Manishearth. I will try to use another way to avoid this case.
Flags: needinfo?(boris.chiou)
Assignee: nobody → boris.chiou
Mark P2 because "panic"
Priority: -- → P2
Status: NEW → ASSIGNED
OK, Gecko also has this assertion, but it's not a fetal assert. In other words, we use NS_ASSERTION(), instead of MOZ_ASSERT(), so that's why we didn't notice this error on Gecko. This may be a bug in Gecko, I think, so I won't revise the decomposition of 2d matrix. Instead, downgrade the debug_assert().
Comment on attachment 8902523 [details] Bug 1393605 - Return Err(()) if determinant is not 1 or -1 while decomposing the 2d matrix. https://reviewboard.mozilla.org/r/174114/#review179364 ::: servo/components/style/properties/helpers/animated_properties.mako.rs:1715 (Diff revision 1) > m22 /= scale_y; > shear_xy /= scale_y; > > let determinant = m11 * m22 - m12 * m21; > - debug_assert!(0.99 < determinant.abs() && determinant.abs() < 1.01, > - "determinant should now be 1 or -1"); > + if 0.99 > determinant.abs() || determinant.abs() > 1.01 { > + // determinant should now be 1 or -1". Nit no need for the " at the end of the comment. Also, I'd put the comment before the "if" (since it explains what we're checking, not the failure case) and make it start with a capital letter: // Determinant should now be 1 or -1
Attachment #8902523 - Flags: review?(bbirtles) → review+
Comment on attachment 8902523 [details] Bug 1393605 - Return Err(()) if determinant is not 1 or -1 while decomposing the 2d matrix. https://reviewboard.mozilla.org/r/174114/#review179364 > Nit no need for the " at the end of the comment. > > Also, I'd put the comment before the "if" (since it explains what we're checking, not the failure case) and make it start with a capital letter: > > // Determinant should now be 1 or -1 Oh yes. I should update this sentence and put it before "if". Thanks.
Attached file Servo PR, #18305 (deleted) —
Flags: needinfo?(boris.chiou)
Flags: needinfo?(boris.chiou)
Keywords: leave-open
Attachment #8902523 - Attachment is obsolete: true
Attachment #8903012 - Flags: review?(bbirtles) → review+
Pushed by bchiou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b53b0454742b Fix gecko assertion and add one crashtest. r=birtles
Keywords: leave-open
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: