Repository navigation
Externalize ImageSharp.Drawing #1002
Description
Prerequisite: #967
UPDATE
Final repository & package structure
Refactor steps
I think the best would be to follow the order I suggest. Please let me know if something is missing!
- Copy all contents of SixLabors.Core into the SixLabors.ImageSharp repository (test and product code)
- Create a new, 100% self-contained repository for ImageSharp.Drawing, copy the following content into the repository:
- Current Drawing code:
- Product code
- Test code
- Benchmark code (!!!)
- Test images used by Drawing tests
- Reference images used by Drawing tests
- Product and test code of
SixLabors.Shapes
- Current Drawing code:
- Configure CI for the ImageSharp.Drawing repository
- Update ImageSharp.Web (dogfeeding)
The steps above will also touch #868. If we want to completely resolve that issue, we shall also:
- Clean-up history of binary artifacts in SixLabors.ImageSharp master
- Copy all reference images to SixLabors.ImageSharp
We shall consider introducing GitHub workflows while implementing these steps, but I need more help if we want to do so.
Activity
antonfirsov commentedon Oct 24, 2019
I have two competing proposals for reorganizing our GitHub repositories. (Ignoring ImageSharp.Web, since it will be kept as-is.)
A, 2 Repositories 2 NuGet packages
SixLabors.ImageSharp
New repository SixLabors.ImageSharp.Drawing
B, 2 Repositories 5 NuGet packages
Repo: SixLabors.ImageSharp
Repo: SixLabors.ImageSharp.Drawing
/cc @tocsoft @dlemstra @JimBobSquarePants
JimBobSquarePants commentedon Oct 24, 2019
I vote for A to keep maintenance costs down.
tocsoft commentedon Oct 24, 2019
I feel SixLabors.Fonts should still be kept a separate repo there is definitely scope there to be a much more useful project standalone from the rest of the projects.
However I feel shapes and drawing should be merged into a single project/repo and just label it SixLabors.ImageSharp.Drawing.
We should inline core into imagesharp and just expose them so that drawing and shapes can depend on them directly.
So in conclusion we would end up with 4 packages & repositories.
Each will version independently an inline with changes to dependent projects. i.e. if either fonts of imagesharp is updated we would try to release an updated drawing package using the latest bits.
antonfirsov commentedon Oct 24, 2019
@tocsoft your point is probably valid, since SixLabors.Fonts has more downloads than SixLabors.Shapes.Text or SixLabors.ImageSharp.Drawing. However, a "standalone" font library depending on the whole heavyweight ImageSharp package seems rather unfortunate to me. If we really want to keep it separate, we should probably avoid that.
How about having SixLabors.Core as a separate NuGet package versioned together with ImageSharp? I don't think it's that much extra maintenance. (Having 2 packages in a single repo is infinitely less painful than keeping them entirely separate IMO.)
tocsoft commentedon Oct 24, 2019
Fonts doesn't have any dependencies on ImageSharp... there might possibly be currently a couple of dependencies on some of the point primitives but we can just switch them out to different classes or just Vector2 directly.
tocsoft commentedon Oct 24, 2019
The internals of Fonts definitely have no dependencies on core anyway. Like i said we can just switch the public API to return Vector2.
However I am not opposed to retaining SixLabors.Core as a package inside the ImageSharp repo and versioned inline with the main ImageSharp package if it makes the public apis & interop between fonts and drawing/core cleaner.
antonfirsov commentedon Oct 24, 2019
It also uses
RectangleF, but I can't see anything more. So we can remove the dependency by defining a (not so nice)FontRectanglestruct or similar.Are font internals memory intensive, or not at all? (=> Is custom memory management a possible optimization enabler?)
tocsoft commentedon Oct 24, 2019
Its been a while since I've been diving deeply into fonts internals but I doubt the memory manager would give us much benefit and in fact would probably end up complicating stuff making maintenance more difficult so I think memory management should probably just be left in ImageSharp and left out of core.
antonfirsov commentedon Oct 24, 2019
Okkay, how about this (let's call it option Z):
Benefits:
JimBobSquarePants commentedon Oct 24, 2019
Ok, I've had a dig around in the repositories and that plan makes sense. Fonts is not memory intensive and does not require memory management.
dlemstra commentedon Oct 25, 2019
Do we really need the Core package in option Z? And I do understand that it would make sense to have a separate Font package but I wonder if people would use it without the Drawing library. Having two libraries would simplify things.
JimBobSquarePants commentedon Oct 25, 2019
It's there so Fonts can reuse the primitives.
dlemstra commentedon Oct 25, 2019
Will need to take a good look at the code but a tiny bit of duplication might not be bad here.
tocsoft commentedon Oct 25, 2019
I think I agree with Dirk lets just drop the Core package and duplicate the 1/2 small structs what we need for Fonts it will simplify the dependency tree.
JimBobSquarePants commentedon Oct 25, 2019
Are there any public API crossover points? I’m thinking handling conversion between structs.
tocsoft commentedon Oct 25, 2019
don't think so, not that can't be replaced by switching usages of
PointFtoVector2which already has implicit conversion toPointFantonfirsov commentedon Oct 25, 2019
@dlemstra I'm also very curious about those independent use cases of SixLabors.Fonts. But based on NuGet stats, it's clear that we have such users.
dlemstra commentedon Oct 25, 2019
@antonfirsov When you go to https://www.nuget.org/packages/SixLabors.Fonts/1.0.0-beta0009 there is a
GitHub Usageblock and that shows only 2 repos.antonfirsov commentedon Oct 25, 2019
@dlemstra those are only OSS consumers, and only top repositories are listed.
What I mean is that SixLabors.Fonts has more downloads than SixLabors.Shapes.Text or SixLabors.ImageSharp.Drawing. I see no other explanation for this, than the existence of independent (closed source) consumers.
Fix color and pixel operations. Touches #967 #1002
antonfirsov commentedon Nov 10, 2019
Posted an update in the issue description summarizing the final plans and the steps to achieve them.
Fix color and pixel operations. Touches SixLabors#967 SixLabors#1002
Fix color and pixel operations. Touches #967 #1002