diff options
| author | wolfbeast <mcwerewolf@gmail.com> | 2018-09-14 20:54:19 +0200 |
|---|---|---|
| committer | wolfbeast <mcwerewolf@gmail.com> | 2018-09-14 20:54:19 +0200 |
| commit | 351ffa462d2314c4b60369e2ba0f13b5d90f03b5 (patch) | |
| tree | 3a65ef976872b6ca31f1c68998113dd25e80dc11 | |
| parent | 2fce053e98de11551df1afa69db7f127c44ef5f4 (diff) | |
| download | UXP-351ffa462d2314c4b60369e2ba0f13b5d90f03b5.tar UXP-351ffa462d2314c4b60369e2ba0f13b5d90f03b5.tar.gz UXP-351ffa462d2314c4b60369e2ba0f13b5d90f03b5.tar.lz UXP-351ffa462d2314c4b60369e2ba0f13b5d90f03b5.tar.xz UXP-351ffa462d2314c4b60369e2ba0f13b5d90f03b5.zip | |
Fix wrong SVG sizes with non-integer values for viewBox width/height.
Includes a standalone reftest.
| -rw-r--r-- | dom/svg/test/reftest_viewport_noninteger.html | 175 | ||||
| -rw-r--r-- | layout/svg/nsSVGOuterSVGFrame.cpp | 9 |
2 files changed, 180 insertions, 4 deletions
diff --git a/dom/svg/test/reftest_viewport_noninteger.html b/dom/svg/test/reftest_viewport_noninteger.html new file mode 100644 index 000000000..3f4852b53 --- /dev/null +++ b/dom/svg/test/reftest_viewport_noninteger.html @@ -0,0 +1,175 @@ +<!DOCTYPE html> +<html> + <head> + <meta charset="utf-8"> + <title>SVG size test</title> + <style> + body { + margin: 30px; + font-family: sans-serif; + } + + #tests { + display: grid; + grid-template-columns: repeat(4, 1fr); + grid-gap: 30px; + } + + #tests div:hover { + opacity: 1 !important; + } + + #tests p { + text-decoration: underline dotted black; + } + + svg { + background: lightgrey; + } + </style> + </head> + <body> + <h1>SVG size test</h1> + + <p>The grey boxes below are <code><svg></code> elements.</p> + <p>All SVGs in each row should have the exact same size.</p> + <p>Each row has its own viewBox width and height: <code>viewBox="0 0 width height"</code>.</p> + <p>Each column has its own width/height styling. For example, <code>style="width: 200px; height: auto;"</code>.</p> + <p>The first column has both an explicit widht and an explicit height, so there's not much that can go wrong there. It acts as a reference.</p> + <p>The first row has integer viewBox width and height. Firefox then sizes all SVGs correctly.</p> + <p>The remaining rows have at least one non-integer viewBox width and height. Firefox then sizes the SVGs a bit wrong.</p> + <p>Chrome, Safari and Edge seem to pass all tests.</p> + + <p id="summary"></p> + + <div id="tests"></div> + + <script> + const testsElement = document.getElementById("tests"); + const summaryElement = document.getElementById("summary"); + + // Turn for instance `2.3` into `230` (px). Round to avoid floating point + // issues. + const scale = (number) => Math.round(100 * number); + + const widths = [2, 2.3, 2.5, 2.8]; + const heights = [3, 3.3, 3.5, 3.8]; + + let numPassed = 0; + let numFailed = 0; + + for (const width of widths) { + for (const height of heights) { + const variations = [ + {width, height}, + {width: "auto", height}, + {width, height: "auto"}, + {width: "auto", height: "auto"}, + ]; + + for (const variation of variations) { + const cellElement = document.createElement("div"); + + const titleElement = document.createElement("h2"); + titleElement.appendChild(makeTitle(width, height, variation)); + + const sizeElement = document.createElement("p"); + + const svgWrapperElement = document.createElement("div"); + svgWrapperElement.style.width = + variation.width === "auto" && variation.height === "auto" + ? `${scale(width)}px` + : "auto"; + + const svgElement = document.createElementNS("http://www.w3.org/2000/svg", "svg"); + svgElement.setAttribute("viewBox", `0 0 ${width} ${height}`); + svgElement.style.width = + typeof variation.width === "number" + ? `${scale(variation.width)}px` + : variation.width; + svgElement.style.height = + typeof variation.height === "number" + ? `${scale(variation.height)}px` + : variation.height; + + svgWrapperElement.appendChild(svgElement); + + cellElement.appendChild(titleElement); + cellElement.appendChild(sizeElement); + cellElement.appendChild(svgWrapperElement); + + testsElement.appendChild(cellElement); + + const rect = svgElement.getBoundingClientRect(); + const actual = { + width: Math.round(rect.width), + height: Math.round(rect.height), + }; + const expected = { + width: scale(width), + height: scale(height), + }; + const passed = + actual.width === expected.width && + actual.height === expected.height; + + const icon = passed ? "✔" : "✘"; + const iconText = passed ? "PASS" : "FAIL"; + const expectedText = passed ? "" : `\nExpected size: ${expected.width}x${expected.height}`; + sizeElement.textContent = `${icon} ${actual.width}x${actual.height}`; + sizeElement.title = `${iconText}. Actual size, as measured by element.getBoundingClientRect().${expectedText}`; + sizeElement.style.color = passed ? "lime" : "red"; + sizeElement.style.fontWeight = passed ? "normal" : "bold"; + + cellElement.style.opacity = passed ? 0.5 : 1; + + if (passed) { + numPassed++; + } else { + numFailed++; + } + } + } + } + + const numTotal = numPassed + numFailed; + const passed = numFailed === 0; + const icon = passed ? "✔" : "✘"; + summaryElement.textContent = `${icon} ${numPassed}/${numTotal} tests passed.`; + summaryElement.style.color = passed ? "lime" : "red"; + summaryElement.style.fontWeight = "bold"; + + function makeTitle(width, height, variation) { + const fragment = document.createDocumentFragment(); + + const first = document.createElement("abbr"); + first.textContent = `${width}/${height}`; + first.title = `SVG viewBox width/height: viewBox="0 0 ${width} ${height}"`; + + const separator = document.createTextNode(" | "); + + const second = document.createElement("abbr"); + + const widthString = typeof variation.width === "number" ? "px" : variation.width; + const heightString = typeof variation.height === "number" ? "px" : variation.height; + second.textContent = `${widthString}/${heightString}`; + + const widthExplanation = + typeof variation.width === "number" + ? "explicit width (px)" + : `${variation.width} width` + const heightExplanation = + typeof variation.height === "number" + ? "explicit height (px)" + : `${variation.height} height` + second.title = `${widthExplanation}, ${heightExplanation}`; + + fragment.appendChild(first); + fragment.appendChild(separator); + fragment.appendChild(second); + + return fragment; + } + </script> + </body> +</html> diff --git a/layout/svg/nsSVGOuterSVGFrame.cpp b/layout/svg/nsSVGOuterSVGFrame.cpp index aeadccbc5..e1b97bb40 100644 --- a/layout/svg/nsSVGOuterSVGFrame.cpp +++ b/layout/svg/nsSVGOuterSVGFrame.cpp @@ -241,8 +241,9 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio() nsSVGLength2 &height = content->mLengthAttributes[SVGSVGElement::ATTR_HEIGHT]; if (!width.IsPercentage() && !height.IsPercentage()) { - nsSize ratio(NSToCoordRoundWithClamp(width.GetAnimValue(content)), - NSToCoordRoundWithClamp(height.GetAnimValue(content))); + nsSize ratio( + nsPresContext::CSSPixelsToAppUnits(width.GetAnimValue(content)), + nsPresContext::CSSPixelsToAppUnits(height.GetAnimValue(content))); if (ratio.width < 0) { ratio.width = 0; } @@ -272,8 +273,8 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio() if (viewBoxHeight < 0.0f) { viewBoxHeight = 0.0f; } - return nsSize(NSToCoordRoundWithClamp(viewBoxWidth), - NSToCoordRoundWithClamp(viewBoxHeight)); + return nsSize(nsPresContext::CSSPixelsToAppUnits(viewBoxWidth), + nsPresContext::CSSPixelsToAppUnits(viewBoxHeight)); } return nsSVGDisplayContainerFrame::GetIntrinsicRatio(); |