[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    Re: Notes on Krita
From:       Patrick Julien <freak () codepimps ! org>
Date:       2003-10-04 21:51:23
[Download RAW message or body]

On October 4, 2003 04:44 pm, Boudewijn Rempt wrote:
> While trying to figure out how to make my test tool put squares of the
> select colour down on the canvas, I made some notes on what how I think
> krita sticks together. I'm still not sure what makes it tick, though, and
> there's a couple of head-scratchers near the end.
>
> Anyway, to preserve this bit of work (might save the next man or woman some
> time) for posterity, I post it here:
>
> A Perspective on Krita's Design
>
>   Krita's codebase has been through as many changes as the app's name
>   itself. It started with KImageShop (hence the 'kis' prefix one finds
>   everwhere, and which would have been unnessary had Krita used
>   namespaces, one fancies), then became Krayon to end up as Krita. The
>   stated design goal was to create a application geared towards the
>   more artistic user; the kind of user who settles down with his
>   tablet to create a spot of Art. It now is partway to becoming a kimplet.

Yes, krayon was considerably simpler, however, due to various issues, I could 
see more artistic users having problem with the application the way it was.  
My rational was that if I'm doing this, might has well go all out.

>
> Design Patterns
>
> 	These are what I recognize as Design patterns in Krita. This is
> by-and-large Patrick Julien's work:
>
>             * Change brushes handling to a generic Mediator pattern
>               (kis_resource_mediator). Resources are brush shapes,
>               patterns (and colours, too?)

There is still a lot of work to do here, again wanted to use flyweight here.

>
>             * Change colourspace handling to a Strategy pattern
>               (strategy/kis_strategy_color)

This is what was discussed in previous emails

>
>             * Change moving chunks of painting to a Strategy pattern
>               (strategy/kis_strategy_move)
>
>             * kis_factory points towards a Factory pattern, but I
>               don't know what it's for yet. (Creates the pluginserver
>               and resourceserver)


Yes, this is from Krayon, loads brushes, etc.  I added code to read in Gimp 
formated brushes, etc.

>
>             * Tools are created by a Factory pattern (but it isn't
>               dynamic yet): kis_tool_factory.

True, I eventually wanted tools to be kparts, this way we can remove inactive 
tools/effects from the running process (keeping only the menu entries).  
However, I still wasn't at a point where I was concentrating on tools yet.

>
>             * There's the start of a Memento pattern for tools, but it
>               doesn't seem to be used (kis_tool_memento)

This was mainly an experiment.  Due to the multiple documents, views and 
images nature of krita.  Add to that koffice supports multiple frames for the 
same application, I wanted to make the tools flyweights, when the user 
changes view, etc, I would reset the tools to the state before the user left.  
I'm not sure that was a good idea since that may cause the GUI in another 
frame to react to the new state.

>
>             * The Builder pattern is used to do conversion to and/or
>               from ImageMagick. I guess this is used for importing
>               images in other file-formats than .krita.

This I wanted to take out completely and put in a filter.  However, this would 
require a bit more work, hence why it wasn't done, this code is used from 
import/export image option. 

Also, Koffice filters aren't the most friendly to an application like krita.  
For very large images, it would be better if we could ask the filter only to 
load the parts that are actually viewable by the user.

Finally, if somebody ever gets around to putting swap file support in Krita, 
unmodified tiles don't need to be written back to the swap file, they can 
simply be throw out and reread from the original.

>
>             * Flyweight pattern used for background tiles.
>               kis_background.
>
>             * Nameserver. Perhaps not a pattern -- at least not one
>               with an obvious GOF name. Appears to be computing a
>               number for new layers and new documents.
>
>             * kis_render. Interface implemented by kis_paint_device
>               and kis_image.
>
>             * Rename kisTool to kisToolInterface -- but the file is
>               still called kis_tool.

Yes, this is my style, I always leave out the "interface" part out of the 
filename.  This is consistent with what we will find in the rest of Krita.

>
>             * Addition of kis_types. Defines shared pointer pointers
>               to core krita objects: image, paintdevice, channel,
>               mask, layer etc.

Here, we have a small problem.  I actually was convinced that boost should be 
used here.   You see KSharedPtr isn't a complete C++ pointer.   You can see 
my bug report in kdebugs for more details.   

In any case,  one does not introduce a new dependency (boost) in koffice 
without, at least, getting silence on koffice-devel :)

I believe that just boost::shared_ptr and boost::scoped_ptr could really help 
the quality of this application.

