Repository navigation
A curve with a point that is not a number is not flattened without end - #38
Merged
Merged
Conversation
addQuad and addCubic clamp the points of a curve that reaches beyond maxCoord and flatten it between the clamped points. A point that is NaN stays NaN when it is clamped, and still counts as beyond: the function called itself until the stack was used up, which ends the process — a fatal error, not a panic a caller could recover. AddPath marks a path with such a point and drops it, but only after the curve was added. A point of a path can be NaN as it is given, or become it under the matrix: infinity turned by a quarter is 0·∞. Such a curve now adds nothing, in the rasterizer and where a Shape records its edges.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Rasterizer.addQuadandaddCubic(and the same two where aShaperecords its edges) clamp the points of a curve that reaches beyondmaxCoordand flatten it between the clamped points, by calling themselves. A point that is NaN stays NaN when it is clamped and still counts as beyond: the function called itself until the stack was used up.That is a
fatal error: stack overflow— it ends the process, and norecoverof a caller catches it.AddPathmarks a path with such a point and drops it, but only after the curve was added. A point can be NaN as the path gives it, or become it under the matrix: a coordinate at infinity under a matrix with a zero in that place (a quarter turn) is 0·∞. A consumer that draws what a file says can be made to end this way.The fix
A curve with a point that is not a number adds nothing (
notNumbers), in all four places. The path is dropped as before.Checked
TestCurveNotNumbers: a cubic and a quadratic with a NaN control point, and a cubic with one at infinity under a quarter turn — throughRasterizer.Fill,Canvas.Fill,Stroke,ClipPath, andShape.SetFill/SetStrokewithFillShape. Without the fix the test binary dies.FuzzFillfound it within seconds; with the fix it andFuzzStrokeran a minute each without a finding, on top of A pixel gets the same value in whatever region it is drawn #37.staticcheck.Wants a release: consumers are exposed until they take it.