Skip to content

fix: support orient="auto-start-reverse" on markers - #3043

Open
chphch wants to merge 1 commit into
software-mansion:mainfrom
chphch:fix/marker-auto-start-reverse
Open

chphch wants to merge 1 commit into
software-mansion:mainfrom
chphch:fix/marker-auto-start-reverse

Conversation

@chphch

@chphch chphch commented Sep 3, 2026

Copy link
Copy Markdown

Summary

A marker with orient="auto-start-reverse" crashes the app on Android, and draws at the wrong angle on Apple platforms.

Orient has 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 to Double.parseDouble:

java.lang.NumberFormatException: For input string: "auto-start-reverse"
  at com.horcrux.svg.MarkerView.renderMarker(MarkerView.java:125)
  at com.horcrux.svg.SvgView.onDraw(SvgView.java:135)

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] answers 0 for 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: CurrentAngle adds the half turn for a start marker when auto_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:

  • renderMarkers already resolves the start marker before computing positions, so it passes whether that marker is auto-start-reverse into fromPath / fromCGPath:, which sets the existing flag.
  • MarkerView / RNSVGMarker 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.

Test Plan

What's required for testing (prerequisites)?

Two SVGs that differ in exactly one attribute — orient="auto" versus orient="auto-start-reverse" — with the same marker on both ends, so the start marker is the only thing that may change:

<svg xmlns="http://www.w3.org/2000/svg" width="400" height="120" viewBox="0 0 400 120">
  <rect width="400" height="120" fill="#ffffff"/>
  <defs>
    <marker id="a" viewBox="0 0 10 8" refX="5" refY="4" markerWidth="14" markerHeight="11" orient="auto-start-reverse">
      <path d="M0 0 L10 4 L0 8 z" fill="#000000"/>
    </marker>
  </defs>
  <path d="M80 60 L320 60" stroke="#000000" stroke-width="6" marker-start="url(#a)" marker-end="url(#a)"/>
</svg>

What are the steps to reproduce (after prerequisites)?

Render both with SvgUri on Android.

Before this change, the auto-start-reverse one kills the process: the app disappears from the launcher and logcat carries the NumberFormatException above.

After, measured on an arm64 Android 30 emulator (react-native-svg built from this branch; react-native-svg:compileDebugJavaWithJavac ran clean):

  • No NumberFormatException and no MarkerView.renderMarker frame 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:

    left half right half
    Android auto 6840 2340
    Android auto-start-reverse 2340 6840
    Chromium auto 3707 1243
    Chromium auto-start-reverse 1243 3707

I have not run the Apple change on a device or simulator. It mirrors the Android one and follows from doubleValue returning 0 for a non-numeric string, but I have not verified it visually — happy to do that before merging if you'd like.

Compatibility

OS Implemented
iOS ✅
MacOS ✅
Android ✅
Web ✅

Web already handled this — the markup reaches the browser's own SVG engine — so nothing changed there.

Checklist

  • I have tested this on a device and a simulator
  • I added documentation in README.md
  • I updated the typed files (typescript)
  • I added a test for the API in the __tests__ folder

Checklist 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/cases SVG for it.

`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.
@chphch

chphch commented Sep 3, 2026

Copy link
Copy Markdown
Author

Visual evidence for the test plan above.

Chromium rendering the two files — it implements SVG 2, so this is what the start marker is supposed to do (top auto, bottom auto-start-reverse):

chromium reference

The same two files on the Android emulator with this branch, same order:

android with this change

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.

1 participant