[Bug] Some icons render incorrectly or throw warnings when used in iOS, macOS xcasset bundles

[eliperkins]

Describe the bug

I put the SVG assets from the v17.9.0 release of Octicons into an xcasset bundle, enabling "Preserve Vector Data", and built a viewer for the Octicons within a macOS app.

A number of the icons rendered odd paths, resulting in improper display of the icons.

List of affected icons

• broadcast-16
• broadcast-24
• copilot-48
• copilot-96
• copilot-warning-16
• eye-closed-24
• file-badge-16
• gear-24
• history-16
• issue-draft-16
• issue-draft-24
• issue-reopened-16
• iterations-16
• iterations-24
• link-16
• link-24
• mention-16
• mention-24
• meter-16
• moon-16
• number-24
• paintbrush-16
• paperclip-16
• paste-16
• paste-16
• people-16
• people-24
• person-16
• person-24
• person-add-16
• person-add-24
• person-fill-16
• person-fill-24
• question-16
• rss-16
• rss-24
• squirrel-16
• squirrel-24
• tools-24
• unmute-16
• unmute-24
• unverified-16
• webhook-16

Steps to reproduce

For Hubbers:

1. Run the Octicons.app from github/octicons

For non-Hubbers:

1. Create a new iOS or macOS app
2. Add any of the affected Octicons to the app as an SVG asset within an xcassets bundle
3. Render the Octicon within the app

Expected behavior

Octicons render on iOS and macOS in the same manner that they do within web browsers, without the need for modifications.

Screenshots

Please add screenshots to help explain the problem.

gear-24	history-16	issue-draft-16	iterations-24	person-24

Device details

Desktop (please complete the following information):

• OS: macOS 12+
• Browser: n/a
• Version: n/a

Smartphone (please complete the following information):

• Device: all iOS devices
• OS: iOS 15+
• Browser: n/a
• Version: n/a

Additional info

Currently, GitHub Mobile uses the PDFs as exported from the Octicons site, which render fine! It seems to be the format of the SVGs that iOS and macOS don't like.

However, if we can build SVGs that work on iOS and macOS, we can leverage the versioning of the npm package to have Octicons be version-bumped by Dependabot automagically, keeping GitHub Mobile's icons consistently up-to-date.

[eliperkins]

Dropping some more debugging logs from setting CORESVG_VERBOSE=true within the app as well:

• A/a command was given the wrong number of floats.: this is the most common error, printed about the same number of times as failing icons that we have
• Warning: ellipses path has invalid rx or ry value. Drawing line.: this mainly seems to be an indicator that something is going to draw differently than expected, but doesn't really let us know where
• Error: this application, or a library it uses, has passed an invalid numeric value (NaN, or not-a-number) to CoreGraphics API and this value is being ignored. Please fix this problem: this is printed only a few times

Unfortunately, all of the above errors are logged without the names of the actual icons, as there's not a way to have the verbose debugging logs print the icon names themselves.

[gavinmn]

Had a chance to go over this with @eliperkins and we identified the issue with the SVG attribute fill-rule="evenodd" on the affected icons. There's a Figma plugin I've used in the past to fix this issue that we should be able to run on the flattened icons and publish an update. In the future, changing the fill rule to nonzero instead of evenodd before publishing should avoid this issue.

[tallys]

Hey @lesliecdubs can we get some engineering backup on automating checking this?

[lesliecdubs]

@tallys to confirm, you're looking for engineering support to automate changing any instances of the fill rule evenodd in this repo to nonzero? I imagine we can fit this into the docket in early Q3 unless there's more urgency around this ask.

[tallys]

Yep, just automating that rule. And, not too much urgency, but would love to make sure it doesn't get lost in the shuffle.

[gavinmn]

🤔 I'm not sure this is our solution — changing any instance of evenodd to nonzero (in a find and replace text way) won't work as far as I can tell. There needs to be changes to the SVG data which I've only been able to achieve when making the SVG in my experimentation. I think the rules have to do withe the defined direction of the path being drawn.

[gavinmn]

A test to check SVG's before they merge would be useful going forward tho!

[lesliecdubs]

@gavinmn do you want to spend a little more time with this to determine a potential solution, or do you want some engineering support to help figure that out? Or does adding the test you suggested feel like a good enough improvement for us to move forward with?

[gavinmn]

I'll keep looking into it — tools like https://github.com/googlefonts/picosvg might be able to help do this!