Split ImageSharp into multiple projects about imagesharp HOT 28 CLOSED

releasing out-of-band isn't the only reason to split it up its also to save end users from having to take all the extra stuff they don't need, i.e. the font loading dependency if they don't want to draw text etc.

but I do agree it would be nicer to only version the parts that have changed and their dependents.

re: bootstraper

Couldn't we have both? Have a global default instance of the bootstraper that will get used by the current new Image(stream) methods but also have one that takes a bootstrapper?

public Image(Stream stream)
    : this(stream, Bootstrapper.Default)
{
}
public Image(Stream stream, Bootstrapper boostrapper)
{
    this.boostrapper = boostrapper;
    //do stuff in here with stream and bootstrapper.
}
public Image(Image parentImage)
{
    this.boostrapper = parentImage.boostrapper;
    //do stuff in here with parentImage and bootstrapper here
}

It would mean the only thing dependent on the static class/properties would be the simple constructors and everything internally would work in the referenced instance. But just to be clear I don't have a strong opinion either way, just offering up ideas.

from imagesharp.

Processors sounds good to me

from imagesharp.

I've actually already put an automated build process together for the split projects. My build process even supports tracking & incrementing build numbers when ever it or one if its project dependencies is altered.

The build process is designed to just work™ it should produce deterministic build numbers for each commit based on the branch your working on. With root version numbers managed within the project.json files.

https://github.com/JimBobSquarePants/ImageSharp/tree/tocsoft/split-projects (note: this branch might be rebased without warning)

All you need to do is run the build.cmd file in the root of the project and out will spit *.nupkgs properly versioned and will even clean up after itself to allow for local development to continue without added dirty state.

from imagesharp.

I wonder if we should always have them under the same version number. I think we only need to bump versions when something has changed in that area.

p.s. I wonder if we should split up the text drawing in a separate library but maybe we are taking a step to far then.

from imagesharp.

ImageSharp.Transforms
Contains methods like Resize, Crop, Skew, Rotate - Anything that alters the dimensions of the image.
ImageSharp.Effects
Contains methods like Gaussian Blur, Pixelate, Edge Detection - Anything that maintains the original image dimensions.

Transforms and Effects can be merged into one project now I have removed IImageSampler.

@dlemstra Drawing.Text should definitely be a separate library since we are introducing a dependency.

from imagesharp.

Totally agree about text drawing being its own thing... it defiantly brings along its own baggage that the rest of drawing doesn't need.

from imagesharp.

@JimBobSquarePants then what should it be called? ImageSharp.Transforms?

from imagesharp.

@tocsoft Maybe ImageSharp.Processors? Transforms are a specific form of image processing. Dunno though, naming is hard!

from imagesharp.

We'll really have to put some thought to how to build, version, and deploy all those projects though. At the moment I have no idea how we will manage it all.

from imagesharp.

@dlemstra To be honest my main reason for keeping the version numbers in sync between packages is simplicity.

Its much easier to have the build server manage version numbers without having to manually go into the code and update the versions in all the project.json when ever you need to bump the number.

If all projects in a solution share a version number you can blindly update the package number everywhere based on build and you can also know exactly which version the intern project dependency will be when building the nuget packages.

@JimBobSquarePants It should be simple enough if we don't mind bumping all version numbers all together otherwise its a lot more manual work to put out a release.

I've had a play creating an updated build project that uses gitversion and dotnet-version to handle all the package versions it will allow us to create a new final release by just adding a github release/tag and appveyor will build a full package without tag.

from imagesharp.

my main concern with the split is actually the formats, how are we expecting the formats to end up inside the bootstraper? I don't think you should rely on a user manually registering a format but then, from what I can see, that only leaves reflection based registration but that's then dependent on the dlls being loaded.

'Tis a quandary, half make me think its not worth splitting out the formats at all, think its defiantly something that will need playing with.

from imagesharp.

Changes to split out ImageSharp.Drawing @ c071ef2

Producing this build : https://ci.appveyor.com/project/JamesSouth/imagesharp/build/1.0.0-tocsoft-split-projects.1+2974.build.271/artifacts

To add a new project to the build pipeline so that it will create a new package versioned with the reset of the projects is as simple as adding the path in build\config.cmd this will ensure that that version numbers are all auto-magically updated.

This is just a nice early preview of what I'm thinking (added bonus this drops the node.js dependency from the build).

from imagesharp.

How about Processing instead of Processors? Seems a bit more customer friendly to me 😄 and then inside we will have effects and transforms, each with their own namespace?

I had a similar thought with Text being its own thing, but based on the dependencies argument I now agree.

Also I am in favour of keeping the library as a whole with one version number. I see no reason to break it down. Let's say the processing DLL doesn't change but there are changes in the core which the processing depends on. Deosnt that count as a new version anyway, since the behaviour has changed? :Smile:

Great effort @tocsoft, I think this is definitely the right thing to do!

from imagesharp.

👍 for not placing all the stuff under a single ImageSharp namespace. It would be soooo C++!

👍 for having a single version number for simplicity!

from imagesharp.

@tocsoft What you've demoed is really cool but it introduces an issue. What happens if there are no changes within individual projects? Upping the version number and releasing a new package when there are no changes breaks semantic versioning which is a big no-no.

Perhaps we could figure out a way to detect whether the individual projects have changed (via git) and only bump the version number then?

On Formats... I intend for users to have to register formats in the manner Asp.NET Core requires registering Middleware and other objects - Convention always trumps configuration.

Reflection is a nightmare, take a look at the crap I have to do in ImageProcessor.

