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

Close #334: gdColorMatch(): doubtful algorithm #751

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cmb69
Copy link
Contributor

@cmb69 cmb69 commented Sep 5, 2021

We change the algorithm of gdColorMatch() to calculate the Euclidean
distance of the color component values, what may require to change some
thresholds in existing code to match the new algorithm, as can be seen
by the need to adapt gdimagecolorreplace.c.

We change the algorithm of `gdColorMatch()` to calculate the Euclidean
distance of the color component values, what may require to change some
thresholds in existing code to match the new algorithm, as can be seen
by the need to adapt gdimagecolorreplace.c.
@pierrejoye
Copy link
Contributor

Looks good to me, Thanks @cmb69 !

@pierrejoye pierrejoye added this to the GD 2.4 milestone Sep 7, 2021
@pierrejoye
Copy link
Contributor

I think we need a 2.3.3 release, then most likely a 2.3.4. Once 2.3.3 is out (I will try today or later this week), we can branch GD_2_2 and do only sec fixes there. Master will be 2.4 then and merging this PR as well.

Thoughts?

@cmb69
Copy link
Contributor Author

cmb69 commented Dec 14, 2021

It seems that GD-2.3 has already been branched. Any objections to merge this into master?

Copy link
Member

@vapier vapier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs a rebase

@@ -4,6 +4,7 @@

#include "gd.h"
#include "gd_color.h"
#include <math.h>

/**
* The threshold method works relatively well but it can be improved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update the comment docs to match what the code is doing

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

Successfully merging this pull request may close these issues.

None yet

3 participants