Proposed change to KisPainter, fix for bug 246639

JL VT pentalis at gmail.com
Thu Sep 2 14:31:38 CEST 2010


https://bugs.kde.org/show_bug.cgi?id=246639

This bug had a basic solution which was mostly a workaround I did in
the Smudge Brush and Hatching Brush.
Slangkamp proposed that I generalized the solution. Much discussion
was had and me and DmitryK concluded that the right solution was to
change bitBltFixedSelection in KisPainter. This function is used only
by 3 classes in all of Krita (SmudgeOp, HatchingOp, DuplicateOp), all
of them were coded "believing" that the function fused the mask with
the canvas selection, but such was not the case, only the mask was
used.
This patch changes this behavior and causes
KisPainter::bitBltFixedSelection to first check if there is a
selection in the canvas, if there is none, it works exactly as it used
to (minus a memory leak it had); but if there is one, it fuses the
canvas selection with the mask in the relevant area before blitting,
thus respecting selections.

Were this change accepted, I'd follow it by restoring the Hatching
Brush and Smudge Brush to simply use KisPainter::bitBltFixedSelection
without the bug workaround.

Below is the new KisPainter::bitBltFixedSelection.

PS: excuse the poor excuse of a "patch" to review, I haven't setup my
system for SVN contributions, but I didn't want to waste the chance to
let people review the code while I'm off reinstalling it and then
sleeping.


void KisPainter::bitBltFixedSelection(qint32 dx, qint32 dy, const
KisPaintDeviceSP srcdev, const KisFixedPaintDeviceSP selection, qint32
sx, qint32 sy, qint32 sw, qint32 sh)
{
    if (sw == 0 || sh == 0) return;
    if (srcdev.isNull()) return;
    if (d->device.isNull()) return;

    Q_ASSERT(srcdev->pixelSize() == d->pixelSize);
    Q_ASSERT(selection->colorSpace() ==
KoColorSpaceRegistry::instance()->alpha8());

    QRect srcRect = QRect(sx, sy, sw, sh);
    QRect dstRect = QRect(dx, dy, sw, sh);

    bool selectedness;

    // Check if there is a selection
    if (d->selection.isNull())
        selectedness = false;
    else if (d->selection->isDeselected())
        selectedness = false;
    else
        selectedness = true;

    // In case of COMPOSITE_COPY restricting bitblt to extent can
    // have unexpected behavior since it would reduce the area that
    // is copied.
    if (d->compositeOp->id() != COMPOSITE_COPY)
        srcRect &= srcdev->extent();

    if (srcRect.isEmpty())
        return;

    dx += srcRect.x() - sx;
    dy += srcRect.y() - sy;

    sx = srcRect.x();
    sy = srcRect.y();
    sw = srcRect.width();
    sh = srcRect.height();

    quint8 * srcBytes = new quint8[ sw * sh * srcdev->pixelSize() ];
    srcdev->readBytes(srcBytes, sx, sy, sw, sh);

    quint8 * dstBytes = new quint8[ sw * sh * d->device->pixelSize() ];
    d->device->readBytes(dstBytes, dx, dy, sw, sh);

    quint8 * mergedSelectionBytes = new quint8[ sw * sh *
selection->pixelSize() ];

    if (!selectedness) {
        selection->readBytes(mergedSelectionBytes, 0, 0, sw, sh);
    }
    else {
        if (selection->pixelSize() != d->selection->pixelSize())
            qWarning("Error while performing
KisPainter::bitBltFixedSelection, selection and mask had different
pixel size, no merge performed");
        else {
            // Here selection->pixelSize() is known to be the same as
d->selection->pixelSize(), both should be equal to 1
            quint32 totalPixels = sw * sh * selection->pixelSize();

            quint8 * selectionBytes = new quint8[ totalPixels ];
            d->selection->readBytes(selectionBytes, dx, dy, sw, sh);

            quint8 * maskBytes = new quint8[ totalPixels ];
            selection->readBytes(maskBytes, 0, 0, sw, sh);

            /* Some algebra-fu happens here, basically:
                z = x * y   (conditional  x * y < 1)
                But that's only valid when all numbers are between 0 and 1.
                For values between 0 and 255:
                z/255 = x/255 * y/255
                Conditional:
                x/255 * y/255 < 1     applying *255 on both sides results in
                x * y/255 < 255
                That explains the simplified formula used below.
                255 is replaced by MAX_SELECTED
                z is replaced by mergedSelectionBytes
                and x and y by selectionBytes and maskBytes
            */
            for (quint32 i = 0; i < totalPixels; ++i) {
                if (selectionBytes[i] * maskBytes[i] / MAX_SELECTED <
MAX_SELECTED )
                    mergedSelectionBytes[i] = selectionBytes[i] *
maskBytes[i] / MAX_SELECTED;
                else
                    mergedSelectionBytes[i] = MAX_SELECTED;
            }

            delete [] selectionBytes;
            delete [] maskBytes;
        }
    }

    d->colorSpace->bitBlt(dstBytes,
                          sw * d->device->pixelSize(),
                          srcdev->colorSpace(),
                          srcBytes,
                          sw * srcdev->colorSpace()->pixelSize(),
                          mergedSelectionBytes,
                          sw  * selection->pixelSize(),
                          d->opacity,
                          sh,
                          sw,
                          d->compositeOp,
                          d->channelFlags);

    d->device->writeBytes(dstBytes, dx, dy, sw, sh);

    delete [] srcBytes;
    delete [] dstBytes;
    delete [] mergedSelectionBytes;

    addDirtyRect(QRect(dx, dy, sw, sh));
}


More information about the kimageshop mailing list