Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow skia library load to fail gracefully #775

Closed

Conversation

webprofusion-chrisc
Copy link

Issues

Fixes #774

Proposed changes

The current SkiaCanvas implementation fails when an exception is thrown locating the libSkiaCanvas library. This change emits a warning instead of an exception and proceeds. This enables the use of the SkiaCanvas renderer in Avalonia UI (see tablature section of test app https://scalex.soundshed.com/)

Checklist

  • [ X] I consent that this change becomes part of alphaTab under it's current or any future open source license
  • [ X] Changes are implemented
  • [ X] Existing builds tests pass
  • New tests were added

Further details

  • This is a breaking change
  • This change will require update of the documentation/website

@webprofusion-chrisc webprofusion-chrisc changed the title All skia library load to fail gracefully Allow skia library load to fail gracefully Mar 2, 2022
Copy link
Member

@Danielku15 Danielku15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of putting a try-catch around the related block, could you rather try to handle this scenario gracefully without an exception? We are already here within a switch which handles the platform specific cases and we just run into the default case because it was easier to assume Windows platform.

I would assume when running .net with WASM the platform might be set to other as documented in the MSDN. We should rather avoid executing the windows specific path by handling the particular case than swallowing the exception.

@webprofusion-chrisc
Copy link
Author

I did anticipate you might not like the try/catch and did try it the other way already - however PlatformID.Other does not appear to be available in .net standard (it originated in 2020 so perhaps it's just part of .net 5/.net 6), either way their doc appears to be wrong.

To test for PlatformID.Other (7) we'd instead have to cast the platform to an int etc, which may be worse?

   switch ((int)System.Environment.OSVersion.Platform)
            {
                case (int)PlatformID.MacOSX:
                case (int)PlatformID.Unix:
                case 7:
                    // I think unix platforms should be fine, to be tested
                    break;
                default:

@Danielku15
Copy link
Member

@webprofusion-chrisc Just had a bit more time to read about the current state and I think with mono/SkiaSharp#713 being fixed in mono/SkiaSharp@d15cb24 we can already remove the whole switch and the LoadLibrary method. SkiaSharp handles the platform loading itself since 2.80.0 and we are on 2.80.3

No need for alphaTab to do the loading again.

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.

SkiaCanvas constructor fails in WASM (Avalonia UI)
2 participants