A first part of the layers/masks patch

[Sven Langkamp]

On Fri, Sep 25, 2009 at 9:25 PM, Dmitry Kazakov <dimula73 at gmail.com> wrote:

> 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)
>

Yes, all selection tools will not work on the mask but on the selection. So
even if you have selected the mask all changes with selection tools will go
to the global selection.


> 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.
>

We might want to kill the actions in the mask menu. See the other thread.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20090926/fb6ecdbb/attachment.html