I'll take a look at this tomorrow morning =)<br><br><br><div class="gmail_quote">On Thu, Sep 2, 2010 at 3:31 PM, JL VT <span dir="ltr"><></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<a href="https://bugs.kde.org/show_bug.cgi?id=246639" target="_blank">https://bugs.kde.org/show_bug.cgi?id=246639</a><br>
<br>
This bug had a basic solution which was mostly a workaround I did in<br>
the Smudge Brush and Hatching Brush.<br>
Slangkamp proposed that I generalized the solution. Much discussion<br>
was had and me and DmitryK concluded that the right solution was to<br>
change bitBltFixedSelection in KisPainter. This function is used only<br>
by 3 classes in all of Krita (SmudgeOp, HatchingOp, DuplicateOp), all<br>
of them were coded "believing" that the function fused the mask with<br>
the canvas selection, but such was not the case, only the mask was<br>
used.<br>
This patch changes this behavior and causes<br>
KisPainter::bitBltFixedSelection to first check if there is a<br>
selection in the canvas, if there is none, it works exactly as it used<br>
to (minus a memory leak it had); but if there is one, it fuses the<br>
canvas selection with the mask in the relevant area before blitting,<br>
thus respecting selections.<br>
<br>
Were this change accepted, I'd follow it by restoring the Hatching<br>
Brush and Smudge Brush to simply use KisPainter::bitBltFixedSelection<br>
without the bug workaround.<br>
<br>
Below is the new KisPainter::bitBltFixedSelection.<br>
<br>
PS: excuse the poor excuse of a "patch" to review, I haven't setup my<br>
system for SVN contributions, but I didn't want to waste the chance to<br>
let people review the code while I'm off reinstalling it and then<br>
sleeping.<br>
<br>
<br>
void KisPainter::bitBltFixedSelection(qint32 dx, qint32 dy, const<br>
KisPaintDeviceSP srcdev, const KisFixedPaintDeviceSP selection, qint32<br>
sx, qint32 sy, qint32 sw, qint32 sh)<br>
{<br>
if (sw == 0 || sh == 0) return;<br>
if (srcdev.isNull()) return;<br>
if (d->device.isNull()) return;<br>
<br>
Q_ASSERT(srcdev->pixelSize() == d->pixelSize);<br>
Q_ASSERT(selection->colorSpace() ==<br>
KoColorSpaceRegistry::instance()->alpha8());<br>
<br>
QRect srcRect = QRect(sx, sy, sw, sh);<br>
QRect dstRect = QRect(dx, dy, sw, sh);<br>
<br>
bool selectedness;<br>
<br>
// Check if there is a selection<br>
if (d->selection.isNull())<br>
selectedness = false;<br>
else if (d->selection->isDeselected())<br>
selectedness = false;<br>
else<br>
selectedness = true;<br>
<br>
// In case of COMPOSITE_COPY restricting bitblt to extent can<br>
// have unexpected behavior since it would reduce the area that<br>
// is copied.<br>
if (d->compositeOp->id() != COMPOSITE_COPY)<br>
srcRect &= srcdev->extent();<br>
<br>
if (srcRect.isEmpty())<br>
return;<br>
<br>
dx += srcRect.x() - sx;<br>
dy += srcRect.y() - sy;<br>
<br>
sx = srcRect.x();<br>
sy = srcRect.y();<br>
sw = srcRect.width();<br>
sh = srcRect.height();<br>
<br>
quint8 * srcBytes = new quint8[ sw * sh * srcdev->pixelSize() ];<br>
srcdev->readBytes(srcBytes, sx, sy, sw, sh);<br>
<br>
quint8 * dstBytes = new quint8[ sw * sh * d->device->pixelSize() ];<br>
d->device->readBytes(dstBytes, dx, dy, sw, sh);<br>
<br>
quint8 * mergedSelectionBytes = new quint8[ sw * sh *<br>
selection->pixelSize() ];<br>
<br>
if (!selectedness) {<br>
selection->readBytes(mergedSelectionBytes, 0, 0, sw, sh);<br>
}<br>
else {<br>
if (selection->pixelSize() != d->selection->pixelSize())<br>
qWarning("Error while performing<br>
KisPainter::bitBltFixedSelection, selection and mask had different<br>
pixel size, no merge performed");<br>
else {<br>
// Here selection->pixelSize() is known to be the same as<br>
d->selection->pixelSize(), both should be equal to 1<br>
quint32 totalPixels = sw * sh * selection->pixelSize();<br>
<br>
quint8 * selectionBytes = new quint8[ totalPixels ];<br>
d->selection->readBytes(selectionBytes, dx, dy, sw, sh);<br>
<br>
quint8 * maskBytes = new quint8[ totalPixels ];<br>
selection->readBytes(maskBytes, 0, 0, sw, sh);<br>
<br>
/* Some algebra-fu happens here, basically:<br>
z = x * y (conditional x * y < 1)<br>
But that's only valid when all numbers are between 0 and 1.<br>
For values between 0 and 255:<br>
z/255 = x/255 * y/255<br>
Conditional:<br>
x/255 * y/255 < 1 applying *255 on both sides results in<br>
x * y/255 < 255<br>
That explains the simplified formula used below.<br>
255 is replaced by MAX_SELECTED<br>
z is replaced by mergedSelectionBytes<br>
and x and y by selectionBytes and maskBytes<br>
*/<br>
for (quint32 i = 0; i < totalPixels; ++i) {<br>
if (selectionBytes[i] * maskBytes[i] / MAX_SELECTED <<br>
MAX_SELECTED )<br>
mergedSelectionBytes[i] = selectionBytes[i] *<br>
maskBytes[i] / MAX_SELECTED;<br>
else<br>
mergedSelectionBytes[i] = MAX_SELECTED;<br>
}<br>
<br>
delete [] selectionBytes;<br>
delete [] maskBytes;<br>
}<br>
}<br>
<br>
d->colorSpace->bitBlt(dstBytes,<br>
sw * d->device->pixelSize(),<br>
srcdev->colorSpace(),<br>
srcBytes,<br>
sw * srcdev->colorSpace()->pixelSize(),<br>
mergedSelectionBytes,<br>
sw * selection->pixelSize(),<br>
d->opacity,<br>
sh,<br>
sw,<br>
d->compositeOp,<br>
d->channelFlags);<br>
<br>
d->device->writeBytes(dstBytes, dx, dy, sw, sh);<br>
<br>
delete [] srcBytes;<br>
delete [] dstBytes;<br>
delete [] mergedSelectionBytes;<br>
<br>
addDirtyRect(QRect(dx, dy, sw, sh));<br>
}<br>
_______________________________________________<br>
kimageshop mailing list<br>
<a href="mailto:kimageshop@kde.org">kimageshop@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/kimageshop" target="_blank">https://mail.kde.org/mailman/listinfo/kimageshop</a><br>
</blockquote></div><br><br clear="all"><br>-- <br>Dmitry Kazakov<br>