From 2d4971eb463a9c273c3f7ccd636504791d52c3ad Mon Sep 17 00:00:00 2001 From: Rob Scheepmaker Date: Fri, 28 Nov 2008 17:06:00 +0000 Subject: [PATCH] Extender Polishing time! The following problems have been adressed: * Far more correct spacer implementation. This avoids the spacer jumping around while dragging an item. Also adjustSizeHints is now only called once/spacer move. * Avoid spacer related memleak. * Only load extenderItems that are actually detached. This way, attached items won't linger around in case of a plasma crash. * Use utilities-desktop-extra as icon for items with no saved icon (e.g. items where the icon is set using setIcon(QIcon) instead of setIcon(QString)). Sure beats the questionmark. * Update mask when offscreen extender items are resized when being dragged to avoid screwed up masks (white borders). * Start the drag only aften being moved a minimum of QApplication::startDragDistance(). * Correct transformation for calls to showDropZone. * Use the mouse position for positioning items in extenders or panels, the topleft corner for positioning in a desktop containment. This feels the most natural. * Move items back to the extender they came from when they're dropped into nowhere. * Some small code style fixes. svn path=/trunk/KDE/kdelibs/; revision=890249 --- extender.cpp | 95 ++++++++++++++++++-------------- extenderitem.cpp | 110 +++++++++++++++++++++++++------------ extenderitem.h | 1 + private/extenderapplet.cpp | 5 +- 4 files changed, 133 insertions(+), 78 deletions(-) diff --git a/extender.cpp b/extender.cpp index 2736b9372..efe9ce84e 100644 --- a/extender.cpp +++ b/extender.cpp @@ -211,24 +211,27 @@ void Extender::itemHoverEnterEvent(ExtenderItem *item) void Extender::itemHoverMoveEvent(ExtenderItem *item, const QPointF &pos) { - int insertIndex = d->insertIndexFromPos(pos); - - if ((insertIndex == d->currentSpacerIndex) || (insertIndex == -1)) { - //relayouting is resource intensive, so don't do that when not necesarry. + if (d->spacerWidget && d->spacerWidget->geometry().contains(pos)) { return; } //Make sure we remove any spacer that might already be in the layout. - itemHoverLeaveEvent(item); + if (d->spacerWidget) { + d->layout->removeItem(d->spacerWidget); + } - d->currentSpacerIndex = insertIndex; + int insertIndex = d->insertIndexFromPos(pos); //Create a widget that functions as spacer, and add that to the layout. - QGraphicsWidget *widget = new QGraphicsWidget(this); - widget->setMinimumSize(QSizeF(item->minimumSize().width(), item->size().height())); - widget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Fixed); - d->spacerWidget = widget; - d->layout->insertItem(insertIndex, widget); + if (!d->spacerWidget) { + QGraphicsWidget *widget = new QGraphicsWidget(this); + widget->setMinimumSize(item->minimumSize()); + widget->setPreferredSize(item->preferredSize()); + widget->setMaximumSize(item->maximumSize()); + widget->setSizePolicy(item->sizePolicy()); + d->spacerWidget = widget; + } + d->layout->insertItem(insertIndex, d->spacerWidget); //Make sure we remove any 'no detachables' label that might be there, and update the layout. d->updateEmptyExtenderLabel(); @@ -357,38 +360,46 @@ void ExtenderPrivate::loadExtenderItems() bool temporarySourceApplet = false; - //find the source applet. - Corona *corona = applet->containment()->corona(); - Applet *sourceApplet = 0; - foreach (Containment *containment, corona->containments()) { - foreach (Applet *applet, containment->applets()) { - if (applet->id() == sourceAppletId) { - sourceApplet = applet; + kDebug() << "applet id = " << applet->id(); + kDebug() << "sourceappletid = " << sourceAppletId; + + if (sourceAppletId != applet->id()) { + //find the source applet. + Corona *corona = applet->containment()->corona(); + Applet *sourceApplet = 0; + foreach (Containment *containment, corona->containments()) { + foreach (Applet *applet, containment->applets()) { + if (applet->id() == sourceAppletId) { + sourceApplet = applet; + } } } - } - //There is no source applet. We just instantiate one just for the sake of creating - //detachables. - if (!sourceApplet) { - kDebug() << "creating a temporary applet as factory"; - sourceApplet = Applet::load(appletName); - temporarySourceApplet = true; - //TODO: maybe add an option to applet to indicate that it shouldn't be deleted after - //having used it as factory. - } - - if (!sourceApplet) { - kDebug() << "sourceApplet is null? appletName = " << appletName; - kDebug() << " extenderItemId = " << extenderItemId; - } else { - ExtenderItem *item = new ExtenderItem(q, extenderItemId.toInt()); - item->setName(extenderItemName); - sourceApplet->initExtenderItem(item); - - if (temporarySourceApplet) { - delete sourceApplet; + //There is no source applet. We just instantiate one just for the sake of creating + //detachables. + if (!sourceApplet) { + kDebug() << "creating a temporary applet as factory"; + sourceApplet = Applet::load(appletName); + temporarySourceApplet = true; + //TODO: maybe add an option to applet to indicate that it shouldn't be deleted after + //having used it as factory. } + + if (!sourceApplet) { + kDebug() << "sourceApplet is null? appletName = " << appletName; + kDebug() << " extenderItemId = " << extenderItemId; + } else { + ExtenderItem *item = new ExtenderItem(q, extenderItemId.toInt()); + sourceApplet->initExtenderItem(item); + + if (temporarySourceApplet) { + delete sourceApplet; + } + } + } else { + //this entry is still here, probably because plasma crashed, but it isn't detached and + //should be reinstantiated. Just delete the entry. + cg.deleteGroup(pair.second); } } @@ -411,9 +422,9 @@ void ExtenderPrivate::adjustSizeHints() //FIXME: what happens in this function are some nasty workarounds for a bug in qt4.4's QGL. //Alexis has told me they are working on a fix for qt4.5, so this can be removed once the bug //has been fixed in Qt. - if (q->layout()) { - q->layout()->updateGeometry(); - q->setMinimumSize(q->layout()->preferredSize()); + if (layout) { + layout->updateGeometry(); + q->setMinimumSize(layout->preferredSize()); } if (applet->layout()) { diff --git a/extenderitem.cpp b/extenderitem.cpp index 736fda585..95ed5e50f 100644 --- a/extenderitem.cpp +++ b/extenderitem.cpp @@ -79,14 +79,18 @@ ExtenderItem::ExtenderItem(Extender *hostExtender, uint extenderItemId) //The item is new dg.writeEntry("sourceAppletPluginName", hostExtender->d->applet->pluginName()); dg.writeEntry("sourceAppletId", hostExtender->d->applet->id()); - dg.writeEntry("sourceIconName", hostExtender->d->applet->icon()); + dg.writeEntry("extenderIconName", hostExtender->d->applet->icon()); d->sourceApplet = hostExtender->d->applet; d->collapseIcon = new IconWidget(KIcon(hostExtender->d->applet->icon()), "", this); } else { //The item already exists. d->name = dg.readEntry("extenderItemName", ""); d->title = dg.readEntry("extenderTitle", ""); - d->collapseIcon = new IconWidget(KIcon(dg.readEntry("extenderIconName", "")), "", this); + QString iconName = dg.readEntry("extenderIconName", "utilities-desktop-extra"); + if (iconName.isEmpty()) { + iconName = "utilities-desktop-extra"; + } + d->collapseIcon = new IconWidget(KIcon(iconName), "", this); //Find the sourceapplet. Corona *corona = hostExtender->d->applet->containment()->corona(); @@ -126,6 +130,10 @@ ExtenderItem::ExtenderItem(Extender *hostExtender, uint extenderItemId) ExtenderItem::~ExtenderItem() { + //make sure the original mousepointer always get's restored. + if (d->mouseOver) { + QApplication::restoreOverrideCursor(); + } delete d; } @@ -470,6 +478,14 @@ void ExtenderItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *opti painter->drawPixmap(d->titleRect().topLeft(), pixmap); } +void ExtenderItem::moveEvent(QGraphicsSceneMoveEvent *event) +{ + if (d->toplevel) { + d->toplevel->setSceneRect(sceneBoundingRect()); + update(); + } +} + void ExtenderItem::resizeEvent(QGraphicsSceneResizeEvent *event) { qreal width = event->newSize().width(); @@ -497,6 +513,12 @@ void ExtenderItem::resizeEvent(QGraphicsSceneResizeEvent *event) d->repositionToolbox(); update(); + + if (d->toplevel) { + d->toplevel->setSceneRect(sceneBoundingRect()); + d->toplevel->setMask(d->background->mask()); + update(); + } } void ExtenderItem::mousePressEvent(QGraphicsSceneMouseEvent *event) @@ -506,30 +528,33 @@ void ExtenderItem::mousePressEvent(QGraphicsSceneMouseEvent *event) return; } - d->mousePressed = true; - d->deltaScene = pos(); - d->mousePos = event->pos().toPoint(); - d->hostApplet()->raise(); - setZValue(d->hostApplet()->zValue()); - - QPointF mousePos = d->scenePosFromScreenPos(event->screenPos()); - - if (!mousePos.isNull()) { - d->extender->itemHoverMoveEvent(this, d->extender->mapFromScene(mousePos)); - } - - d->extender->d->removeExtenderItem(this); QApplication::setOverrideCursor(Qt::ClosedHandCursor); + + d->mousePos = event->pos().toPoint(); + d->deltaScene = pos(); } void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) { - if (!d->mousePressed) { - return; + if ((d->mousePos - event->pos().toPoint()).manhattanLength() + >= QApplication::startDragDistance()) { + d->mousePressed = true; + //start the drag: + //set the zValue to the maximum. + d->hostApplet()->raise(); + setZValue(d->hostApplet()->zValue()); + + QPointF mousePos = d->scenePosFromScreenPos(event->screenPos()); + if (!mousePos.isNull()) { + d->extender->itemHoverMoveEvent(this, d->extender->mapFromScene(mousePos)); + } + d->extender->d->removeExtenderItem(this); + + d->themeChanged(); } - if (d->background->enabledBorders() != FrameSvg::AllBorders) { - d->themeChanged(); + if (!d->mousePressed) { + return; } //keep track of the movement in scene coordinates. we use this to set the position of the @@ -551,10 +576,10 @@ void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) if (!d->toplevel) { //FIXME: duplication from applethandle //create a toplevel view and aim it at the applet. - d->toplevel = new QGraphicsView(corona, 0); - corona->addOffscreenWidget(this); + d->toplevel = new QGraphicsView(scene(), 0); + d->toplevel->setWindowFlags( Qt::ToolTip | Qt::FramelessWindowHint | Qt::WindowStaysOnTopHint); d->toplevel->setFrameShape(QFrame::NoFrame); @@ -575,20 +600,16 @@ void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) d->toplevel->show(); } - //move the toplevel view. d->toplevel->setSceneRect(sceneBoundingRect()); d->toplevel->setGeometry(screenRect); - update(); } else { corona->removeOffscreenWidget(this); setParentItem(d->hostApplet()); setPos(d->deltaScene); //remove the toplevel view. - if (d->toplevel) { - delete d->toplevel; - d->toplevel = 0; - } + delete d->toplevel; + d->toplevel = 0; } //let's insert spacers in applets we're hovering over for some useful visual feedback. @@ -596,7 +617,6 @@ void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) //position in scene coordinates (event->scenePos won't work, since it doesn't take in //consideration that you're leaving the current view). QPointF mousePos = d->scenePosFromScreenPos(event->screenPos()); - QPointF topleft = d->scenePosFromScreenPos(event->screenPos() - d->mousePos); //find the extender we're hovering over. Extender *targetExtender = 0; @@ -618,8 +638,8 @@ void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) } } - if (containment->sceneBoundingRect().contains(topleft) && !targetExtender) { - containment->showDropZone(event->screenPos() - d->mousePos); + if (containment->sceneBoundingRect().contains(mousePos) && !targetExtender) { + containment->showDropZone(containment->mapFromScene(mousePos).toPoint()); } else { containment->showDropZone(QPoint()); } @@ -696,7 +716,7 @@ void ExtenderItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) //find the extender we're hovering over. Extender *targetExtender = 0; - Corona *corona = qobject_cast(scene()); + Corona *corona = qobject_cast(d->extender->d->applet->scene()); corona->removeOffscreenWidget(this); @@ -722,19 +742,38 @@ void ExtenderItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) } } else { //apparently, it is not, so instantiate a new ExtenderApplet. - //TODO: maybe we alow the user to choose a default extenderapplet. - kDebug() << "Instantiate a new ExtenderApplet"; - mousePos = d->scenePosFromScreenPos(event->screenPos() - d->mousePos); + bool extenderCreated = false; + mousePos = d->scenePosFromScreenPos(event->screenPos()); if (!mousePos.isNull()) { foreach (Containment *containment, corona->containments()) { if (containment->sceneBoundingRect().contains(mousePos)) { + kDebug() << "Instantiate a new ExtenderApplet"; + + //when on a desktopcontainment, we use the widgets topleft corner to + //position the applet in the containment, on other containments, we use the + //actual mouse position. + QPointF position; + if (containment->type() == Plasma::Containment::DesktopContainment) { + position = containment->mapFromScene( + d->scenePosFromScreenPos(event->screenPos() - d->mousePos)); + } else { + position = containment->mapFromScene(mousePos); + } + Applet *applet = containment->addApplet("internal:extender", QVariantList(), QRectF(containment->mapFromScene(mousePos), size())); setExtender(applet->d->extender); + extenderCreated = true; } } } + + //if no containment is at the position where the item is dropped, just move the item + //back to where it came from. + if (!extenderCreated) { + setExtender(extender()); + } } QApplication::restoreOverrideCursor(); @@ -995,8 +1034,11 @@ void ExtenderItemPrivate::sourceAppletRemoved() qreal ExtenderItemPrivate::iconSize() { + //read the icon size hint, and enforce a minimum size of 16x16 QSizeF size = dragger->elementSize("hint-preferred-icon-size"); + size = size.expandedTo(QSizeF(16,16)); + //return the size of the text, if that is larger then the recommended icon size Plasma::Theme *theme = Plasma::Theme::defaultTheme(); QFont font = theme->font(Plasma::Theme::DefaultFont); QFontMetrics fm(font); diff --git a/extenderitem.h b/extenderitem.h index 3f9bbe52e..046c280f0 100644 --- a/extenderitem.h +++ b/extenderitem.h @@ -232,6 +232,7 @@ class PLASMA_EXPORT ExtenderItem : public QGraphicsWidget protected: void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget); + void moveEvent(QGraphicsSceneMoveEvent *event); void resizeEvent(QGraphicsSceneResizeEvent *event); void mousePressEvent(QGraphicsSceneMouseEvent *event); diff --git a/private/extenderapplet.cpp b/private/extenderapplet.cpp index 3905f4536..1278cccfb 100644 --- a/private/extenderapplet.cpp +++ b/private/extenderapplet.cpp @@ -27,7 +27,6 @@ ExtenderApplet::ExtenderApplet(QObject *parent, const QVariantList &args) : Plasma::PopupApplet(parent, args) { - setPopupIcon("utilities-desktop-extra"); } ExtenderApplet::~ExtenderApplet() @@ -36,6 +35,8 @@ ExtenderApplet::~ExtenderApplet() void ExtenderApplet::init() { + setPopupIcon("utilities-desktop-extra"); + QGraphicsLinearLayout *layout = new QGraphicsLinearLayout(this); layout->setSpacing(0); setLayout(layout); @@ -47,7 +48,7 @@ void ExtenderApplet::init() this, SLOT(itemDetached(Plasma::ExtenderItem*))); layout->addItem(extender()); - updateGeometry(); + //updateGeometry(); } void ExtenderApplet::itemDetached(Plasma::ExtenderItem *)