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

Painting on masks is a bit weird as now the meaning of brush and eraser is
reversed. So before the patch I could paint transparency with the brush, now
I have to use the eraser.

Also painting on mask causes some artifacts:
http://img23.imageshack.us/img23/3918/kritafiltermask.png
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20090929/6469ac3f/attachment.html