On Monday 13 February 2012 13:25:13 Timothy Pearson wrote:
> >> Care to elaborate? I am willing to listen. Your original message was
> >> disregarded to some extent as you linked to changes that were made on
> >> purpose, and I usually expect people who claim something is wrong to
> >> make
> >> an attempt at stating *why( they think said something is wrong.
> >
> > yes I know they are made on purpose, nevertheless they are wrong. It is
> > quite
> > common that developers not knowing a codebase do things incorrectly. I
> > will
> > now only elaborate on the two commits I outlined. In fact all commits I
> > have
> > seen so far would not pass a review request for KWin and as I mentioned
> > there
> > is at least one commit with the potential to prevent KWin from starting at
> > all.
> >
> > Let's start with 1f40ada: you modify the inline getter for keepAbove. This
> > is
> > not how KWin internally works to have window being as keep above. The
> > proper
> > method to go through is Client::setKeepAbove() which would also tell other
> > interested parties that the window is in fact kept above. This method is
> > quite
> > important to use as it also takes care of putting the window into the
> > correct
> > layer of the stacking order. I think you solved that by hacking the
> > stacking
> > order.
> >
> > The simplest way to achieve what you actually wanted would have been to
> > make
> > your "modal system notification" an override redirect window.
>
> This would have caused the modal system notifications to lose their window
> handles/decorations and drag/resize abilities, unless I have been reading
> the FreeDesktop WM specifications incorrectly. Example:
> http://standards.freedesktop.org/wm-spec/wm-spec-latest.html#id2528706
In the case you want them to have window decorations, yes override redirect
does not work.
Still it's wrong what you did there :-) It's not how KWin works.
>
> > The second commit I pointed out was 9cc1e2c1: I think others already
> > commented
> > in my blog comments why this one is rather bad from a users point of view
> > (introducing new config options without removing the obsoleted ones). But
> > well
> > the main issue from my point of view is that it modified an enum in a
> > public
> > header by not appending to the end, but in the middle. I think you can
> > imagine
> > what happens to 3rd party offerings compiled against the previous version.
>
> We are not too concerned about ABI compatibility here, but since the
> requested change is so trivial I suppose I can push it through.
yes I noticed but that is quite severe as it can result in KWin no longer
starting (not this commit but other ones). So you have to be aware of ABI
compatibility or at least have to take care to not load incompatible
libraries.
>
> Anything else?
Sure, as I mentioned all commits would not pass review, hardly anyone is only
close to correct. You are not a window manager developer - that's nothing bad.
Hardly anyone is actually developing window managers. It took me more than a
year to understand KWin. It is no surprise that you - given the amount of code
you have to handle, are not able to properly develop it. Think about that we
develop the window manager in a peer reviewed style and hardly any commit just
enters the tree. Also we have hundreds of developers testing our code right
from the day it enters master, have thousands of beta testers and much much
more infrastructure to handle the development of critical code pathes.
That's why it would be important to switch to KWin. It would seriously improve
the quality for your users and as it has been stated here more than once,
that's what you care about :-)
Cheers
Martin
Attachments: