oxygen style and kwin client, svn external
"Aaron J. Seigo" <[email protected]>
| Newsgroups | gmane.comp.kde.devel.kwin,gmane.comp.kde.artists |
|---|---|
| Message-ID | <[email protected]> |
hi everyone ... a blocker for the git migration is the usage of svn externals. they are a broken concept that we need to get away from. there is one in kdebase between runtime/kstyles/oxygen/lib and workspace/kwin/clients/oxygen/lib. in looking at it today, i noticed something odd / interesting in the code itself: OxygenAnimation. this can be completely replaced with QPropertyAnimation (see attached patch that does this in the kwin client code). with Hugo's blessing i'd like to do the same for the oxygen style as well. there also appears to be something of a misunderstanding is the usage of QPointer in the code. data() is never checked (so it's not actually protecting the code against anything!) and QPointer itself results in a global hash being populated and checked *whenever a QObject is deleted*. this is made even worse since the hash is global to the app it requires aquiring an app-global lock every time it is accessed. oy vey! please use QWeakPointer instead of QPointer, which avoids this problem. and in the case of code like this where the value of data() is never checked, don't bother using a shared pointer at all, it just gives the code a false sense of security at the expense of some overhead. anyways, if Hugo will so bless the removal of OxygenAnimation, i'll fix this in the oxy style as well and then oxygenanimation.h can be removed from the oxygen "lib". next is to get rid of the svn external altogether, which contains the Helper and TileSet classes. both have non-trivial amounts of code in them and it is indeed useful to share them. there are two options i can see here: * create a shared lib in runtime that both the kwin client and qstyle can link against. pros: minimal code dupe; cons: ABI would probably need to committed to so that running random combinations of kdebase-runtime and kdebase- workspace would work together, means a bit more time spent in runtime linking (though probably not significant) * create a static lib in runtime that both kwin client and qstyle can link against. pros: the actual source code is shared, no runtime linkage overhead. cons: the code is duplicated in both plugins. * keep a copy of the files (4 of them; 2 headers, 2 imlementations) in both runtime and workspace and hand-synchronize them when needed. pros: it's the "least works right now" solution, cons: it's the most "require vigilance in the future" solution. TileSet doesn't seem to have changed very rapidly, but Helper has gone through it's fits and starts the static lib is, imho, the "least worst" solution here. since we won't have to do a "check for the static lib" Find* cmake module (it will be part of kdebase runtime!), it should be relatively straight forward. for 4.4, i think we really need to straighten out the Oxygen::Animation issue ASAP. the QPointer usage is really not great, and as that code just duplicates Qt's own classes now (though i did notice it has a very nice Kinetic- compatible API, huzzah for that! :) it should be surgically removed. for the svn extern issue, we could wait for 4.5 to open up in trunk, and that's probably sensible to avoid that kind of change when not absolutely required at this stage of development. thoughts? -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks _______________________________________________ kwin mailing list [email protected] https://mail.kde.org/mailman/listinfo/kwin
qpropertyanimation_in_oxygen_kwin_client.diff
(text/x-patch, 20 KB)
Index: oxygenclientgroupitemdata.cpp
===================================================================
--- oxygenclientgroupitemdata.cpp (revision 1067853)
+++ oxygenclientgroupitemdata.cpp (working copy)
@@ -39,7 +39,7 @@
QObject( parent ),
QList<ClientGroupItemData>(),
client_( *parent ),
- animation_( new Animation( 150, this ) ),
+ animation_( new QPropertyAnimation( this ) ),
animationType_( AnimationNone ),
progress_(0),
draggedItem_( NoItem ),
@@ -47,14 +47,16 @@
{
// setup animation
- animation().data()->setStartValue( 0 );
- animation().data()->setEndValue( 1.0 );
- animation().data()->setTargetObject( this );
- animation().data()->setPropertyName( "progress" );
+ animation_->setDuration( 150 );
+ animation_->setStartValue( 0 );
+ animation_->setEndValue( 1.0 );
+ animation_->setTargetObject( this );
+ animation_->setPropertyName( "progress" );
// setup connections
- connect( animation().data(), SIGNAL( valueChanged( const QVariant& ) ), SLOT( updateBoundingRects( void ) ) );
- connect( animation().data(), SIGNAL( finished( void ) ), SLOT( updateBoundingRects( void ) ) );
+ connect( animation_, SIGNAL( valueChanged( const QVariant& ) ), SLOT( updateBoundingRects( void ) ) );
+ connect( animation_, SIGNAL( finished( void ) ), SLOT( updateBoundingRects( void ) ) );
+ connect( animation_, SIGNAL( finished( void ) ), this, SIGNAL( animationFinished( void ) ) );
}
@@ -84,7 +86,7 @@
if( type == AnimationNone )
{
- if( isAnimationRunning() ) animation().data()->stop();
+ if( isAnimationRunning() ) animation_->stop();
targetItem_ = NoItem;
draggedItem_ = NoItem;
targetRect_ = QRect();
@@ -103,7 +105,7 @@
} else if( (type&AnimationMove) && targetItem_ == target ) return;
// check animation state
- if( isAnimationRunning() ) animation().data()->stop();
+ if( isAnimationRunning() ) animation_->stop();
targetItem_ = target;
targetRect_ = QRect();
@@ -157,7 +159,7 @@
targetRect_.setWidth( width );
}
- if( animate ) animation().data()->start();
+ if( animate ) animation_->start();
else {
for( int index = 0; index < count(); index++ )
@@ -173,7 +175,7 @@
} else if( type & AnimationLeave ) {
// stop animation state
- if( isAnimationRunning() ) animation().data()->stop();
+ if( isAnimationRunning() ) animation_->stop();
// reset target
targetItem_ = NoItem;
@@ -236,7 +238,7 @@
}
- animation().data()->start();
+ animation_->start();
}
Index: oxygenclientgroupitemdata.h
===================================================================
--- oxygenclientgroupitemdata.h (revision 1067853)
+++ oxygenclientgroupitemdata.h (working copy)
@@ -29,11 +29,11 @@
//////////////////////////////////////////////////////////////////////////////
#include "oxygenbutton.h"
-#include "lib/oxygenanimation.h"
#include <QtCore/QList>
+#include <QtCore/QPropertyAnimation>
+#include <QtCore/QRect>
#include <QtCore/QWeakPointer>
-#include <QtCore/QRect>
namespace Oxygen
{
@@ -142,8 +142,12 @@
//! true if animation is in progress
bool isAnimationRunning( void ) const
- { return animation().data()->isRunning(); }
+ { return animation_->state() == QAbstractAnimation::Running; }
+ //! sets the duration of the animation
+ void setAnimationDuration( int duration )
+ { animation_->setDuration( duration ); }
+
//! update button activity
void updateButtonActivity( int visibleItem ) const;
@@ -157,10 +161,6 @@
//!@name animation progress
//@{
- //! return animation object
- virtual const Animation::Pointer& animation() const
- { return animation_; }
-
void setProgress( qreal value )
{ progress_ = value; }
@@ -169,6 +169,9 @@
//@}
+ signals:
+ void animationFinished();
+
protected slots:
//! update bounding rects
@@ -184,7 +187,7 @@
bool dirty_;
//! animation
- Animation::Pointer animation_;
+ QPropertyAnimation *animation_;
//! last animation type
AnimationTypes animationType_;
Index: oxygenbutton.cpp
===================================================================
--- oxygenbutton.cpp (revision 1067853)
+++ oxygenbutton.cpp (working copy)
@@ -32,6 +32,7 @@
#include <cmath>
+#include <QtCore/QPropertyAnimation>
#include <QtGui/QPainter>
#include <QtGui/QPen>
@@ -51,7 +52,7 @@
helper_( parent.helper() ),
type_(type),
forceInactive_( false ),
- glowAnimation_( new Animation( 150, this ) ),
+ glowAnimation_( new QPropertyAnimation( this ) ),
glowIntensity_(0)
{
setAutoFillBackground(false);
@@ -66,17 +67,18 @@
setToolTip(tip);
// setup animation
- glowAnimation().data()->setStartValue( 0 );
- glowAnimation().data()->setEndValue( 1.0 );
- glowAnimation().data()->setTargetObject( this );
- glowAnimation().data()->setPropertyName( "glowIntensity" );
+ glowAnimation_->setDuration( 150 );
+ glowAnimation_->setStartValue( 0 );
+ glowAnimation_->setEndValue( 1.0 );
+ glowAnimation_->setTargetObject( this );
+ glowAnimation_->setPropertyName( "glowIntensity" );
// set curve shape. Warning: this is not portable to Qt Kinetic
- glowAnimation().data()->setCurveShape( Animation::EaseInOutCurve );
+ glowAnimation_->setEasingCurve( QEasingCurve::InOutQuad );
// setup connections
- connect( glowAnimation().data(), SIGNAL( valueChanged( const QVariant& ) ), SLOT( update( void ) ) );
- connect( glowAnimation().data(), SIGNAL( finished( void ) ), SLOT( update( void ) ) );
+ connect( glowAnimation_, SIGNAL( valueChanged( const QVariant& ) ), SLOT( update( void ) ) );
+ connect( glowAnimation_, SIGNAL( finished( void ) ), SLOT( update( void ) ) );
reset(0);
}
@@ -95,6 +97,11 @@
else return buttonDetailColor( palette, isActive() || client_.isForcedActive() );
}
+ bool OxygenButton::isAnimated( void ) const
+ {
+ return glowAnimation_->state() == QAbstractAnimation::Running;
+ }
+
//_______________________________________________
QColor OxygenButton::buttonDetailColor(const QPalette &palette, bool active)
{
@@ -126,7 +133,7 @@
//___________________________________________________
void OxygenButton::reset( unsigned long )
- { glowAnimation().data()->setDuration( client_.configuration().animationsDuration() ); }
+ { glowAnimation_->setDuration( client_.configuration().animationsDuration() ); }
//___________________________________________________
@@ -135,8 +142,8 @@
KCommonDecorationButton::enterEvent(e);
if (status_ != Oxygen::Pressed) status_ = Oxygen::Hovered;
- glowAnimation().data()->setDirection( Animation::Forward );
- if( !isAnimated() ) glowAnimation().data()->start();
+ glowAnimation_->setDirection( QAbstractAnimation::Forward );
+ if( !isAnimated() ) glowAnimation_->start();
}
//___________________________________________________
@@ -147,8 +154,8 @@
if( status_ == Oxygen::Hovered )
{
- glowAnimation().data()->setDirection( Animation::Backward );
- if( !isAnimated() ) glowAnimation().data()->start();
+ glowAnimation_->setDirection( QAbstractAnimation::Backward );
+ if( !isAnimated() ) glowAnimation_->start();
}
status_ = Oxygen::Normal;
Index: oxygentitleanimationdata.cpp
===================================================================
--- oxygentitleanimationdata.cpp (revision 1067853)
+++ oxygentitleanimationdata.cpp (working copy)
@@ -37,7 +37,7 @@
TitleAnimationData::TitleAnimationData( QObject* parent ):
QObject( parent ),
dirty_( false ),
- animation_( new Animation( 200, this ) ),
+ animation_( new QPropertyAnimation( this ) ),
opacity_(0)
{}
@@ -46,13 +46,14 @@
{
// setup title animation
- animation().data()->setStartValue( 0 );
- animation().data()->setEndValue( 1 );
- animation().data()->setTargetObject( this );
- animation().data()->setPropertyName( "opacity" );
- animation().data()->setCurveShape( Animation::EaseInOutCurve );
- connect( animation().data(), SIGNAL( valueChanged( const QVariant& ) ), SLOT( updatePixmaps( void ) ) );
- connect( animation().data(), SIGNAL( finished( void ) ), SLOT( updatePixmaps( void ) ) );
+ animation_->setDuration( 200 );
+ animation_->setStartValue( 0 );
+ animation_->setEndValue( 1 );
+ animation_->setTargetObject( this );
+ animation_->setPropertyName( "opacity" );
+ animation_->setEasingCurve( QEasingCurve::InOutQuad );
+ connect( animation_, SIGNAL( valueChanged( const QVariant& ) ), SLOT( updatePixmaps( void ) ) );
+ connect( animation_, SIGNAL( finished( void ) ), SLOT( updatePixmaps( void ) ) );
}
@@ -62,7 +63,7 @@
{
// stop animation
- if( isAnimated() ) animation().data()->stop();
+ if( isAnimated() ) animation_->stop();
// update pixmaps
contrastPixmap_.initialize( rect, contrast );
Index: oxygenclient.cpp
===================================================================
--- oxygenclient.cpp (revision 1067853)
+++ oxygenclient.cpp (working copy)
@@ -83,7 +83,7 @@
KCommonDecorationUnstable(b, f),
factory_( f ),
sizeGrip_( 0 ),
- glowAnimation_( new Animation( 200, this ) ),
+ glowAnimation_( new QPropertyAnimation( this ) ),
titleAnimationData_( new TitleAnimationData( this ) ),
glowIntensity_(0),
initialized_( false ),
@@ -116,22 +116,23 @@
widget()->setAcceptDrops( true );
// setup glow animation
- glowAnimation().data()->setStartValue( glowBias() );
- glowAnimation().data()->setEndValue( 1.0 );
- glowAnimation().data()->setTargetObject( this );
- glowAnimation().data()->setPropertyName( "glowIntensity" );
- glowAnimation().data()->setCurveShape( Animation::EaseInOutCurve );
- connect( glowAnimation().data(), SIGNAL( valueChanged( const QVariant& ) ), widget(), SLOT( update( void ) ) );
- connect( glowAnimation().data(), SIGNAL( finished( void ) ), widget(), SLOT( update( void ) ) );
- connect( glowAnimation().data(), SIGNAL( finished() ), this, SLOT( clearForceActive() ) );
+ glowAnimation_->setDuration( 150 );
+ glowAnimation_->setStartValue( glowBias() );
+ glowAnimation_->setEndValue( 1.0 );
+ glowAnimation_->setTargetObject( this );
+ glowAnimation_->setPropertyName( "glowIntensity" );
+ glowAnimation_->setEasingCurve( QEasingCurve::InOutQuad );
+ connect( glowAnimation_, SIGNAL( valueChanged( const QVariant& ) ), widget(), SLOT( update( void ) ) );
+ connect( glowAnimation_, SIGNAL( finished( void ) ), widget(), SLOT( update( void ) ) );
+ connect( glowAnimation_, SIGNAL( finished() ), this, SLOT( clearForceActive() ) );
// title animation data
- titleAnimationData_.data()->initialize();
- connect( titleAnimationData_.data(), SIGNAL( pixmapsChanged() ), widget(), SLOT( update( void ) ) );
+ titleAnimationData_->initialize();
+ connect( titleAnimationData_, SIGNAL( pixmapsChanged() ), widget(), SLOT( update( void ) ) );
// lists
- connect( itemData_.animation().data(), SIGNAL( finished() ), this, SLOT( clearTargetItem() ) );
+ connect( &itemData_, SIGNAL( animationFinished() ), this, SLOT( clearTargetItem() ) );
// in case of preview, one wants to make the label used
// for the central widget transparent. This allows one to have
@@ -170,12 +171,12 @@
configuration_ = factory_->configuration( *this );
// animations duration
- glowAnimation().data()->setDuration( configuration_.animationsDuration() );
- titleAnimationData_.data()->setDuration( configuration_.animationsDuration() );
- itemData_.animation().data()->setDuration( configuration_.animationsDuration() );
+ glowAnimation_->setDuration( configuration_.animationsDuration() );
+ titleAnimationData_->setDuration( configuration_.animationsDuration() );
+ itemData_.setAnimationDuration( configuration_.animationsDuration() );
// reset title transitions
- titleAnimationData_.data()->reset();
+ titleAnimationData_->reset();
// should also update animations for buttons
resetButtons();
@@ -808,39 +809,39 @@
void OxygenClient::renderTitleText( QPainter* painter, const QRect& rect, const QColor& color, const QColor& contrast ) const
{
- if( !titleAnimationData_.data()->isValid() )
+ if( !titleAnimationData_->isValid() )
{
// contrast pixmap
- titleAnimationData_.data()->reset(
+ titleAnimationData_->reset(
rect,
renderTitleText( rect, caption(), color ),
renderTitleText( rect, caption(), contrast ) );
}
- if( titleAnimationData_.data()->isDirty() )
+ if( titleAnimationData_->isDirty() )
{
// contrast pixmap
- titleAnimationData_.data()->setPixmaps(
+ titleAnimationData_->setPixmaps(
rect,
renderTitleText( rect, caption(), color ),
renderTitleText( rect, caption(), contrast ) );
- titleAnimationData_.data()->setDirty( false );
- titleAnimationData_.data()->startAnimation();
+ titleAnimationData_->setDirty( false );
+ titleAnimationData_->startAnimation();
renderTitleText( painter, rect, color, contrast );
- } else if( titleAnimationData_.data()->isAnimated() ) {
+ } else if( titleAnimationData_->isAnimated() ) {
if( isMaximized() ) painter->translate( 0, 2 );
- if( !titleAnimationData_.data()->contrastPixmap().isNull() )
+ if( !titleAnimationData_->contrastPixmap().isNull() )
{
painter->translate( 0, 1 );
- painter->drawPixmap( rect.topLeft(), titleAnimationData_.data()->contrastPixmap() );
+ painter->drawPixmap( rect.topLeft(), titleAnimationData_->contrastPixmap() );
painter->translate( 0, -1 );
}
- painter->drawPixmap( rect.topLeft(), titleAnimationData_.data()->pixmap() );
+ painter->drawPixmap( rect.topLeft(), titleAnimationData_->pixmap() );
if( isMaximized() ) painter->translate( 0, -2 );
@@ -1126,8 +1127,8 @@
// reset animation
if( animateActiveChange() )
{
- glowAnimation().data()->setDirection( isActive() ? Animation::Forward : Animation::Backward );
- if(!glowIsAnimated()) { glowAnimation().data()->start(); }
+ glowAnimation_->setDirection( isActive() ? QAbstractAnimation::Forward : QAbstractAnimation::Backward );
+ if(!glowIsAnimated()) { glowAnimation_->start(); }
}
// update size grip so that it gets the right color
@@ -1162,7 +1163,7 @@
KCommonDecorationUnstable::captionChange();
itemData_.setDirty( true );
if( animateTitleChange() )
- { titleAnimationData_.data()->setDirty( true ); }
+ { titleAnimationData_->setDirty( true ); }
}
@@ -1500,7 +1501,7 @@
int itemClicked( OxygenClient::itemClicked( point ) );
if( itemClicked < 0 ) return false;
- titleAnimationData_.data()->reset();
+ titleAnimationData_->reset();
QDrag *drag = new QDrag( widget() );
QMimeData *groupData = new QMimeData();
@@ -1678,7 +1679,7 @@
}
- titleAnimationData_.data()->reset();
+ titleAnimationData_->reset();
return true;
}
Index: oxygenbutton.h
===================================================================
--- oxygenbutton.h (revision 1067853)
+++ oxygenbutton.h (working copy)
@@ -31,8 +31,9 @@
#include <kcommondecoration.h>
#include "oxygen.h"
-#include "lib/oxygenanimation.h"
+class QPropertyAnimation;
+
namespace Oxygen
{
class OxygenClient;
@@ -81,10 +82,6 @@
//!@name glow animation
//@{
- //! return animation object
- virtual const Animation::Pointer& glowAnimation() const
- { return glowAnimation_; }
-
void setGlowIntensity( qreal value )
{ glowIntensity_ = value; }
@@ -120,8 +117,7 @@
QColor buttonDetailColor(const QPalette&, bool active );
//! true if animation is in progress
- bool isAnimated( void ) const
- { return glowAnimation().data()->isRunning(); }
+ bool isAnimated( void ) const;
//! true if button is active
bool isActive( void ) const;
@@ -144,12 +140,10 @@
bool forceInactive_;
//! glow animation
- Animation::Pointer glowAnimation_;
+ QPropertyAnimation *glowAnimation_;
//! glow intensity
qreal glowIntensity_;
-
-
};
} //namespace Oxygen
Index: oxygentitleanimationdata.h
===================================================================
--- oxygentitleanimationdata.h (revision 1067853)
+++ oxygentitleanimationdata.h (working copy)
@@ -28,12 +28,11 @@
// IN THE SOFTWARE.
//////////////////////////////////////////////////////////////////////////////
-#include "lib/oxygenanimation.h"
-
#include <cassert>
#include <QtCore/QObject>
#include <QtCore/QPointer>
#include <QtGui/QPixmap>
+#include <QtCore/QPropertyAnimation>
namespace Oxygen
{
@@ -78,8 +77,8 @@
//! duration
void setDuration( int duration )
{
- assert( animation() );
- animation().data()->setDuration( duration );
+ assert( animation_ );
+ animation_->setDuration( duration );
}
//! retrieve contrast pixmap
@@ -95,13 +94,13 @@
bool isAnimated( void ) const
- { return animation().data()->isRunning(); }
+ { return animation_->state() == QAbstractAnimation::Running; }
//! start animation
void startAnimation( void )
{
assert( !isAnimated() );
- animation().data()->start();
+ animation_->start();
}
//@}
@@ -138,12 +137,6 @@
//! update pixmaps
virtual void updatePixmaps( void );
- protected:
-
- //! animation object
- const Animation::Pointer& animation( void ) const
- { return animation_; }
-
private:
//! used to blend pixmap
@@ -213,7 +206,7 @@
BlendedPixmap pixmap_;
//! title animation
- Animation::Pointer animation_;
+ QPropertyAnimation *animation_;
//! title opacity
qreal opacity_;
Index: oxygenclient.h
===================================================================
--- oxygenclient.h (revision 1067853)
+++ oxygenclient.h (working copy)
@@ -32,11 +32,11 @@
#include "oxygenclientgroupitemdata.h"
#include "oxygenconfiguration.h"
#include "oxygentitleanimationdata.h"
-#include "lib/oxygenanimation.h"
#include "lib/helper.h"
#include <kcommondecoration.h>
#include <QBasicTimer>
+#include <QPropertyAnimation>
#include <QTimerEvent>
namespace Oxygen
@@ -78,7 +78,7 @@
//! true if glow is animated
bool glowIsAnimated( void ) const
- { return glowAnimation_.data()->isRunning(); }
+ { return glowAnimation_->state() == QAbstractAnimation::Running; }
//! true when decoration is forced active
bool isForcedActive( void ) const
@@ -112,9 +112,6 @@
//!@name glow animation
//@{
- virtual const Animation::Pointer& glowAnimation( void ) const
- { return glowAnimation_; }
-
void setGlowIntensity( qreal value )
{ glowIntensity_ = value; }
@@ -129,7 +126,7 @@
//! true (biased) intensity
/*! this is needed to have glow go from either 0.2->1 or 0.8->0 depending on the animation direction */
qreal glowIntensity( void ) const
- { return glowAnimation().data()->direction() == Animation::Forward ? glowIntensity_ : glowIntensity_-glowBias(); }
+ { return glowAnimation_->direction() == QAbstractAnimation::Forward ? glowIntensity_ : glowIntensity_-glowBias(); }
//@}
@@ -362,10 +359,10 @@
OxygenConfiguration configuration_;
//! glow animation
- Animation::Pointer glowAnimation_;
+ QPropertyAnimation *glowAnimation_;
//! title animation data
- TitleAnimationData::Pointer titleAnimationData_;
+ TitleAnimationData *titleAnimationData_;
//! glow intensity
qreal glowIntensity_;
signature.asc
(application/pgp-signature, 197 B)
-----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) iEYEABECAAYFAks7ufoACgkQ1rcusafx20Pb5QCglqyaF1HnUWNBVyKe74W1GgK4 okEAn0Zhx3vrFIvp8dG91PxDhfZh9e+u =DPkw -----END PGP SIGNATURE-----