Bug 249288 - KWin::EffectWindow class documentation needs improvement
Summary: KWin::EffectWindow class documentation needs improvement
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: compositing (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: VLO wishlist
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-28 01:57 UTC by Richard
Modified: 2012-03-11 18:24 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard 2010-08-28 01:57:27 UTC
Version:           unspecified (using KDE 4.4.5) 
OS:                Linux

Some of the member function names are ambiguous and very few of the member functions have any information on what they do. Working with the Qt documentation is a joy in comparison to working with the KDE documentation, which requires that I do trial and error to try to figure out what means what when all I want to do is make a small change to some of KDE's code that depends on it.

Reproducible: Sometimes

Steps to Reproduce:
1. With no knowledge of the KDE codebase, attempt to make a minor change to make an effect behave a little differently.
2. Observe that you have no clue what the member functions available to you do and that the documentation does not help you understand what they do.



The main issue I encountered is that the name of KWin::EffectWindow::geometry() is ambiguous in that it could mean either the current window geometry as the effect is in progress or the window geometry before the effect started and the documentation makes no attempt to distinguish between which of the two that this function means. This is in no way limited to KWin::EffectWindow::geometry(). The following suffer from the same exact issue:

KWin::EffectWindow::height()
KWin::EffectWindow::pos()
KWin::EffectWindow::rect()
KWin::EffectWindow::size()
KWin::EffectWindow::width()
KWin::EffectWindow::x()
KWin::EffectWindow::y()

Something even more concerning is that one of the few functions with documentation, KWin::EffectWindow::hasOwnShape(), is intended for internal use only, but it is a public member function(). It should be at least be protected if not private.

There are many more issues, but rather than enumerate all of them, I would like to give an example of a class that I consider to be better documented, which is the Qt QRect class:

http://doc.trolltech.com/4.6/qrect.html

It has a nice description. It even has an example of its use. It also has clear descriptions of what each of the member functions do. It is the complete opposite of KWin::EffectWindow's documentation:

http://api.kde.org/4.5-api/kdebase-workspace-apidocs/kwin/lib/html/classKWin_1_1EffectWindow.html

The documentation for the class is machine generated from comments in the code, so I am filing this as an issue with the code rather than the KDE website.
Comment 1 Thomas Lübking 2010-08-28 06:52:04 UTC
a) all your examples are one :-P

b) as you might have figured, effects are "stateless" - so there's no access to the manipulated window geometry since it's only known by the effecthandler (the engine, which might or might not support several transitions, ie. try to name the geometry() during the magic lamp effect =) - therefore the name is maybe unclear w/o understanding of the effect engine class (which you need to have to implement an effect) but it's not ambiguous

c) no idea why hasOwnShape() is marked "for internal use" (martin?, lucas?) but that sounds pretty much wrong to me (and privating the function is please rather no option)

d) in general and aside this, please notice that the effect lib is NOT part of kdelibs and esp. NOT stable

e) HOWTO - never read it, though ;-P
http://blog.martin-graesslin.com/blog/2009/07/how-to-write-a-kwin-effect/
(is it in KDE's howtos as well?)
Comment 2 Martin Flöser 2010-08-28 07:30:20 UTC
to c) No idea, but yes sounds wrong.

to e) http://techbase.kde.org/User:Mgraesslin/Effects but I have to move it from my private space

In general more and better documentation would be nice, but up to now the effects library has almoast exclusively been used by KWin developers. And EffectWindow is more or less just a copy of an internal class so it's to me understandable that it is not documented properly.
Comment 3 Martin Flöser 2012-03-11 18:24:28 UTC
The API documentation has been improved lately and will see another update very soon (one branch needs merging). If there is anything specific unclear please ask on the mailinglist, irc channel or in the forum. If we know exactly what needs improving, it's quite simple to add documentation :-)