Fix infinite recursion in bezier stroke hit-testing at extreme zoom - #58
Open
Gamedirection wants to merge 1 commit into
Open
Fix infinite recursion in bezier stroke hit-testing at extreme zoom#58Gamedirection wants to merge 1 commit into
Gamedirection wants to merge 1 commit into
Conversation
d2d_point_on_bezier_segment() recurses on itself to subdivide a stroked bezier curve until its approximation error drops below tolerance. It had no recursion depth limit, unlike the sibling function d2d_figure_add_cubic_bezier_recursive(), which already caps at depth 16. At extreme zoom levels, the transform matrix scale grows large enough that the transformed coordinates this function computes exceed float32's mantissa precision (2^23). Past that point the error estimate no longer shrinks as the curve subdivides, so the function recurses without bound instead of terminating. Root caused and reported by DenisJosifoski in seapear/AffinityOnLinux issue #134 (affinity apps freeze completely at extremely high zoom on Wine, first observed on an NVIDIA hybrid Fedora Wayland setup). They measured the exact freeze point at 8,232,116% zoom, matching the 2^23 float32 mantissa limit almost exactly. Adds the same depth cap the sibling function already uses. Past that depth the curve is treated as not on the stroke rather than looping indefinitely, the same fallback direction (false) as the function's own two fast-exit checks already use for definitely-not-on-curve cases. Verified geometry.c compiles cleanly with this change (0 new errors or warnings). Could not verify the full d2d1.dll build end to end, device.c fails to compile against this machine's Wine headers with or without this change, a pre-existing, unrelated issue.
3 tasks
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.
Summary
Fixes infinite recursion in bezier stroke hit-testing at extreme zoom levels, reported in seapear/AffinityOnLinux#134.
Root cause
d2d_point_on_bezier_segment()recurses on itself to subdivide a stroked bezier curve until its approximation error drops below tolerance. It had no recursion depth limit, unlike the sibling functiond2d_figure_add_cubic_bezier_recursive(), which already caps at depth 16.At extreme zoom levels, the transform matrix scale grows large enough that the transformed coordinates this function computes exceed float32's mantissa precision (2^23). Past that point the error estimate no longer shrinks as the curve subdivides, so the function recurses without bound instead of terminating.
DenisJosifoski root-caused this precisely in the issue thread: they measured the exact freeze point at 8,232,116% zoom, matching the 2^23 float32 mantissa limit almost exactly, and pointed at the
d2d1.dllmatrix transform as the likely cause.The fix
Adds the same depth cap the sibling function already uses (20 levels here, since this recursion halves the curve each time rather than converting cubic to quadratic). Past that depth, the curve is treated as not on the stroke rather than looping indefinitely, the same fallback direction (
FALSE) the function's own two fast-exit checks already use for definitely-not-on-curve cases.Test plan
geometry.ccompiles cleanly with this change, 0 new errors or warnings (verified viamake TARGET=x86_64-unixinWineFix/lib/d2d1).d2d1.dllbuild end to end,device.cfails to compile against this machine's Wine headers with or without this change (confirmed viagit stash), a pre-existing, unrelated issue.d2d1.dllto test with on this machine. Would appreciate someone with a working WineFix build environment confirming this actually resolves the freeze.