KFadeWidgetEffect bugfix

[Matthias Kretz]

On Thursday 21 February 2008, Rafael Fernández López wrote:
> Hi,
>
> > that's the setGeometry call getting "wrong" numbers
>
> I don't think so. Is about making it a child of the passed widget parent.

I see that you were trying to do that. But it's wrong to do it as I already 
explained.

> While it is not being layouted (that will happen later),

It is not layouted. It's not inserted in any layout and calls setGeometry to 
place and size itself correctly. Or are you talking about the other widget 
now?

> the widget lives 
> at the top left bottom.

top left you mean? That's because the setGeometry gets wrong numbers, i.e. 
QPoint(0, 0). It gets the wrong number because the widget it is animating is 
not layouted/visible.

> When the widget is layouted, it is removed from 
> there and goes on the position it should. On the meanwhile you see a
> flickering that is what I'm talking about. When opening systemsettings and
> entering on some category, you will see all KTitleWidget of each KCM on the
> top left for a second, then after that, they dissappear, and are moved to
> their correct positions (because their layout code was run).

They are not moved. They disappear because the animation duration is over.

> > > As the widget developer user, I'd like to know who the parent is, maybe
> > >
> > > MyWidget(QWidget *parent)
> > >         : QWidget(parent->parent());
> > >
> > > I really think is better to do it as the patch does:
> > >
> > > MyWidget(QWidget *parent)
> > >         : QWidget(parent)
> >
> > I don't understand what you're trying to say. Did you ever read the
> > apidox? The problem is that if you make KFadeWidgetEffect a child of the
> > widget you're about to take a snapshot of, then the child will be asked
> > to paint itself, too. Which is not what is wanted here. I.e. the fade
> > widget may not be a child of the widget that is to be animated.
> >
> > See attached patch for what I think you wanted to fix.
>
> Always that your patch or mine had:

Sorry?

> -    : QWidget(destWidget && destWidget->parentWidget() ?
> destWidget->parentWidget() : destWidget)
> -    , d_ptr(new KFadeWidgetEffectPrivate(destWidget))
> +    : QWidget(destWidget ? destWidget->parentWidget() : 0),
> +    d_ptr(new KFadeWidgetEffectPrivate(destWidget))
>
> The flickering is removed. About the other stuff, I don't understand why we
> can't do the deleteLater() on the constructor. Anyway, if that's for safety
> because of code doing widget->start() when widget had been deleted with no
> notice, I'm ok. About your latest patch, well, yes... is the same. For
> being honest I didn't try the latest patch I sent because kdelibs weren't
> linking correctly for a while for me.
>
> So, that means, I'm ok with your patch if the hide() is needed. Actually is
> the only difference that yours and last mine had.

I don't know if the hide() is necessary, but I like to have it explicit. Also 
makes it easier to understand the code, I think.

The important difference is that KFadeWidgetEffect gets disabled when the 
widget it is animating is not visible. The two checks for !destWidget 
and !destWidget->parentWidget() are just for non-debug builds of code that 
was never run against a debug-build kdelibs (they wouldn't have hit the 
Q_ASSERT).

-- 
________________________________________________________
Matthias Kretz (Germany)                            <><
http://Vir.homelinux.org/
MatthiasKretz at gmx.net, kretz at kde.org,
Matthias.Kretz at urz.uni-heidelberg.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080221/d95e0b84/attachment.sig>