https://github.com/JimBobSquarePants/ImageProcessor/blob/bfa1105ff6dc9d517740b12b1ad598e20d84afb8/src/ImageProcessor/Configuration/ImageProcessorBootstrapper.cs#L95

https://github.com/JimBobSquarePants/ImageProcessor/blob/bfa1105ff6dc9d517740b12b1ad598e20d84afb8/src/ImageProcessor/Common/Helpers/TypeFinder.cs

@olivif Processing works for me. Namespacing also.

@antonfirsov I'd like to keep some stuff under the same flat namespace Common, Helpers, Exceptions, Image, PixelAccessor etc. In VS2017 we will be able to explicitly state folders as non namespaced with Resharper so warnings will go away.

from imagesharp.

On Formats... I intend for users to have to register formats in the manner Asp.NET Core requires registering Middleware and other objects - Convention always trumps configuration.

Having a static bootstrapper reminds me the service locator anti-pattern. Shouldn't a bootstrapper/configuration be an optional parameter on methods and Image constructors? Or shouldn't we integrate aspnet/DI somehow?

from imagesharp.

We shouldn't use the Asp DI since that would be a dependency limiting our deployment possibilities.

A static Bootstrapper allows third parties to create formats and then register them without having to use reflection or any other craziness. It also works in every conceivable application environment I can think of. (Reflection don't work with statically linked libraries for example)

If I add it to Image as a constructor argument I then have to use that overload for all images (including internally created ones) which simply wouldn't be possible.

That article is interesting but to me it boils down to application requirements.

from imagesharp.

What you've demoed is really cool but it introduces an issue. What happens if there are no changes within individual projects? Upping the version number and releasing a new package when there are no changes breaks semantic versioning which is a big no-no.

Perhaps we could figure out a way to detect whether the individual projects have changed (via git) and only bump the version number then?

That'll be a nightmare, its not just tracking file changes in the project itself its also having to bump things on dependency changes, so you would end up needing to calculate the dependency chain for each project, and then start calculating the versions from there (tagging/suffixing as required).

hmm.. actually I think I might have an idea on how to do it the more I think about it... I'll have a play tonight and see if i can get something working.

from imagesharp.

@JimBobSquarePants what about users who need different bootstrapping configurations in a single process? It's uncommon, but it can happen in extreme cases with laaaaarge applications.

from imagesharp.

Sweet. Looking forward to seeing what you come up with.

Basically, as it stands, without correct individual versioning there is no advantage to be gained in splitting up the library since we cannot release individual projects out-of-band.

@JimBobSquarePants what about users who need different bootstrapping configurations in a single process? It's uncommon, but it can happen in extreme cases with laaaaarge applications.

Tell them to bugger off for being awkward.

from imagesharp.

Basically, as it stands, without correct individual versioning there is no advantage to be gained in splitting up the library since we cannot release individual projects out-of-band.

Disagree! Advantages could be:

• For users: Reduced number of dependenciess --> smaller deployment package size
• For us: Modularized ImageSharp solution, better separation

Tell them to bugger off for being awkward.

🤣 You're right, different configuration requirements should be very uncommon for an image processing library.

from imagesharp.

@tocsoft I was thinking about the same actually. There's no need to push a complicated API towards basic users.
@JimBobSquarePants maybe it's YAGNI, but who knows, the zoo of users is large :D

from imagesharp.

Something else that just occurred to me, is Extension methods.

I'm assuming all extension methods targeting ImageBase<TColor> should live in the ImageSharp namespace no matter the packages namespace?

e.g. DrawPolygon(..) would be available in ImageSharp namespace event though the Polygon class will live inside ImageSharp.Drawing...

from imagesharp.

@antonfirsov I was being tongue -in-cheek about the buffer off thing. @tocsoft solution seems like a sound plan.

Extension methods... Yeah, I guess they should all be flat. It makes discovery much easier for the end-user.

from imagesharp.

Looks good to me on the progress splitting ImageSharp into separate projects. If the decision is to version separately then is it worth splitting each project into a separate GitHub repo? You can then allow CI builds to version on their own, and set release versions independently with a git tag. This is the approach I took with https://github.com/OkraFramework, and is how https://github.com/aspnet is organised.
@JimBobSquarePants You can create an 'ImageSharp' GitHub organisation to keep all the relevant projects together... And you get a nice imagesharp.github.io domain via GitHub pages. 😄

from imagesharp.

As the project is being split into multiple projects, it is important to have a robust and consistent build process across multiple CI servers, and for multiple contributors. Is this a good point to introduce a build automation system?

I have quite a bit of experience of doing this using Cake (http://cakebuild.net) from my Okra Framework project. This is cross-platform, highly configurable via C# and can run dotnet builds, run tests, patch assembly versions, create NuGet packages etc.

This could all be done as a separate PR once the projects have been split up. I would be happy to pick this one up if people are interested...

from imagesharp.

Awesome! A build.cmd script is a big step forward. And deterministic versions for each commit is great 👍

From experience I would still vouch for a build automation system, and this can be independent of the actual logic used (since Cake is C# based then your versioning logic could be copied and pasted directly too). It is more used as a task runner, handling which tasks to run along with their dependent tasks. The main advantage is robustness and flexibility, for example imaging being able to type,

• build to do the versioning and builds, along with failing the build with a failed unit test, generation of documentation, etc. And will clean everything up correctly even with a failed build.
• build RunBenchmarks to automatically build everything correctly and run the benchmarks
• build RunTests --Configuration=debug to run tests on a debug build

This can all be done with simple build scripts, but it is way simpler if a build framework like Cake is used.

from imagesharp.

PS... Cake is fully cross-platform too, so only one set of build scripts to write for Windows/Linux/OS X too. 😉

from imagesharp.