>
>             * Tile architecture. There's Mediator and a Singleton
>               pattern here at least. This stuff seems to work, doesn't
>               need hacking at the moment.

Nope, this works.

>
>             * Visitor pattern used to flatten an image or merge
>               layers. Merging exposes a funny bug where the tiles
>               are re-arranged in an interesting mosaic pattern.
>
>
> User interface
>
>   Krita uses a fairly ugly side-panel to collect what are palettes
>   in other applications. I would prefer dockable, attachable
>   palettes myself. Doesn't KDE have a lib for that already?

Maybe, I'm not sure what you're talking about here, but I'm sure it's there.

>
>   ui/labels
>
>         These classes are used in the statusbar.
>
>   ui
>
>         The dialogs appear to be created by hand, not with Qt
>         Designer. Besides, many are inoperational.
>
>   other
>
>         The canvas, rules and sidebar panels are defined here, too,
>         nicely separated.
>
> Tools
>
>   Working: select_rectangular, test, zoom, colorpicker, paste, move
>
>   Not working: airbrush, brush, colorchanger (no idea what it should
>   do), ellipse, eraser, fill, line, pen, polygon, polyline,
>   rectangle, select_contiguous, select_elliptical, select_freehand,
>   select_polygonal, stamp
>
>   Missing tools: convolve, smear, smudge, whirl, charcoal, chalk,
>   oils, clone, select_by_colour
>
> Plugins
>
>   The single plugin example has been present in krita since about day
>   one, as far as I can see. It doesn't show anything about what one
>   can do with plugins.
>
>   To judge from the code in kis_plugin_server, it never got beyond
>   an idea. (kdDebug() << "hallo\n"...)
>
> ImageMagick
>
>   There still appear to be some dependencies upon ImageMagick (builder
>   subdir). What are these for, and should they stay? Is it for
>   importing/exporting foreign file formats?

Yep, see above, wanted a filter.

>
> Undo/Redo
>
>   Undo and Redo make use of the standard KDE mechanism, which I don't
>   really understand either.

KDE undo/redo is pretty much basic support, i.e. a command interface and a 
command list.  In koffice, you also get some standard actions if I remember 
too.

See the command design pattern.

>
>
> Obsolete files
>
>   The following files appear to be unused:
>
>     core/helper/kis_timer

This is handy, it's used to debug stuff.

>     core/helper/kis_scopedlock

yep, replace by QMutexLocker, was flirting with the use of worker threads at 
one point.

>     core/kis_krayon (was predecessor of kis_resource)
>     core/kis_mask
>     core/kis_util
>     ui/kis_krayon_widget
>
> Random head-scratchings
>
>
>        - Why the QUANTUM redefinition?

This is the number of bits per color channel, if you want a 32 bit color 
krita, set this to 8.  For 64 bit color, set this to 16, etc.

>
>        - is_mask unimplemented? What was it meant to do, or is it an
>        interface?
>
>        - nameserver presents unique numbers for new layers?

Just a simple generator, yes.

>
>        - what does 'upscale' do?

Very important, in 8 bit/channel color mode, upscale doesn't do anything.  
However, downscale and upscale in > 8 bit, this is needed to covert the color 
for the actual display rendering, i.e. X, etc, only do 8 bit color.  Using 
this everywhere you work on color will keep krita color channel bit depth 
independent.

>
>        - Is KisVector only used in obsolete tools?

	Don't know, what is KisVector?  Wasn't this an actual "vector", i.e. not an 
array.

>
>        - I take it that the two tests that are present in krita/test are
>        obsolete?

yes

>
>        - what with the dummmmmy.cc?

This has to do with the koffice build system, or at least dummy.cc does.  Ask 
one of the autoconf/automake guys on kde-devel.  I, on the other hand, have 
gone to great lengths to make sure I don't know anything about autoconf/
automake.

>
>        - which bits of the krita/ui files are still relevant?

All of them need reworking really.

>
>        - kis_selection.h needs to be included to compile code that
>          uses kis_paint_device, and I wonder why.
>
>        - what is paint-offset?

In the canvas, the actual origin of the image on the widget.

A recommendation now, if you will, you should try to keep the code in the same 
style.  I don't advocate that my style is superior to any other, however, 
there is value in uniformity.  When I took over Krayon, the code was almost 
unreadable, there was has many stiles than lines of code.


_______________________________________________
kimageshop mailing list
kimageshop@mail.kde.org
http://mail.kde.org/mailman/listinfo/kimageshop
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic