vidstige / NRasterizer

OpenType parser in pure c#
Apache License 2.0
38 stars 11 forks source link

Reorganised repository to support building Netstandard1.1 and NET20 shared package #36

Closed tocsoft closed 7 years ago

tocsoft commented 7 years ago

Hi @vidstige

Thanks for getting the nuget package one small problem with it is that it only has a NET20 library inside and not a netstandard one.

What I've done with this PR is moved all the core NOpenType code (that doesn't have target full framework dependencies) into a shared project and referenced that from the current Net20 project and added a new netstandard portable project also with the shared project referenced.

I also updated your travis build script and and nuspec so that it will create a nuget package with both the versions of the library in.

What this means is now new core development on the font loading glyph manipulation code can happen in the shared project and everything keep in sink and updated can everything will keep working.

(please feel free to not pull this PR in directly and use it if you want as inspiration to get a netstandard build out that fine.)

tocsoft commented 7 years ago

I'll rebase to fix those conflicts now.

I also noticed that the project is currently setup to create a dll called NRasterizer.dll and it has the namespace NRasterizer throughout, while i'm in here moving everything around do you want me to change them both/either to NOpenType to match the package name?

tocsoft commented 7 years ago

😠 grr.. looks like travis doesn't like the netstandard project, this might require some thinking about.

tocsoft commented 7 years ago

ok looks like the issue might actually be with the version of nuget that comes with mono... now fighting with tweaking the build script to download the latest nuget.exe and then use that one instead of the one bundled with mono.

tocsoft commented 7 years ago

sigh... I hate mono, I'm sure this would be so much simpler using https://www.appveyor.com/ no mono/linux stuff then just msbuild on a windows box... but i'm sure i'll figure out the issue, just might take me a while. 😢

vidstige commented 7 years ago

Mmm, perhaps easier to go with appveyor instead then?

tocsoft commented 7 years ago

I think I give up... 😞 stupid linux, if you could try getting an appveyor build up and running that would be awesome and I can then pretend travis-ci doesn't exist.

vidstige commented 7 years ago

haha, it's not optimal for windows stuff. I have no experience with appveyor. Perhaps you could set up a sample build?

As for the split of projects it seems there are a lot of projects in here. What's the restrictions in netstandard? Is it not possible to open files, etc? What API calls are the problems?

tocsoft commented 7 years ago

Sure, i'll setup an appveyor build on my fork that you should be able to copy.

The reason for having to spilt the project up is trying to maintain a net20 compatibility.. I'll see if I can figure out a cleaner way for maintaining the widest compatibility I can muster. Its tricky to make a simple project layout that's easy to maintain and targets multiple runtimes simultaneously.

I'll have more of a play tonight and see if I can't get something running smoothly from my fork that can target the widest set of runtimes I can and auto builds.

prepare commented 7 years ago

@vidstige , It is OK for me to use a shared code technique. you don't need to maintain a compat with NET20.

tocsoft commented 7 years ago

OK I think I've figured this all out now, managed to get a build pipeline working with appveyor that should mean we only need a single portable/netstandard project that be used to produce a single nuget package that will target net20 and netstandard1.1. This pipeline will build, automagically update versions , build packages, and and run all unit tests.

@vidstige you will need to setup an appveyor build for this, and once this PR lands in master it will start producing build/packages per commit to master. To try it out locally you can just run build.cmd in the root.

NOTE: the packages it produces will all be preview (X.X.X-ciXX) builds. To force appveyor to produce a releasable package (X.X.X without the suffix) then all you will need to do is add a tag with the new version number as the tag name, that will trigger a new build that you could release to nuget.org

tocsoft commented 7 years ago

Once/if this lands i'll start throwing over some issues/feature requests I've come across while i'm attempting to implement it in ImageSharp (hopefully with accompanying PRs).

It would also be great where yours and @prepare codebases have diverged could be merged back together... we don't want multiple version of this code all over the place, all slightly different and incompatible, do we.

tocsoft commented 7 years ago

turns out Unicode encoding was a bad choice, it needed to be UTF8.

I added a unit test to ensure that the library can at least load fonts now.

vidstige commented 7 years ago

@tocsoft awesome, thank you so much! :-) I'm travelling right now and only got my macbook air with me. But if it builds on appveyor I'm ready to merge, just going to eye through the diffs.

I don't see any reason to keep net 2.0 compatibility, but really the lib only reads stuff from a stream so we should target as low runtime as possible. But it would be cool if we can have a high version of the language to use e.g. extension methods, etc.

As for tests, there are some awesome end-to-end tests that loads a font and renders sample to an image.

tocsoft commented 7 years ago

there that's done it.

It wan't a bother to port generate_sample.sh to generate_sample.cmd i've made appveyor call this directly and then push it up the generated images as a build artefact. As for managing to get them into S3 I think your on you own 😉

vidstige commented 7 years ago

Wow, the generate_sample.cmd got you the extra credit. Just checking through the last diffs real quick and then hitting the button.

vidstige commented 7 years ago

★★★★★

vidstige commented 7 years ago

Alright, I hit the switch over at appveyor and green on first attempt. And wow, is the build faster now. :+1:

https://ci.appveyor.com/project/vidstige/nrasterizer

I just missed one thing - The build status indicator. But I'll fix that. :-)

tocsoft commented 7 years ago

yay... got there in the end. 😁