Skip to content

Add QuadDistortion to ProjectiveTransformBuilder#2748

Merged
JimBobSquarePants merged 10 commits into
SixLabors:mainfrom
Socolin:quad-distortion
Oct 21, 2024
Merged

Add QuadDistortion to ProjectiveTransformBuilder#2748
JimBobSquarePants merged 10 commits into
SixLabors:mainfrom
Socolin:quad-distortion

Conversation

@Socolin

@Socolin Socolin commented Jun 8, 2024

Copy link
Copy Markdown
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Add 2 new methods: PrependQuadDistortion and AppendQuadDistortion to ProjectiveTransformBuilder.

Those method allow to apply a distortion on an image by specifying the requested coordinate for each corner.

Example:

using (var img1 = Image.Load("cat-original.png"))
{
	var topLeft = new Point(25, 25);
	var topRight = new Point(200, 75);
	var bottomRight = new Point(225, 175);
	var bottomLeft = new Point(38, 225);

	img1.Mutate(c => c.Transform(new ProjectiveTransformBuilder()
		.AppendQuadDistort(topLeft, topRight, bottomRight, bottomLeft)));
	img1.Save("output-small2.png");
}

Source
image
Result
image

@CLAassistant

CLAassistant commented Jun 8, 2024

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@JimBobSquarePants

Copy link
Copy Markdown
Member

Thanks @Socolin for this, it is very interesting! I'll have to put my math's hat on to look at this.

I'm curious. Is this potentially an alternative (better) approach to ProjectiveTransformBuilder.PrependTaper?

Comment thread src/ImageSharp/Common/Helpers/GaussianEliminationSolver.cs Outdated
Comment thread src/ImageSharp/Common/Helpers/GaussianEliminationSolver.cs Outdated
Comment thread src/ImageSharp/Common/Helpers/QuadDistortionHelper.cs Outdated
@Socolin

Socolin commented Jul 10, 2024

Copy link
Copy Markdown
Contributor Author

FYI: The performance test I did

TestPerformanceGaussianElimination.zip

  • Matrix is using MathNet.Numerics Nuget
  • MatrixCustom is the implementation included in this PR
  • MatrixCustomFloat is the same implementation with float instead of a generic
  • Matrix8x8 is using TNumber[,] instead of TNumber[][] with static size of 8x8

BenchmarkDotNet v0.13.12, Ubuntu 22.04.4 LTS (Jammy Jellyfish)
AMD Ryzen 9 7950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 8.0.302
  [Host]     : .NET 8.0.6 (8.0.624.26715), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  DefaultJob : .NET 8.0.6 (8.0.624.26715), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI


| Method            | Mean     | Error   | StdDev  |
|------------------ |---------:|--------:|--------:|
| Matrix            | 526.6 ns | 1.02 ns | 0.96 ns |
| MatrixCustom      | 237.0 ns | 3.31 ns | 3.09 ns |
| MatrixCustomFloat | 234.1 ns | 4.64 ns | 4.34 ns |
| Matrix8x8         | 293.5 ns | 0.32 ns | 0.28 ns |

@JimBobSquarePants

Copy link
Copy Markdown
Member

@Socolin I'll see if I can find some time over the next few days to pull down your fork and help reimplement it into the latest codebase. I'm keen to add this functionality.

@Socolin

Socolin commented Jul 31, 2024

Copy link
Copy Markdown
Contributor Author

@Socolin I'll see if I can find some time over the next few days to pull down your fork and help reimplement it into the latest codebase. I'm keen to add this functionality.

I finally have some time again.
I'll push changes related to the today. If it's easier for you to take over this PR, feel free to do it :)

@Socolin

Socolin commented Jul 31, 2024

Copy link
Copy Markdown
Contributor Author

I rebased the branch and pushed the changes

@Socolin

Socolin commented Jul 31, 2024

Copy link
Copy Markdown
Contributor Author

I just saw I'm getting an error because ProjectiveTransformBuilder Append take 2 arguments now, should this be computed twice ? Since this operation is taking some processing time, I'm not sure about what to do here.

@JimBobSquarePants

Copy link
Copy Markdown
Member

I just saw I'm getting an error because ProjectiveTransformBuilder Append take 2 arguments now, should this be computed twice ? Since this operation is taking some processing time, I'm not sure about what to do here.

The operation takes nanoseconds so I'm not concerned by that.

@JimBobSquarePants

Copy link
Copy Markdown
Member

Hi @Socolin would it be possible for you to give me full write access to your fork so I can complete this PR for you?

There's a known issue with Git LFS and GitHub which means I cannot update the reference images without full write access.

Thanks!

@Socolin

Socolin commented Oct 16, 2024

Copy link
Copy Markdown
Contributor Author

I did grant you access to the repo, can you confirm it's all right ?

@JimBobSquarePants

Copy link
Copy Markdown
Member

I did grant you access to the repo, can you confirm it's all right ?

Perfect. Thank you!

@JimBobSquarePants

Copy link
Copy Markdown
Member

I think this is good to go now. Thanks @Socolin for submitting the solution for this. It's a fantastic addition to the library!

This was referenced May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants