A first part of the layers/masks patch

Dmitry Kazakov dimula73 at gmail.com
Fri Sep 25 21:25:45 CEST 2009


What is done in the patch.


1) Now all the layers apply their own masks in a uniform way.

How it was before:
Every type of layer (paint, adjustment,etc) had it's own
updateProjection() function. And EVERY layer implemented it's own
cycle of applying masks. No need to say that they all differed. And
most of them was broken, because they didn't take into account
need/change rects of filters inside masks (at best).

How it is now:
updateProjection() and ALL the stuff about masks AND projections is
moved to KisLayer. Specialized layer can operate with only a few
functions those are not connected with a projection much. Every child
can overload these functions:

original() - returns a paint device where the layer stores it's
image. NOTE: This is not the same with projection() as the latter one
includes original() with all the masks applied.

repaintOriginal() - called by updateProjection when original should be
repainted.

OPTIONAL:
A layer CAN (not should) overload:

needProjection() - if a layer needs an always-in-memory projection
(e.g. KisPaintLayer needs one for indirect painting)

copyOriginalToProjection() - used in couple with the previous one. A
layer can enhance original during painting on a projection
(e.g. indirect painting, see KisPaintDevice's method)


This all means that a KisLayer's child shouldn't do any work to create
a projection. It's created by KisLayer in a deferred way (when masks
are present)

Masks application strategy changed too (and quite much).
Now changeRect()/needRect() functions are part of KisNode. And before
application of a mask-stack we first get to know what need rect we
want to copy from original() device.

Rect calculating algorithm is quite overwhelming. It's implemented in
two functions: masksChangeRect and masksNeedRect.
First we get a change rect of a DESTINATION PROJECTION with a first
function in a bottom-up(!) way. Then we calculate needRects for all
the masks in a top-down(!) way with a second function, and by the end
of this process we get a rect that we should copy from original()
device to a temporary device for applying filters.

BUGFIX:
This part of a patch fixes a bug with a blur-filter-mask. In current
version  you simply you get a slack if you paint on a layer with
blur-mask present (i'm not sure you can check it up now, because
blur-filter has an internal bug - it writes the result to the source
instead of destination).


2) The second task for this patch was to unify adjustment and
generator layers. The point is, they both use selections, that means
they both should work in the same way.

How it was before:
KisAdjustmentLayer and KisGeneratorLayer had code duplications for
selections stuff.

How it is now:
All the selection-related stuff is moved to a separate class -
KisSelectionBasedLayer. Both problematic classes are derived from this
base class. Speaking truly, KisAdjustmentLayer class has very few code
inside now.

BUGFIX:
Selections on selection based layers was not working at all
before. Now the the code applies selections (if you manage to
create one ;) (see * for more)) to the layers well.


3) The third task was to clean masks classes. We had much code
duplication there too. Most duplications and misuses was connected to
selections (again).

How it was before:
KisFilterMaks, KisGeneratorMask, KisTransparencyMask - they all had
their own implementation

How it is now:
All the selection-related stuff is moved to KisMask class. Here is an
extract form a commit message:

    Refactoring for masks

    Now masks utilize selections in a uniform way. KisMask is the only
    place that knows anything about mask's selection[0]. It's descendant's
do
    very simple task, they just decorate the rect with the desired
    effect[1]. The task made by KisTransparencyMask fell back to even more
    trivial routine - it just needs to copy source image to destination,
    without any modifications[2].

    Another significant change accumulated by this commit is a small
    cleanup of KisMaskManager. There are still many FIXME's for it. The
    most annoying is code duplication [3] of KActions for masks.

    [0] - KisMask::apply()
    [1] - virtual KisMask::decorateRect()
    [2] - KisTransparencyMask::decorateRect()
    [3] - KisMaskManager's actions for mask functions are duplicated in
    KisLayerBox.

As you can see, this commit accumulates some cleanups for
KisMaskManager too. Most of them remove code duplications and uniform
the code. More than that [and this is the only place where i worry
about the patch] this fix adds a new method(!) to KoDocument. It's
called undoLastCommand() - that is an equivalent to a user-undo
command. It is added to be able to cancel creation of an filter mask
in case a user presses 'Cancel' button in a creation dialog. Please
review this change!

BUGFIXES:
- cancelling creation of the filter mask now do not break undo stack
- selections (see *) on filter masks work well now


4) Cleaning selections infrastructure, made me create a benchmark for
different selection application algorithms. This benchmark is placed
in ./image/tests/kis_filter_selections_benchmark.cpp. I wrote about it
already here.


*) Still present bugs:

a) [the worst one] Painting on any alpha mask do not work because of
alpha() colorspace issue i was described before in this maillist. You
can paint if your mask is transparent, but the brush isn't. This is
not an issue of an algorithm. This is an issue of the alpha()
colorspace and the fact that we use brush's alpha channel for painting
alpha image-channel (tha latter assumption is completely wrong(!)).

Solution: Brush's dab should use grayscale colorspace during painting
on alpha-mask/selection. KisSelection should use grayscale, or a
special alpha_special() colorspace to work with grayscale channel of a
dab.

b) Vector selection tools DO NOT work on masks (And i'm not sure they
work on adjustment layers). The reason of the first fact (i think) is
that KisSelectionToolHelper class uses KisLayerSP type for internal
storage of a paint device. It should be changed to KisNodeSP to fix
that (of course alongside some other changes in logics of this class)

a),b) => c) There is no(!) way to paint on alpha-channels/masks
currently. You simply CAN'T change the mask you created.

The only way to create a mask now is the following:
    i) make a vector selection first
    ii) create a  mask - it'll derive this selection for it's own

[some minor bugs not dependent on the patch]
d) Blur filter works wrong. It writes filtered image to the source
device instead of the destination one

e) Drag&Drop is broken in KisLayerBox. Just try to change an order of
masks with a mouse - you can easily lift it up above the root
layer. Of course,  your next action will cause krita to crash.

f) Curves filter DO NOT(!) work. I guess, there is some bug in
KoColorTransformation as every curve works as a brightness/contrast
curve instead of a channel curve. Just try it out!


[some design bugs]
g) KisNodeManager::allowAsChild uses some silly string comparison
algorithm. Why?! Every node has a specially designed method for this!

h) KActions for mask-manipulation functions are duplicated in
KisMaskManager and in KisLayerBox. They must be moved to
KisMaskManager.

PS:
btw, a,b,c,d,e,f are good, well-fed release blockers.


-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20090925/af0a5def/attachment-0001.htm 


More information about the kimageshop mailing list