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

List:       kwin
Subject:    Re: [PATCH] non-linear timelines for effects
From:       Sebastian Kuegler <sebas () kde ! org>
Date:       2008-03-22 17:59:36
Message-ID: 200803221859.36557.sebas () kde ! org
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Friday 21 March 2008 19:00:26 Lubos Lunak wrote:
> On Friday 21 of March 2008, Sebastian Kuegler wrote:
> > On Thursday 20 March 2008 13:47:07 Louai Al-Khanji wrote:
> > > On Thu, Mar 20, 2008 at 11:32 AM, Sebastian Kuegler <sebas@kde.org> 
wrote:
> > > >  150 sec feels kind of choppy (at least on my hardware, you only get
> > > > to see two, maybe three frames).
> > >
> > > I think that makes more of a case that performance needs to be
> > > improved somehow. I see the same thing on one of my machines with an
> > > nvidia chip, but my notebook with an intel chip that _should_ be a lot
> > > slower runs all effects very smoothly. On the nvidia machine it is
> > > actually faster to disable direct rendering for kwin.
>
>  It's not faster, it just feels smoother. And current SVN should be smooth
> on nvidia even normally, as __GL_YIELD=NOTHING is again turned on by
> default.

Cool. Social life prevented me from testing on my NVidia based machine. The 
main issue I had in the past is not smoothness of the animations (those are 
fine), but some kind of lag in switching windows (not sure how I can describe 
it better...) It's a dualhead / Twinview setup with a 7600GS, also need to 
investigate if that makes the difference. ATi x1300 with fglrx feels a bit 
better in that respect, even if the card itself is quite a bit slower.

> > > If you get only 2 or 3 frames in 125 ms that means you are running
> > > this (rather simple) effect at around 13-20 fps - and that is not
> > > enough to present the user with a smooth experience no matter what.
> >
> > I'm using an ATI x1300 on this machine, effects run relatively smooth
> > (around 30fps at normal usage). I've done a bit of testing, with
> > fullscreen
>
>  You mean when idle? That's pretty low, it's supposed to run at the monitor
> refresh rate by default.

Nah, idle it's around 45-ish frames. "Normal usage means minimising a window, 
switching desktop but not clicking like crazy just to get performance down as 
low as possible. (Doing that, I can get it down to a choppy 15-20 FPS.)

> > > I disagree :) IMHO animations should just be visual hints or cues as
> > > to what is happening on the screen. 300ms is a really long time -
> > > instead of focusing on the windows and their content you start to
> > > focus on the animations, taking attention _away_ from the task at
> > > hand. Or at least that's what happens to myself. :)
> > >
> > > Maybe it's a question for the usability people.
>
>  I agree here. But I think a global option affecting speed of all
> animations wouldn't hurt.

Problem is that the time an animation should take partly depends on the 
animation. I've partly tried to catch that (or at least encourage users to 
use sensible values) by setting a default in TimeLine. We surely don't want 
hard-coded animations that take 5 seconds just because the developers thinks 
they look cool.

> > Latest version attached.
>
>  Let's see. I think the patch has problems with not using the class as a
> value. I don't mean just this
>
> > @@ -28,8 +28,13 @@ KWIN_EFFECT( minimizeanimation,
> > MinimizeAnimationEffect )
> > MinimizeAnimationEffect::MinimizeAnimationEffect()
> >      {
> >      mActiveAnimations = 0;
> > +    mTimeLine = new TimeLine();
> >      }
> >
> > +MinimizeAnimationEffect::~MinimizeAnimationEffect()
> > +{
> > +    delete mTimeLine;
> > +}
>
>  where simply using it directly as a member without pointers could save all
> this, including the chance somebody forgets the delete, or the fact the
> class is missing Q_DISABLE_COPY, but my problem with the patch is that
> looks like a proof of concept with the class just somehow added in. It's ok
> as a proof of concept, but I think the class should simply replace all
> those 'double progress' entirely.

Yes, that's the goal.

>  Why would you want to have two variables that keep the progress? 

Thanks for the review. I'll address those issues with the next version of the 
patch.

>  Also, if 
> I'm reading the patch correctly, having one timeline and several progress
> variables in the Minimize effect already shows how this can be a bug - they
> all share the one timeline, so if you e.g. minimize a window and restore
> another at the same time, the curve shape setting in windowMinimized() and
> windowUnminimized() will conflict and the drawing will be wrong.

Yes, correct assumption. We need one TimeLine per windows. I'll fix that. 
Might take a couple of days, however, as I've some other coding stuff to do, 
and KWin is mighty distracting :-)
-- 
sebas

 http://www.kde.org | http://vizZzion.org |  GPG Key ID: 9119 0EF9 

["signature.asc" (application/pgp-signature)]

_______________________________________________
Kwin mailing list
Kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin


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