Weird wrong colors in gradients

[jongleur1983]

Prerequisites

• I have written a descriptive issue title
• I have verified that I am running the latest version of ImageSharp
• I have verified if the problem exist in both DEBUG and RELEASE mode
• I have searched open and closed issues to ensure it has not already been reported

Description

This bug has been reported by @Wibble199 in Gitter on Oct, 29th 2019.
The following unit test (adapted from https://pastebin.com/RJQWm0za ) generates gradients with wrong colored pixels. Those pixels are at random(?) positions, not stable across re-running the same code.

[Theory]
[WithBlankImages(200, 200, PixelTypes.Rgb24)]
public void NewIssueTest<TPixel>(
    TestImageProvider<TPixel> provider)
    where TPixel : struct, IPixel<TPixel>
{
    provider.VerifyOperation(
        img =>
        {
            var brush = new PathGradientBrush(
                new[]
                {
                    new PointF(0, 0),
                    new PointF(200, 0),
                    new PointF(200, 200),
                    new PointF(0, 200),
                    new PointF(0, 0)
                },
                new[] { Color.Red, Color.Yellow, Color.Green, Color.DarkCyan, Color.Red });
            img.Mutate(m => m.Fill(brush));
        });
}

Current sample output (two different test runs):

or

The expected result is a smooth gradient between the four colors on the corners instead.

Steps to Reproduce

see code sample above.

While investigating the cause for easier debugging I reduced the maxDegreeOfParallelism to 1 and the issue is gone.
Thus it looks like something going on when applying the brush in parallel.

System Configuration

• ImageSharp version: current master branch
• Other ImageSharp packages and versions: -
• Environment (Operating system, version and so on): Windows 10 Version 1903, Build 18362.418
• .NET Framework version: 4.7.2
• Additional information:

[jongleur1983]

added unit test to reproduce the issue at https://github.com/jongleur1983/ImageSharp/tree/jongleur1983/1044-wrongPixelsInPathGradients :
see commit: jongleur1983@0f91e70

[jongleur1983]

investigations:

• errors don't occur when disabling parallelization
• when adding a debug output line in the this[x,y] accessor of the PathGradientBrushApplicator reduces wrong pixels dramatically, but doesn't solve all. I guess this is due to accidently limiting parallelization due to the System.Console access.
• Even though in my test with a debug output line per pixel access there's much less errors, in my first test run there was at least one, but no pixel has been accessed twice.

[JimBobSquarePants]

I can't spot anything in our code that specifically that would cause thread safety issues, however, IEnumerable<T> is not thread safe. I would assume read operations would be ok but without digging I cannot be sure. My advice would be to remove any Linq usage from the code (It must perform horribly anyway) and use an array instead of a list for PathGradientBrushApplicator<TPixel>.edges.

[jongleur1983]

I'll try, thanks.

[jongleur1983]

Looks like there are some more relatively easy performance and memory improvements possible while removing Linq.
Working on it...

[jongleur1983]

@JimBobSquarePants removing LINQ and using arrays/for-loops doesn't solve the issue itself, yet.
Should be faster though, so I commited the changes to the branch nevertheless.

[JimBobSquarePants]

Very odd. I couldn’t see any other potential areas where multi threading would be an issue.

Thanks for investigating this btw!

[jongleur1983]

You're welcome @JimBobSquarePants - as long as it could have been my "fault", at least I wanted to make sure it's not ;)
Now it's a challenge to solve it, so let's see.

[JimBobSquarePants]

Ignore the above commit message, it doesn't relate to this issue.