[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    Re: [calligra/krita_selections_no_clipboard_kazakov] krita/image:
From:       Boudewijn Rempt <boud () valdyas ! org>
Date:       2011-05-14 21:34:24
Message-ID: 201105141734.24837.boud () valdyas ! org
[Download RAW message or body]

On Saturday 14 May 2011 May, Dmitry Kazakov wrote:
> Git commit 162695af2661476956c2d8666e15d5e008976f0e by Dmitry Kazakov.
> Committed on 14/05/2011 at 17:55.
> Pushed by dkazakov into branch 'krita_selections_no_clipboard_kazakov'.
> 
> Added ability to crop masks
> 
> There are two hacks right now:
> 1) KisMask does not have an image() pointer. I'm not sure it should
> have, so currently the image pointer is requested through the parent.

No, I don't think so. And please take care: there is no reason on earth why masks or \
nodes should be associated with an image. So if you attempt to retrieve the image, \
always check for 0.


> +    KisImageWSP getImage(KisNode *node) {

It's a long time since we prefixed our getters with "get". This should be called \
"image()". And since it is called only once, it shouldn't be a function, but done \
inline. There is no advantage in clarity by putting this bit of code in its own \
function.

Also -- where is the crop visitor created? 

It's in KisImage and in KisToolCrop. That means that instead of trying to get the \
image from the mask through the layer, you should pass the image as a parameter in \
the constructor of KisCropVisitor and use that. 

So, this getImage method must be removed and replaced by KisImage as a parameter to \
the constructor of KisCropVisitor.


> +        // temporary hack, our masks do not have a link to image.
> +
> +        while (node) {
> +            KisLayer *layer = dynamic_cast<KisLayer*>(node);
> +            if(layer) {
> +                return layer->image();
> +            }
> +            node = node->parent().data();
> +        }
> +        return 0;
> +    }
> 
> -    bool cropPaintDeviceLayer(KisLayer * layer) {
> -        KisUndoAdapter *undoAdapter = layer->image()->undoAdapter();
> -
> -        KisSelectedTransaction transaction(i18n("Crop"), layer);
> -        layer->paintDevice()->crop(m_rect);
> +    /**
> +     * Crops the specified layer and adds the undo information
> +     * to the undo adapter of the layer's image.
> +     */
> +    bool cropPaintDeviceNode(KisNode *node) {
> +        /**
> +         * TODO: implement actual robust cropping of the selections,
> +         * including the cropping of vector (!) selection.
> +         */
> +        KisImageWSP image = getImage(node);
> +        KisUndoAdapter *undoAdapter = image->undoAdapter();
> +
> +        QRect nodeExtent = node->extent();
> +        KisTransaction transaction(i18n("Crop"), node->paintDevice());
> +        node->paintDevice()->crop(m_rect);
> transaction.commit(undoAdapter);
> 
> if (m_movelayers) {
> -            QPoint oldPos(layer->x(), layer->y());
> -            QPoint newPos(layer->x() - m_rect.x(), layer->y() - m_rect.y());
> -            QUndoCommand *cmd = new KisNodeMoveCommand(layer, oldPos, newPos, \
> layer->image()); +            QPoint oldPos(node->x(), node->y());
> +            QPoint newPos(node->x() - m_rect.x(), node->y() - m_rect.y());
> +            QUndoCommand *cmd = new KisNodeMoveCommand(node, oldPos, newPos, \
> image); undoAdapter->addCommand(cmd);
> }
> +
> +        node->setDirty(nodeExtent);
> return true;
> }
> 
> diff --git a/krita/image/tests/kis_crop_visitor_test.cpp \
> b/krita/image/tests/kis_crop_visitor_test.cpp index 0ed890a..b806701 100644
> --- a/krita/image/tests/kis_crop_visitor_test.cpp
> +++ b/krita/image/tests/kis_crop_visitor_test.cpp
> @@ -24,6 +24,7 @@
> #include "kis_image.h"
> #include "kis_paint_device.h"
> #include "kis_fill_painter.h"
> +#include "kis_transparency_mask.h"
> 
> #include "testutil.h"
> 
> @@ -63,6 +64,41 @@ void KisCropVisitorTest::testUndo()
> delete undoAdapterDummy;
> }
> 
> +void KisCropVisitorTest::testCropTransparencyMask()
> +{
> +    const KoColorSpace * cs = KoColorSpaceRegistry::instance()->rgb8();
> +
> +    TestUtil::KisUndoAdapterDummy* undoAdapterDummy = new \
> TestUtil::KisUndoAdapterDummy(); +    KisImageSP image = new \
> KisImage(undoAdapterDummy, 300, 300, cs, "test", false); +    KisPaintLayerSP layer \
> = new KisPaintLayer(image, "testlayer", OPACITY_OPAQUE_U8); +    KisPaintDeviceSP \
> dev = layer->paintDevice(); +
> +    QRect fillRect(0,0,300,300);
> +    dev->fill(fillRect, KoColor(Qt::white, cs));
> +
> +    KisTransparencyMaskSP mask = new KisTransparencyMask();
> +
> +    image->addNode(layer, image->rootLayer());
> +    image->addNode(mask, layer);
> +
> +    QRect selectionRect(50,50,100,100);
> +
> +    mask->setSelection(new KisSelection());
> +    KisPixelSelectionSP pixelSelection = \
> mask->selection()->getOrCreatePixelSelection(); +    \
> pixelSelection->select(selectionRect, MAX_SELECTED); +
> +    QCOMPARE(pixelSelection->selectedExactRect(), selectionRect);
> +
> +    QRect cropRect(75,75,50,50);
> +
> +    KisCropVisitor visitor(cropRect, false);
> +
> +    mask->accept(visitor);
> +
> +    QCOMPARE(pixelSelection->selectedExactRect(), cropRect);
> +    delete undoAdapterDummy;
> +}
> +
> 
> QTEST_KDEMAIN(KisCropVisitorTest, GUI)
> #include "kis_crop_visitor_test.moc"
> diff --git a/krita/image/tests/kis_crop_visitor_test.h \
> b/krita/image/tests/kis_crop_visitor_test.h index 454a085..65bca36 100644
> --- a/krita/image/tests/kis_crop_visitor_test.h
> +++ b/krita/image/tests/kis_crop_visitor_test.h
> @@ -28,6 +28,7 @@ private slots:
> 
> void testCreation();
> void testUndo();
> +    void testCropTransparencyMask();
> };
> 
> #endif
> 
> 


-- 
Boudewijn Rempt | http://www.valdyas.org, http://www.krita.org
_______________________________________________
kimageshop mailing list
kimageshop@kde.org
https://mail.kde.org/mailman/listinfo/kimageshop


[prev in list] [next in list] [prev in thread] [next in thread]