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

Stub for non-Windows DPI; possible fix for #427 #450

Merged
merged 3 commits into from
May 9, 2019

Conversation

chucker
Copy link
Contributor

@chucker chucker commented May 9, 2019

No description provided.

garyranson and others added 2 commits May 7, 2019 19:19
- support v3.5, v4.0, v4.5.2, v4.7.2 and dotnetcore2.2
- added support for Appveyor build
- added explict FrameworkPathOverride to projects
- added <IsTextProject> to csproj file
- remove net35 from unittests. Updated licence method.
- removed / replaced EXCLUDE defines
- added back System.Web
- added back projects for SVG viewer and test runner
- added back AssemblyInfo, no not generate it in .NETCore
- removed FrameworkPathOverride - seems not to work correctly with current version
- removed unneeded framework versions
- don't use Win32-specific interop calls to determine the DPI
  if not on Windows
- possible fix for svg-net#427
- add a barebones unit test for this
- note that even the Windows code isn't correct as soon as you have multiple screens
- on macOS, causes 20 instead of 5 unit tests to pass
@mrbean-bremen
Copy link
Member

Ok, I cleaned up the commits a bit and will merge this in a moment.

@mrbean-bremen mrbean-bremen merged commit 88026af into svg-net:master May 9, 2019
@mrbean-bremen
Copy link
Member

You may need to re-branch from master now, as I force-pushed to your branch - sorry for that...

@mrbean-bremen
Copy link
Member

And thanks a lot for the fix - any help is welcome!
Specifically, any help to test/check the stuff under Linux/MacOS would help, for example, setting up CI tests for Linux. Generally, any help to reduce the number of open issues, to add documentation, or to improve platform support is appreciated.

@chucker
Copy link
Contributor Author

chucker commented May 10, 2019

Anything to help move .NET Standard support forward I benefit from, so… :-)

I’ll have to read up on how the AppVeyor stuff works. I assume I could theoretically make pull requests to that config to add Linux CI builds?

(And there’s a few remaining railing unit tests; a glance suggested it may be a path problem, which sounds plausible enough. I’ll try and have a look at those.)

@mrbean-bremen
Copy link
Member

I’ll have to read up on how the AppVeyor stuff works. I assume I could theoretically make pull requests to that config to add Linux CI builds?

Yes, of course! You can setup AppVeyor for your own repo, so you can test it in your own branch.
I haven't used AppVeyor for Linux so far (used Travis.CI for that, but that have been Python projects), but there seems to be Linux support specifically for .NETCore.

@arendvw
Copy link

arendvw commented May 27, 2019

Just tested this build on debian buster. I still needed libgdiplus installed from the mono repositories for this to work; but it did work. Is this to be expected?

Update: It seems this is required for System.Drawing under linux, and is not a dependency of this project.

@mrbean-bremen
Copy link
Member

Ok, good to know that!

@gvheertum
Copy link
Member

Just tested this build on debian buster. I still needed libgdiplus installed from the mono repositories for this to work; but it did work. Is this to be expected?

Update: It seems this is required for System.Drawing under linux, and is not a dependency of this project.

MacOs has a similar issue, when running the tests an error is thrown regarding the libgdiplus libraries not available. For MacOs there is a possibility to stub the libgdiplus by including the runtime.osx.10.10-x64.CoreCompat.System.Drawing Nuget (see: https://github.com/CoreCompat/libgdiplus-packaging).

I'm not sure if there is an option to have specific Nuget settings for specific configurations (since this is most likely not to work on Windows or Linux), but to get the package up and running on MacOs I think having the CoreCompat package might be the most usable option.

@mrbean-bremen
Copy link
Member

Thanks for digging into this! I'm not aware of a way for OS-specific Nuget settings, but we need at least the instructions how to install the package under Linux and MacOS somewhere with the package - probably with the package description.
@tebjan - what do you think?

@mrbean-bremen mrbean-bremen mentioned this pull request Jun 25, 2019
@chucker
Copy link
Contributor Author

chucker commented Jun 25, 2019

Haven't tried, but something like this might work:

  <PropertyGroup Condition="'$(RuntimeIdentifier)'=='osx.10.10-x64'">
    <PackageReference>runtime.osx.10.10-x64.CoreCompat.System.Drawing</PackageReference >
  </PropertyGroup>

It might also be that you don't have to do any of that and can just add the reference regardless (it might just be a noop on other platforms).

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Jun 25, 2019

It might also be that you don't have to do any of that and can just add the reference regardless (it might just be a noop on other platforms).

Definitely not - just tried it and got it working again only after manually deleting all object files and restarting the solution.
Anyway, we still would need documentation for linux.

@gvheertum
Copy link
Member

gvheertum commented Jun 25, 2019

It might also be that you don't have to do any of that and can just add the reference regardless (it might just be a noop on other platforms).

Definitely not - just tried it and got it working only after manually deleting all object files and restarting the solution.
Anyway, we still would need documentation for linux.

Yeah, I'm also not sure about having this kind of stuff included by default. This might be very breaking. I've been playing a bit with the settings, but haven't got it to work yet (it seems to not pickup the RuntimeIdentifier somehow).

From what I've read/done until now, the MacOs package needs to be included in the SVG library and needs to be taken all the way up to the executable/library consuming the SVG component (in that way it differs from the Linux version which seems to install the gdi libs in your global scope). MacOs with the CoreCompat package will put the stubs together with the project and apparently it will "auto resolve" to the CoreCompat implementation for Mac instead of letting the framework figure it out (not having the package will result in errors).

I know that you can setup NuGet packages to have different content/deploys for different frameworks and versions of the framework, but I'm not sure if you can do platform specific stuff. If that's not possible the users somehow need to get the gdi stuff global on their system or build their own binaries (with additional instructions).

One of the options (which would be more complex) is to put a proxy between everything gdi related coming out of the SVG (for example, I've seen it break on Matrix) and check if the GDI is available, if not, load the alternative route (for Mac and Linux), but I'm afraid that might be quite a big change.

@mrbean-bremen
Copy link
Member

I think for the time being some simple instructions will do (like the ones in your link - I copied them over to the NetCore 2.2 PR discussion). Anything more complicated may be added later, though I think your last option (proxying GDI access) would be a bit of overkill.

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.

5 participants