Skip to content

Needed patch to avoid Access Violations for paths without previous points#191

Open
NicklasBergfeldt wants to merge 7 commits intoAngusJohnson:mainfrom
NicklasBergfeldt:main
Open

Needed patch to avoid Access Violations for paths without previous points#191
NicklasBergfeldt wants to merge 7 commits intoAngusJohnson:mainfrom
NicklasBergfeldt:main

Conversation

@NicklasBergfeldt
Copy link
Copy Markdown

No description provided.

…nt TSvgReader instance (under concurrent rendering) or elements of the wrong type (hash collision on short single-letter IDs like a, b, c in the SVG). The original code hard-casts these results without type checks and accesses renderers through with statements that resolve fSvgReader ambiguously.

Fix 1 - const drawDat on PrepareRenderer (lines 463, 473, 483, implementations): prevents by-value copy of the large TDrawData record, eliminating stack corruption during parameter passing.
Fix 2 - Renderer re-read in PrepareRenderer (TLinGradElement, TRadGradElement): both implementations now ignore the renderer parameter and re-read from fSvgReader.LinGradRenderer / fSvgReader.RadGradRenderer at method entry. Eliminates parameter corruption traced through debugging where the renderer pointer changed between call site and function entry.
Fix 3 - DrawFilled/DrawStroke use refEl.fSvgReader: gradient rendering accesses renderers through the gradient element's own reader (refEl.fSvgReader) instead of the shape's reader (Self.fSvgReader). Removes with TLinGradElement(refEl) do pattern that resolved fSvgReader ambiguously.
Fix 4 - AV guard in TGradientElement.PrepareRenderer: wraps el is TGradientElement in try/except to catch corrupt element pointers from FindRefElement, converting AV to a descriptive exception with the bad refEl string and gradient ID.
Fix 5 - Type-safe FindRefElement casts: added is type checks before all hard casts of FindRefElement results in TGroupElement.Draw (TMaskElement, TClipPathElement, TFilterElement) and TShapeElement.Draw (same three). Previously, FindRefElement returning a wrong-type element (e.g., linearGradient "b" when looking up a clipPath) caused hard-cast to garbage fields -> AV in CopyPaths or TFilterElement.Clear.
@AngusJohnson
Copy link
Copy Markdown
Owner

Hi Nicklas.
Thank you for your feedback and for your PR.
However, there are a few things I'd like to discuss before merging.

  1. Because my intention is to keep this library backward compatible to Delphi 7 (at least for the time being), inline vars will need to be removed.
  2. You're suggesting putting a guard again an invalid return values from FindRefElement in Img32.SVG.Reader. But I would much prefer to find out why this function misbehaves rather than simply workaround its broken behaviour.
  3. Likewise for your guard against self-referencing markers.

It would really help if you could provide minimal test SVG images that generate the errors you are trying to fix.
Cheers.

@NicklasBergfeldt
Copy link
Copy Markdown
Author

Hello,

Sure; please find attached a few troublesome SVGs

Best regards.
Back
freedom-4
headbg
logo-varnish
Presentkort Tranan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants