Bug 274834

Summary: DOMMatrix.inverse() returns NaN matrix for small but normal values
Product: WebKit Reporter: halifirien
Component: Layout and RenderingAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, karlcow, sabouhallawa, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari 17   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
test case none

Description halifirien 2024-05-29 08:17:52 PDT
DOMMatrix and SVGMatrix, with same values, calculate different inverses



const matrix = new DOMMatrix([9.780006597705043e-07, 3.532646825298159e-07, -2.528174268419108e-07, 1.326003454762908e-06, 42.05985792398261, -76.82054904789567])
const svgMatrix = document.createElementNS('http://www.w3.org/2000/svg', 'svg').createSVGMatrix()

// copy values
svgMatrix.a = matrix.a
svgMatrix.b = matrix.b
svgMatrix.c = matrix.c
svgMatrix.d = matrix.d
svgMatrix.e = matrix.e
svgMatrix.f = matrix.f

matrix.inverse() // filled with NaN
svgMatrix.inverse() // 👍
Comment 1 Simon Fraser (smfr) 2024-05-29 10:26:07 PDT
Does this reproduce in Firefox or Chrome?
Comment 2 halifirien 2024-05-29 10:51:29 PDT
On Sonoma 14.5 (23F79)

* Firefox 126.0.1
* Chrome 125.0.6422.113

`new DOMMatrix([9.780006597705043e-07, 3.532646825298159e-07, -2.528174268419108e-07, 1.326003454762908e-06, 42.05985792398261, -76.82054904789567]).inverse()`

calculates the inverse and matches the SVGMatix in webkit. (Firefox has minor rounding differences between DOM and SVG versions)
Comment 3 Said Abou-Hallawa 2024-05-29 11:07:23 PDT
Created attachment 471543 [details]
test case
Comment 4 Said Abou-Hallawa 2024-05-29 11:57:19 PDT
The reason of the discrepancy between DOMMatrix.inverse() and SVGMatrix.inverse() is DOMMatrix is based on TransformationMatrix while SVGMatrix is based on AffineTransform.

TransformationMatrix::inverse() has this check 

        if (std::abs(determinant) < SMALL_NUMBER)
            return std::nullopt;

AffineTransform does not have a similar check. If this if-statement is removed, TransformationMatrix::inverse() will return the same value that AffineTransform::inverse() returns.
Comment 5 Said Abou-Hallawa 2024-05-29 12:31:55 PDT
AffineTransform::inverse() has this check:


    if (!std::isfinite(determinant) || determinant == 0)
        return std::nullopt;


And TransformationMatrix::inverse() has this check:

    if (std::abs(determinant) < SMALL_NUMBER)
        return std::nullopt;

And SMALL_NUMBER is defined as this:

    const double SMALL_NUMBER = 1.e-8;

So I think we can have something between checking if equal to zero and checking if less than SMALL_NUMBER. Maybe something like this:

    if (!std::isnormal(determinant))
        return std::nullopt;
Comment 6 Radar WebKit Bug Importer 2024-05-29 13:05:38 PDT
<rdar://problem/128960283>
Comment 7 Said Abou-Hallawa 2024-05-29 13:37:04 PDT
Pull request: https://github.com/WebKit/WebKit/pull/29244
Comment 8 EWS 2024-05-31 11:03:07 PDT
Committed 279588@main (1f6b653b2343): <https://commits.webkit.org/279588@main>

Reviewed commits have been landed. Closing PR #29244 and removing active labels.