Fix incorrect gradient color-stop calculation

[Poyo-SSB]

Gradients with more than two color stops were being incorrectly calculated, leading to bizarre cutoffs.

[CLAassistant]

All committers have signed the CLA.

[JimBobSquarePants]

Hi there, thanks for this!

Could you please upload some demo images to demonstrate before/after?

Cheers!

[Poyo-SSB]

Quick example:

Top old, bottom new.

Gradient in question:

new ColorStop<Rgba32>(0, Rgba32.Black),
new ColorStop<Rgba32>(1 / 4f, Rgba32.Blue),
new ColorStop<Rgba32>(2 / 4f, Rgba32.Red),
new ColorStop<Rgba32>(3 / 4f, Rgba32.White),
new ColorStop<Rgba32>(1, Rgba32.Lime)

The first four quadrants, instead of being sampled as

0%–25%
25%–50%
50%–75%
75%–100%

are sampled as

0%–25%
25%–37.5%
50%–58.333%
75%–81.25%

because the lerp math failed to subtract the local gradient start from the local gradient end.

[antonfirsov]

@jongleur1983 as the original author, do you have some time to check this? 😄

[jongleur1983]

@antonfirsov I hope to find the time tomorrow.

[jongleur1983]

@Poyo-SSB code looks good so far, thanks for spotting that bug.
Can you add the image as a test as well?

See FillLinearGradientBrushTests.cs for other tests on that.
Ideally use an image of height 1px for that, generate that image once (ensure it's correct as it should be) and ask here to get @antonfirsov or someone else to put it into the reference images repository. Add your code as a test that compares those images with each other.

Somehow my setup still fails to compile ImageSharp here, no idea what's wrong, but the code looks correct and the output is definitly far more correct than before.

Thanks

[JimBobSquarePants]

Somehow my setup still fails to compile ImageSharp here, no idea what's wrong

@jongleur1983 You're probably missing the new standards submodule. If you rerun the command from the readme it'll most likely work.

[jongleur1983]

thanks @JimBobSquarePants - I did initialize the submodules, the init/update command didn't give any output anymore, but compilation is fine now.

@Poyo-SSB code looks good, but we'd have to adapt the tests accordingly.
All 4 test cases in FillLinearGradientBrushTests.ArbitraryGradients fail for me, but that's expected as those are in fact test cases for this issue.

So we have to replace the reference images there.
Created a PR for that here SixLabors/Imagesharp.Tests.Images#8 - if that's merged the original master should fail but this PR should be green again.

[antonfirsov]

@jongleur1983 thanks a lot for your help with the tests!

@Poyo-SSB so what still needs to be pushed in order to merge this:

• Updating the submodules to pull down the new reference images from #864: reference output for gradient fix of Poyo-SSB Imagesharp.Tests.Images#8
• A test case base based on your "Quick example". (Just push a test into FillLinearGradientBrushTests based on the existing ones, I'll to the rest with the reference images.)

PS:
I keep thinking whether applying a constant level of strictness regarding test coverage is a good strategy. Maybe we can relax requirements for less critical processors to lower the barrier for new contributors.
However, probably it's much better if we provide help adding the test cases. What do you guys think?

[JimBobSquarePants]

Plan B. I like how strict our reference tests are.

The only problem is the centralized repo. If I update a reference image after fixing a bug and my PR is blocked then that can potentially break any other builds.

[antonfirsov]

@JimBobSquarePants cool! Looks like this is something we need to deal with! Opened #868.

[codecov]

Codecov Report

Merging #864 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #864      +/-   ##
==========================================
+ Coverage   88.97%   88.97%   +<.01%     
==========================================
  Files        1017     1017              
  Lines       44436    44438       +2     
  Branches     3227     3228       +1     
==========================================
+ Hits        39538    39540       +2     
  Misses       4176     4176              
  Partials      722      722

Impacted Files	Coverage Δ
...rp.Drawing/Processing/GradientBrushBase{TPixel}.cs	95.34% <100%> (ø)	⬆️
src/ImageSharp/Image.FromBytes.cs	76.92% <0%> (ø)	⬆️
src/ImageSharp/MetaData/ImageProperty.cs	83.33% <0%> (ø)	⬆️
...harp/MetaData/Profiles/ICC/Various/IccProfileId.cs	66.66% <0%> (ø)	⬆️
src/ImageSharp/Memory/RowInterval.cs	100% <0%> (ø)	⬆️
src/ImageSharp/ColorSpaces/Rgb.cs	100% <0%> (ø)	⬆️
...Data/Profiles/ICC/Various/IccColorantTableEntry.cs	58.33% <0%> (ø)	⬆️
src/ImageSharp/Primitives/ValueSize.cs	78.94% <0%> (ø)	⬆️
src/ImageSharp/ColorSpaces/CieLch.cs	85.71% <0%> (ø)	⬆️
src/ImageSharp/ColorSpaces/Illuminants.cs	100% <0%> (ø)	⬆️
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a86cddb...6aad76f. Read the comment docs.

[codecov]

Codecov Report

Merging #864 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #864      +/-   ##
==========================================
+ Coverage   88.97%   88.98%   +<.01%     
==========================================
  Files        1017     1017              
  Lines       44436    44466      +30     
  Branches     3227     3228       +1     
==========================================
+ Hits        39538    39568      +30     
  Misses       4176     4176              
  Partials      722      722

Impacted Files	Coverage Δ
...harp.Tests/Drawing/FillLinearGradientBrushTests.cs	99.56% <100%> (+0.06%)	⬆️
...rp.Drawing/Processing/GradientBrushBase{TPixel}.cs	95.34% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a86cddb...10f5ca2. Read the comment docs.

[JimBobSquarePants]

I'll have a look at this later on to see why builds are failing and fix accordingly.