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 implementing custom brush types #20

Closed
mysticfall opened this issue Aug 12, 2019 · 11 comments · Fixed by #149
Closed

Allow implementing custom brush types #20

mysticfall opened this issue Aug 12, 2019 · 11 comments · Fixed by #149

Comments

@mysticfall
Copy link
Contributor

For now, IBrush is supposed to return an instance of BrushApplicator, but both its constructor and indexer are defined with an internal modifier, which makes it practically impossible to create a custom brush implementation.

It can prevent users from adopting ImageSharp in case they need a type of brush that the library doesn't support, as described in SixLabors/ImageSharp#969, for example.

@JimBobSquarePants
Copy link
Member

Hi @mysticfall

We've currently been keeping those APIs internal as we expect some churn as we separate out and optimize the drawing portion of the library, however we would be very happy work with you if you would like to implement a PR against the main library and add the PathGradientBrush described in SixLabors/ImageSharp#969

@antonfirsov
Copy link
Member

antonfirsov commented Aug 12, 2019

Yeah those API-s are very unstable, many changes are expected, we try to avoid breaking changes, so we keep them hidden.

@antonfirsov
Copy link
Member

@mysticfall SixLabors/ImageSharp#967 might be of your interest to see the whole picture.

@mysticfall
Copy link
Contributor Author

@antonfirsov Thanks for the pointer. I'm still undecided what to do with the situation, as serializing textures was initially only a small part of the API which I've been writing.

Not sure if I could justify spending much time in creating such an implementation that can be accepted to be merged (i.e. with studying conventions, writing tests, and etc.) and postponing the main project until it gets released.

I'm not complaining, and I'd love to contribute if I could. It's just that I can only spend only small amount of time for my own project for which texture serialization is but a small part.

I will take a serious look at the code to see if I can make a PR quickly though.

@antonfirsov
Copy link
Member

antonfirsov commented Aug 13, 2019

@mysticfall yeah we are quite strict with both code style and test, but we can support you with those, if you make up your mind to contribute. I'm even happy to push code changes to your PR branch, if it's not too much & I have the time.

Benefit: we will maintain the code afterwards, and keep it in the refactor chain we plan.

@mysticfall
Copy link
Contributor Author

@antonfirsov I opened a pull request for SixLabors/ImageSharp#969, and I think I need some help or an advice as to what to do with test fixtures which belongs to a different repository(submodule).

I'd appreciate if you could help me resolving this problem.

@JimBobSquarePants
Copy link
Member

@mysticfall Will have a good look for you as soon as possible. Thanks so much for your work so far!

@antonfirsov
Copy link
Member

antonfirsov commented Aug 29, 2019

@mysticfall thanks a lot for investing your time! Looks like @JimBobSquarePants has it under control, but ping me if I can help.

@antonfirsov antonfirsov transferred this issue from SixLabors/ImageSharp Jan 17, 2020
@antonfirsov
Copy link
Member

antonfirsov commented Jun 19, 2021

There is not much work left:

We need to make BrushApplicator constructor protected:

internal BrushApplicator(Configuration configuration, GraphicsOptions options, ImageFrame<TPixel> target)

We shall consider changing Apply to consume RowInterval instead of y:

public abstract void Apply(Span<float> scanline, int x, int y);

@antonfirsov antonfirsov added this to the 1.0.0-rc.1 milestone Jun 19, 2021
@JimBobSquarePants
Copy link
Member

Yeah if we can use RowInterval that’d be much preferable

@JimBobSquarePants
Copy link
Member

@antonfirsov Found issues with using RowInterval. Looks like we're stuck working one line at a time.

for (int row = firstRow; row < end; row++)
{
int y = startY + row;
Span<float> span = buffer.GetRowSpan(row).Slice(offsetSpan);
currentApp.Apply(span, startX, y);
}

while (scanner.MoveToNextPixelLine())
{
if (scanlineDirty)
{
scanline.Clear();
}
scanlineDirty = scanner.ScanCurrentPixelLineInto(minX, 0, scanline);
if (scanlineDirty)
{
int y = scanner.PixelLineY;
if (!graphicsOptions.Antialias)
{
bool hasOnes = false;
bool hasZeros = false;
for (int x = 0; x < scanline.Length; x++)
{
if (scanline[x] >= 0.5)
{
scanline[x] = 1;
hasOnes = true;
}
else
{
scanline[x] = 0;
hasZeros = true;
}
}
if (isSolidBrushWithoutBlending && hasOnes != hasZeros)
{
if (hasOnes)
{
source.GetPixelRowSpan(y).Slice(minX, scanlineWidth).Fill(solidBrushColor);
}
continue;
}
}
applicator.Apply(scanline, minX, y);
}
}
}
finally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants