Hm, that shouldn't matter -- the threshold filter is a filter where the neighbouring pixels don't matter. Disabling threading only is needed for those filters that need access to the whole image. There must be something else going on, probably some overflow.

I can reproduce the issue, though.

The logical candidate is the intensity8 function, though that doesn't explain why sometimes a block remains colored.

This is interesting: the RGB u8 colorspace does:

quint8 RgbU8ColorSpace::intensity8(const quint8 *src) const
{

const KoBgrU8Traits::Pixel *p = reinterpret_cast<const KoBgrU8Traits::Pixel *>(src);
return (quint8)(p->red * 0.30 + p->green * 0.59 + p->blue * 0.11);

}

While the base class converts to QColor, uses the same formula _but_ adds 0.5

virtual quint8 intensity8(const quint8 * src) const {
    QColor c;
    const_cast<KoColorSpaceAbstract<_CSTrait> *>(this)->toQColor(src, &c);
    return static_cast<quint8>((c.red() * 0.30 + c.green() * 0.59 + c.blue() * 0.11) + 0.5);
}

Not sure about the addition of 0.5, but your mention of intensity8 made me try something else. I wonder if this is the issue?
(I'm compiling with "CMAKE_BUILD_TYPE=Debug" to prevent unused variables from being optimized out.)

If the call to intensity8 is removed, the colored blocks will go away.
For example, the following gives a fine result, although obviously not a threshold:

if ((rand() % 255) > threshold) {

However, if I do this instead, the blocks are present:

quint8 intensity = device->colorSpace()->intensity8(it.oldRawData());
if ((rand() % 255) > threshold) {

I followed the calls that I expected to be involved with the base class version of intensity8 (toQColor > normalisedChannelsValue > nativeArray, although I may have looked in the wrong files), but nothing immediately stuck out to me as something that would violate const.

I actually found the real culprit: the base intensity8 function was not threadsafe! I fixed that in 746c6b1558e2f02fa2a9eba27abe437330f4d16c -- and many, many thanks for bringing the problem to our attention. This function has been fundamentally unsafe since 2006!

I'm glad to hear that a problem was fixed! Unfortunately, I pulled the changes and I'm still able to reproduce both the rectangles and the incorrect single pixels. Did your commits fix it for you?

As a note, just after my previous post, I tested memcmp in an assert to compare oldRawData before and after intensity8. There was no change.

Yes, with that commit I cannot reproduce anymore, before, it was really easy.

Ah, I see. In toQColor, you added the lock inside the "else" clause of an if statement. If I set a breakpoint, I end up inside the "profile == 0" part.
I can no longer reproduce the issue after moving the lock to the first line of the function.

Ah, I still cannot reproduce a problem, but it might be safer to move the mutexlocker up, so I'll do that.

http://commits.kde.org/krita/f5bb5fd6ac7bf5705189e142dae10c4a2a6c50de

I actually was wrong, I hadn't seen that the d->qcolordata was also used in the other part of the if statement.