Proposed change to KisPainter, fix for bug 246639

[Dmitry Kazakov]

I'll take a look at this tomorrow morning =)


On Thu, Sep 2, 2010 at 3:31 PM, JL VT <> wrote:

> 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));
> }
> _______________________________________________
> kimageshop mailing list
> kimageshop at kde.org
> https://mail.kde.org/mailman/listinfo/kimageshop
>



-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20100903/9b92df19/attachment.htm