Yet another bug. This time filters vs selections

[Dmitry Kazakov]

Hi all!

It seems to me that filters do not pay any attention to the
*selection*parameter passed to them. They simply ignore it (at least i
can say so for
colors filters). And i don't know how to fix this well.

Let's take a look into KisColorTransformationFilter:

KisHLineConstIteratorPixel srcIt =
src->createHLineConstIterator(srcTopLeft.x(), srcTopLeft.y(), size.width());
    KisHLineIteratorPixel dstIt = dst->createHLineIterator(dstTopLeft.x(),
dstTopLeft.y(), size.width());

    for (int row = 0; row < size.height(); ++row) {
        while (! srcIt.isDone()) {
            int srcItConseq = srcIt.nConseqHPixels();
            int dstItConseq = dstIt.nConseqHPixels();
            int conseqPixels = qMin(srcItConseq, dstItConseq);

            int pixels = 0;

            if (hasSelection) {
                // Get largest horizontal row of selected pixels

                while (srcIt.isSelected() && pixels < conseqPixels) {
                    ++pixels;
                }
                inverter->transform(srcIt.oldRawData(), dstIt.rawData(),
pixels);

                // We apparently found a non-selected pixels, or the row
                // was done; get the stretch of non-selected pixels
                while (!srcIt.isSelected() && pixels < conseqPixels) {
                    ++ pixels;
                }
            } else {
                pixels = conseqPixels;
                inverter->transform(srcIt.oldRawData(), dstIt.rawData(),
pixels);
            }

            // Update progress
            srcIt += pixels;
            dstIt += pixels;
        }
        srcIt.nextRow();
        dstIt.nextRow();
    }


First of all, this function doesn't initialize srcIt with *selection*. But
this is not a problem - the first line should be changed to:

KisHLineConstIteratorPixel srcIt =
src->createHLineConstIterator(srcTopLeft.x(), srcTopLeft.y(), size.width(),
selection);

That is not a problem. The problem is:

* The filter can't update semi-selected pixel (0<selectedness<255). The
destination pixel will be either fully substituted with a new one
(selectedness>=1), or left untouched (selectedness==0).

The right way - a filter should blend colors if selectedness is more than 0
and less than 255

I see two solutions for this problem:

1) To force filters to use some complex algorithm that applies selectedness
(which is a usual alpha mask (but inverted)). Something like:

inverter->transform(srcIt.oldRawData(), dstIt.rawData(), pixels);
colorSpace->applyInverseAlphaU8Mask(dstIt.rawData(), selectionIt.rawData(),
pixels);

Pros:
+ may (i'm not sure) be a bit faster than the second solution
Cons:
- all the filters should be modified (in case they are broken, of course)
- the architecture as a whole is quite complex (too many roles for the
filter class; more parameters are given to the filter in comparison with the
second solution)
- the work of filter-developers is more difficult. More than that, it's
really unneeded work for them and the fact they should work with selections
discloses our internals of selections framework, that breaks encapsulation.


2) Remove all the selection stuff from filters. Just modify caller's code:
filter a layer onto a temporary paint device and bitBlt it to destination
device then (with selection turned on in KisPainter).

Pros:
+ the system is clear KisFilter - works with filtering only, KisPainter -
works with selections
+ it's much easier to write new filters, you shouldn't bother with
selections
+ we do not disclose our internals to filter developers
+ this solution fits very well into Adj. layers stuff (as it's already have
additional device - projection)
Cons:
- can be a bit slower because of cold cache (two cycles instead of one)
- can be MUCH slower with selections with holes inside
- fits not so well into Filter masks stuff as it needs additional paint
device


What can you say about this?
For me, i like the second case for simplicity of the design and
maintainability, but i'm afraid of some perfomance problems.

How should i solve this problem? Any ideas?


PS:

Btw, where are filters called form in Krita? I know only three places:
- adj. layers
- filter masks
- filter menu (actually, uses masks)

PPS:
Btw, this bug seems to be present in Blur filter too.

-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20090908/91d7e584/attachment.htm