Regression using last version(s) of ImageMagick

[Steve Allewell]

On 13/10/2018 18:32, Sean Enck wrote:
> On 2018-10-13, Steve Allewell wrote:
>> On 12/10/2018 20:42, Sean Enck wrote:
>>> On 2018-10-08, Sean Enck wrote:
>>>> It appears importing an image has regressed (no longer works) against 
>>>> kxstitch 2.1.1 when using the latest (well archlinux latest) versions of 
>>>> image/libmagick. The current (not working version) in arch:
>>>>> imagemagick-7.0.8.12-1-x86_64.pkg.tar.xz
>>>>> libmagick-7.0.8.12-1-x86_64.pkg.tar.xz
>>>>
>>>> I downgraded my versions to verify and import worked (no crash):
>>>>> imagemagick-7.0.7.13-1-x86_64.pkg.tar.xz
>>>>> libmagick-7.0.7.13-1-x86_64.pkg.tar.xz
>>>>
>>>> The steps are:
>>>> 1. open kxstitch
>>>> 2. go to import image
>>>> 3. select an image (any image, I haven't seen it work)
>>>> 4. doesn't matter if you do anything before hitting "OK" when the image 
>>>> would go to the normal grid
>>>> 5. full crash, can provide a trace if there is anything special in kxstitch 
>>>> or any specific tracing method is preferred.
>>>>
>>>> Thanks for any help/insight,
>>>> --Sean
>>>
>>> I was able to do some digging and found the issue started around ImageMagick 
>>> 7.0.7.24 (works) -> 7.0.7.25 (broken). I traced it to the FlossScheme.cpp 
>>> color matching where nullptr was being returned and I patched this in 
>>> Arch using the referenced patch below.
>>>
>>> https://aur.archlinux.org/cgit/aur.git/plain/0001-Change-nearest-color-lookup-logic-for-finding-floss-.patch?h=kxstitch
>>>
>> Hi Sean
>>
>> Apologies for not getting back to you earlier.  I noticed you sent a
>> message to the list on the 8th, but it doesn't look like I got a copy of it.
>>
>> I have version 7.0.8 of ImageMagick too and confirm the bug.
>>
>> I had a look at the patch and it does the job, although it does not
>> require float for near and closest; abs and QColor red, green, blue
>> functions all return int.
>>
>> I did a little rewrite which works for me, please check it for yourself:
>> -------------------------------------------------------------------------------
>> Floss *FlossScheme::find(const QColor &color) const
>> {
>>     QListIterator<Floss *> flossIterator(m_flosses);
>>
>>     Floss *matched = nullptr;
>>     int closest = 100;
>>
>>     while (flossIterator.hasNext()) {
>>         Floss *floss = flossIterator.next();
>>         QColor c = floss->color();
>>
>> 	// the color mapping may not be perfect so search for a near match.
>>         int distance = abs(color.red()-c.red()) +
>> abs(color.green()-c.green()) + abs(color.blue()-c.blue());
>>
>>         if (distance < closest) {
>>             matched = floss;
>>             closest = distance;
>>         }
>>     }
>>
>>     return matched;
>> }
>> -----------------------------------------------------------------------------
>>
>> Do you want to commit the change?
>>
>> I will have a look to see what changed in ImageMagic between those two
>> versions and see if there are any improvements to make.
>>
>> Regards
>>
>> Steve
>>
> 
> Thanks for another set of eyes on it!
> 
> I initially emailed (without troubleshooting more than what I put) 
> because I didn't have the time until (days) later to look into the code.
> I'm not (generally) a C++ guy so I need a little extra time to try
> things (before realizing I could just use the call you had as a the
> criteria, I tried another which did require a float and didn't go back
> once I realized what you already had was an int approximation... I just
> needed to track it). I started trying to see what changed in ImageMagick
> but I have no idea what kxstitch uses from it so it was more of "where, 
> in the code does this seem to be happening?"
> 
> I applied your update locally and it appears to work just the same as my
> initial patch (aka bug still gone).
> 
> Happy to commit the (joint) change, I wasn't sure what the process 
> is/was? I initially used format-patch to try and bundle it fully as a
> consumable idea/be able to patch the arch packaging.
> 
> --Sean
> 
Sorry, I thought I had seen your name against a commit before, but after checking I realised I was mistaken.  I know we have had a
number of emails in the past.

I will do the commit and credit you for the fix if that's ok.

I had a look at the ImageMagick code changes for 7.0.7-25 and there wasn't anything that stood out.  I will have a look over the
kxstitch code and how it uses ImageMagick to see if I have made an error in that.

Thanks for all the efforts.

Steve