Closed
Bug 610594
Opened 9 years ago
Closed 9 years ago
"ABORT: F.6.6.3 should prevent this. Will sqrt(num)!"
Categories
(Core :: SVG, defect, critical)
Tracking
()
RESOLVED
FIXED
Tracking  Status  

blocking2.0    final+ 
People
(Reporter: jruderman, Assigned: jwatt)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, regression, testcase)
Attachments
(2 files)
119 bytes,
text/html

Details  
4.82 KB,
patch

longsonr
:
review+

Details  Diff  Splinter Review 
###!!! ABORT: F.6.6.3 should prevent this. Will sqrt(num)!: 'numerator >= 0', file content/svg/content/src/SVGPathData.cpp, line 609
Assignee  
Updated•9 years ago

blocking2.0:  → final+
Updated•9 years ago

Keywords: regression
Comment 1•9 years ago


FWIW, this is what we have now. if (numerator < 0.0) { // F.6.6 step 3  numerator < 0.0 is equivalent to the result of // F.6.6.2 (lamedh) being greater than one. What we have here is radii // that do not reach between segStart and segEnd, so we need to correct // them. float lamedh = 1.0  numerator/(rx*rx*ry*ry); // equiv to eqn F.6.6.2 float s = sqrt(lamedh); rx *= s; // F.6.6.3 ry *= s; // rx and ry changed, so we have to recompute numerator numerator = rx*rx*ry*ry  rx*rx*y1p*y1p  ry*ry*x1p*x1p; NS_ABORT_IF_FALSE(numerator >= 0, "F.6.6.3 should prevent this. Will sqrt(num)!"); } root = sqrt(numerator/(rx*rx*y1p*y1p + ry*ry*x1p*x1p)); The code it replaced simply set root to 0 in this case rather than recomputing the numerator... if (numerator < 0.0) { // If mRx , mRy and are such that there is no solution (basically, // the ellipse is not big enough to reach from (x1, y1) to (x2, // y2)) then the ellipse is scaled up uniformly until there is // exactly one solution (until the ellipse is just big enough). // > find factor s, such that numerator' with mRx'=s*mRx and // mRy'=s*mRy becomes 0 : float s = (float)sqrt(1.0  numerator/(mRx*mRx*mRy*mRy)); mRx *= s; mRy *= s; root = 0.0; } else { root = (largeArcFlag == sweepFlag ? 1.0 : 1.0) * sqrt( numerator/(mRx*mRx*y1dash*y1dash + mRy*mRy*x1dash*x1dash) ); }
Assignee: nobody → jwatt
Assignee  
Comment 2•9 years ago


The problem is rounding error. Using double instead of float for the intermediary calculations helps in some cases, but in the attached testcase the value still ends up being a very small negative number. Anyway, this reverts to doing what the old code used to do and just setting root to 0.0. That at least gets us back to where we were, although I'm still wondering about the values of rx/ry.
Attachment #498753 
Flags: review?(longsonr)
Comment 3•9 years ago


Comment on attachment 498753 [details] [diff] [review] patch Again, please check in the testcase as a crashtest when landing.
Attachment #498753 
Flags: review?(longsonr) → review+
Assignee  
Comment 4•9 years ago


http://hg.mozilla.org/mozillacentral/rev/6a8cca708a48
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution:  → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•