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)

x86
macOS
defect
Not set
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)

Attached file testcase
###!!! ABORT: F.6.6.3 should prevent this. Will sqrt(-num)!: 'numerator >= 0', file content/svg/content/src/SVGPathData.cpp, line 609
blocking2.0: --- → final+
Depends on: 522306
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) );
  }
Attached patch patchSplinter Review
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 on attachment 498753 [details] [diff] [review]
patch


Again, please check in the testcase as a crashtest when landing.
Attachment #498753 - Flags: review?(longsonr) → review+
http://hg.mozilla.org/mozilla-central/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.