<div class="gmail_quote">On Fri, Sep 25, 2009 at 9:25 PM, Dmitry Kazakov <span dir="ltr"><<a href="mailto:dimula73@gmail.com">dimula73@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
What is done in the patch.<br><br><br>1) Now all the layers apply their own masks in a uniform way.<br><br>How it was before:<br>Every type of layer (paint, adjustment,etc) had it's own<br>updateProjection() function. And EVERY layer implemented it's own<br>
cycle of applying masks. No need to say that they all differed. And<br>most of them was broken, because they didn't take into account<br>need/change rects of filters inside masks (at best).<br><br>How it is now:<br>updateProjection() and ALL the stuff about masks AND projections is<br>
moved to KisLayer. Specialized layer can operate with only a few<br>functions those are not connected with a projection much. Every child<br>can overload these functions:<br><br>original() - returns a paint device where the layer stores it's<br>
image. NOTE: This is not the same with projection() as the latter one<br>includes original() with all the masks applied.<br><br>repaintOriginal() - called by updateProjection when original should be<br>repainted.<br><br>
OPTIONAL:<br>
A layer CAN (not should) overload:<br><br>needProjection() - if a layer needs an always-in-memory projection<br>(e.g. KisPaintLayer needs one for indirect painting)<br><br>copyOriginalToProjection() - used in couple with the previous one. A<br>
layer can enhance original during painting on a projection<br>(e.g. indirect painting, see KisPaintDevice's method)<br><br><br>This all means that a KisLayer's child shouldn't do any work to create<br>a projection. It's created by KisLayer in a deferred way (when masks<br>
are present)<br><br>Masks application strategy changed too (and quite much).<br>Now changeRect()/needRect() functions are part of KisNode. And before<br>application of a mask-stack we first get to know what need rect we<br>
want to copy from original() device.<br><br>Rect calculating algorithm is quite overwhelming. It's implemented in<br>two functions: masksChangeRect and masksNeedRect.<br>First we get a change rect of a DESTINATION PROJECTION with a first<br>
function in a bottom-up(!) way. Then we calculate needRects for all<br>the masks in a top-down(!) way with a second function, and by the end<br>of this process we get a rect that we should copy from original()<br>device to a temporary device for applying filters.<br>
<br>BUGFIX:<br>This part of a patch fixes a bug with a blur-filter-mask. In current<br>version you simply you get a slack if you paint on a layer with<br>blur-mask present (i'm not sure you can check it up now, because<br>
blur-filter has an internal bug - it writes the result to the source<br>instead of destination).<br><br><br>2) The second task for this patch was to unify adjustment and<br>generator layers. The point is, they both use selections, that means<br>
they both should work in the same way.<br><br>How it was before:<br>KisAdjustmentLayer and KisGeneratorLayer had code duplications for<br>selections stuff.<br><br>How it is now:<br>All the selection-related stuff is moved to a separate class -<br>
KisSelectionBasedLayer. Both problematic classes are derived from this<br>base class. Speaking truly, KisAdjustmentLayer class has very few code<br>inside now.<br><br>BUGFIX:<br>Selections on selection based layers was not working at all<br>
before. Now the the code applies selections (if you manage to<br>create one ;) (see * for more)) to the layers well.<br><br><br>3) The third task was to clean masks classes. We had much code<br>duplication there too. Most duplications and misuses was connected to<br>
selections (again).<br><br>How it was before:<br>KisFilterMaks, KisGeneratorMask, KisTransparencyMask - they all had<br>their own implementation<br><br>How it is now:<br>All the selection-related stuff is moved to KisMask class. Here is an<br>
extract form a commit message:<br><br> Refactoring for masks<br><br> Now masks utilize selections in a uniform way. KisMask is the only<br> place that knows anything about mask's selection[0]. It's descendant's do<br>
very simple task, they just decorate the rect with the desired<br> effect[1]. The task made by KisTransparencyMask fell back to even more<br> trivial routine - it just needs to copy source image to destination,<br>
without any modifications[2].<br><br> Another significant change accumulated by this commit is a small<br> cleanup of KisMaskManager. There are still many FIXME's for it. The<br> most annoying is code duplication [3] of KActions for masks.<br>
<br> [0] - KisMask::apply()<br> [1] - virtual KisMask::decorateRect()<br> [2] - KisTransparencyMask::decorateRect()<br> [3] - KisMaskManager's actions for mask functions are duplicated in<br> KisLayerBox.<br>
<br>As you can see, this commit accumulates some cleanups for<br>KisMaskManager too. Most of them remove code duplications and uniform<br>the code. More than that [and this is the only place where i worry<br>about the patch] this fix adds a new method(!) to KoDocument. It's<br>
called undoLastCommand() - that is an equivalent to a user-undo<br>command. It is added to be able to cancel creation of an filter mask<br>in case a user presses 'Cancel' button in a creation dialog. Please<br>review this change!<br>
<br>BUGFIXES:<br>- cancelling creation of the filter mask now do not break undo stack<br>- selections (see *) on filter masks work well now<br><br><br>4) Cleaning selections infrastructure, made me create a benchmark for<br>
different selection application algorithms. This benchmark is placed<br>in ./image/tests/kis_filter_selections_benchmark.cpp. I wrote about it<br>already here.<br><br><br>*) Still present bugs:<br><br>a) [the worst one] Painting on any alpha mask do not work because of<br>
alpha() colorspace issue i was described before in this maillist. You<br>can paint if your mask is transparent, but the brush isn't. This is<br>not an issue of an algorithm. This is an issue of the alpha()<br>colorspace and the fact that we use brush's alpha channel for painting<br>
alpha image-channel (tha latter assumption is completely wrong(!)).<br><br>Solution: Brush's dab should use grayscale colorspace during painting<br>on alpha-mask/selection. KisSelection should use grayscale, or a<br>
special alpha_special() colorspace to work with grayscale channel of a<br>
dab.<br><br>b) Vector selection tools DO NOT work on masks (And i'm not sure they<br>work on adjustment layers). The reason of the first fact (i think) is<br>that KisSelectionToolHelper class uses KisLayerSP type for internal<br>
storage of a paint device. It should be changed to KisNodeSP to fix<br>that (of course alongside some other changes in logics of this class)<br><br>a),b) => c) There is no(!) way to paint on alpha-channels/masks<br>currently. You simply CAN'T change the mask you created.<br>
<br>The only way to create a mask now is the following:<br> i) make a vector selection first<br> ii) create a mask - it'll derive this selection for it's own<br><br>[some minor bugs not dependent on the patch]<br>
d) Blur filter works wrong. It writes filtered image to the source<br>device instead of the destination one<br><br>e) Drag&Drop is broken in KisLayerBox. Just try to change an order of<br>masks with a mouse - you can easily lift it up above the root<br>
layer. Of course, your next action will cause krita to crash.<br><br>f) Curves filter DO NOT(!) work. I guess, there is some bug in<br>KoColorTransformation as every curve works as a brightness/contrast<br>curve instead of a channel curve. Just try it out!<br>
<br><br>[some design bugs]<br>g) KisNodeManager::allowAsChild uses some silly string comparison<br>algorithm. Why?! Every node has a specially designed method for this!<br><br>h) KActions for mask-manipulation functions are duplicated in<br>
KisMaskManager and in KisLayerBox. They must be moved to<br>KisMaskManager.<br><br>PS:<br>btw, a,b,c,d,e,f are good, well-fed release blockers.<br></blockquote><div><br>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.<br>
<br>Also painting on mask causes some artifacts:<br><a href="http://img23.imageshack.us/img23/3918/kritafiltermask.png">http://img23.imageshack.us/img23/3918/kritafiltermask.png</a> <br></div></div><br>