Repository navigation
Conversation
`Orient` has listed 'auto-start-reverse' as a valid value since markers were added, but neither native renderer implements it. Android passes every non-"auto" value to Double.parseDouble, so the string throws a NumberFormatException out of MarkerView.renderMarker — on the UI thread inside SvgView.onDraw, where no JS error boundary can catch it, taking the app down. Apple does not throw, because doubleValue answers 0 for a non-numeric string, so the marker is drawn at the wrong angle instead. The angle maths already exists: CurrentAngle adds the half turn for a start marker when auto_start_reverse_ is set. Nothing ever set that flag — on Android it was marked TODO. So finish the wiring rather than add a parallel path: renderMarkers already resolves the start marker before computing positions, so it passes whether that marker is auto-start-reverse into fromPath / fromCGPath:. MarkerView and RNSVGMarker then treat the value like "auto", and an orient that is neither "auto" nor a number falls back to the property's initial value of 0 instead of throwing.
Author
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
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
A marker with
orient="auto-start-reverse"crashes the app on Android, and draws at the wrong angle on Apple platforms.Orienthas listed'auto-start-reverse'as a valid value since markers were added (src/elements/Marker.tsx), but neither native renderer implements it. Android passes every non-"auto"value toDouble.parseDouble:That runs on the UI thread inside
onDraw, where no JS error boundary can catch it, so a single such SVG takes the whole app down. Apple does not throw —[_orient doubleValue]answers0for a non-numeric string — so the marker is silently drawn at the wrong angle instead.The angle maths for this value already exists in the library:
CurrentAngleadds the half turn for a start marker whenauto_start_reverse_is set (RNSVGMarkerPosition.java,RNSVGMarkerPosition.mm). Nothing ever set that flag; on Android it was even marked// TODO. So this finishes the wiring rather than adding a parallel path:renderMarkersalready resolves the start marker before computing positions, so it passes whether that marker isauto-start-reverseintofromPath/fromCGPath:, which sets the existing flag.MarkerView/RNSVGMarkertreat the value like"auto", and an orient that is neither"auto"nor a number falls back to the property's initial value of0instead of throwing.Test Plan
What's required for testing (prerequisites)?
Two SVGs that differ in exactly one attribute —
orient="auto"versusorient="auto-start-reverse"— with the same marker on both ends, so the start marker is the only thing that may change:What are the steps to reproduce (after prerequisites)?
Render both with
SvgUrion Android.Before this change, the
auto-start-reverseone kills the process: the app disappears from the launcher and logcat carries theNumberFormatExceptionabove.After, measured on an arm64 Android 30 emulator (
react-native-svgbuilt from this branch;react-native-svg:compileDebugJavaWithJavacran clean):No
NumberFormatExceptionand noMarkerView.renderMarkerframe in logcat; the app stays up.Comparing the two renders pixel by pixel, the end marker is byte-identical (difference
0) and only the start marker changes — which is what the spec asks for.The start marker is not merely different, it is reversed. Measuring the triangle's thickness per column, its base and apex swap sides between the two values, and a browser rendering the same two files — Chromium implements SVG 2 here — swaps them the same way:
autoauto-start-reverseautoauto-start-reverseI have not run the Apple change on a device or simulator. It mirrors the Android one and follows from
doubleValuereturning0for a non-numeric string, but I have not verified it visually — happy to do that before merging if you'd like.Compatibility
Web already handled this — the markup reaches the browser's own SVG engine — so nothing changed there.
Checklist
README.md__tests__folderChecklist left unticked deliberately: Android was verified on an emulator only, the TypeScript already accepts this value, and I did not add a test — let me know if you'd like an
e2e/casesSVG for it.