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

List:       kde-commits
Subject:    branches/work/~dang/kdegraphics/kolourpaint/tools/selection
From:       Clarence Dang <dang () kde ! org>
Date:       2007-08-05 4:55:00
Message-ID: 1186289700.385573.30834.nullmailer () svn ! kde ! org
[Download RAW message or body]

SVN commit 696503 by dang:

More selection tool refactoring:

* Move timers out of kpAbstractSelectionTool.cpp into the files responsible
  for those aspects
  - Replace with assertions in the generic kpAbstractSelectionTool.cpp however,
    for all timers

* Push down bits of kpAbstractSelectionTool::beginDraw()



 M  +27 -48    kpAbstractSelectionTool.cpp  
 M  +3 -2      kpAbstractSelectionTool.h  
 M  +56 -23    kpAbstractSelectionTool_Create.cpp  
 M  +15 -3     kpAbstractSelectionTool_Move.cpp  
 M  +19 -0     kpAbstractSelectionTool_ResizeScale.cpp  
 M  +23 -16    text/kpToolText.cpp  
 M  +1 -1      text/kpToolText.h  


--- branches/work/~dang/kdegraphics/kolourpaint/tools/selection/kpAbstractSelectionTool.cpp \
#696502:696503 @@ -66,6 +66,15 @@
 #include <kpViewManager.h>
 
 
+// For either of these timers, they are only active during the "drawing" phase
+// of kpTool.
+static void AssertAllTimersInactive (struct kpAbstractSelectionToolPrivate *d)
+{
+    Q_ASSERT (!d->createNOPTimer->isActive ());
+    Q_ASSERT (!d->RMBMoveUpdateGUITimer->isActive ());
+}
+
+
 kpAbstractSelectionTool::kpAbstractSelectionTool (
         const QString &text,
         const QString &description,
@@ -82,6 +91,10 @@
     initCreate ();
     initMove ();
     initResizeScale ();
+
+    // It would be bad practice to have timers ticking even when this tool
+    // is not in use.
+    ::AssertAllTimersInactive (d);
 }
 
 kpAbstractSelectionTool::~kpAbstractSelectionTool ()
@@ -169,6 +182,8 @@
     kDebug () << "kpAbstractSelectionTool<" << objectName () << ">::begin()" << \
endl;  #endif
 
+    ::AssertAllTimersInactive (d);
+
     kpToolToolBar *tb = toolToolBar ();
     Q_ASSERT (tb);
 
@@ -218,6 +233,8 @@
     d->toolWidgetOpaqueOrTransparent = 0;
 
     viewManager ()->unsetCursor ();
+
+    ::AssertAllTimersInactive (d);
 }
 
 // virtual
@@ -245,8 +262,8 @@
                << endl;
 #endif
 
-    d->createNOPTimer->stop ();
-    d->RMBMoveUpdateGUITimer->stop ();
+    // endDraw() and cancelShape() should have taken care of these.
+    ::AssertAllTimersInactive (d);
 
 
     // In case the cursor was wrong to start with
@@ -273,29 +290,16 @@
     #if DEBUG_KP_TOOL_SELECTION
         kDebug () << "\thas sel region rect=" << sel->boundingRect () << endl;
     #endif
-        QRect selectionRect = sel->boundingRect ();
-
         if (onSelectionResizeHandle () && !controlOrShiftPressed ())
         {
         #if DEBUG_KP_TOOL_SELECTION
             kDebug () << "\t\tis resize/scale" << endl;
         #endif
-
-            d->startDragFromSelectionTopLeft = currentPoint () - \
selectionRect.topLeft ();  d->dragType = ResizeScale;
-            d->resizeScaleType = onSelectionResizeHandle ();
-
-            viewManager ()->setQueueUpdates ();
-            {
-                viewManager ()->setSelectionBorderVisible (true);
-                viewManager ()->setSelectionBorderFinished (true);
-                viewManager ()->setTextCursorEnabled (false);
-            }
-            viewManager ()->restoreQueueUpdates ();
         }
         else if (sel->contains (currentPoint ()))
         {
-            d->dragType = /*virtual*/beginDrawInsideSelection ();
+            d->dragType = /*virtual*/dragTypeInsideSelection ();
         }
         else
         {
@@ -307,30 +311,7 @@
         }
     }
 
-    // creating new selection?
-    if (d->dragType == Create)
-    {
-        viewManager ()->setQueueUpdates ();
-        {
-            // LOREFACTOR: I suspect some calls to viewManager() in this
-            //             file (including this) are redundant since any
-            //             code that tweaks such settings, returns them to
-            //             their original state, after the code is complete.
-            viewManager ()->setSelectionBorderVisible (true);
-
-            viewManager ()->setSelectionBorderFinished (false);
-            viewManager ()->setTextCursorEnabled (false);
-        }
-        viewManager ()->restoreQueueUpdates ();
-
-        // (single shot)
-        d->createNOPTimer->start (200/*ms*/);
-    }
-
-    if (d->dragType != SelectText)
-    {
-        setUserMessage (cancelUserMessage ());
-    }
+    operation (d->dragType, BeginDraw);
 }
 
 
@@ -474,10 +455,6 @@
               << mouseButton () << endl;
 #endif
 
-    d->createNOPTimer->stop ();
-    d->RMBMoveUpdateGUITimer->stop ();
-
-
     viewManager ()->setQueueUpdates ();
     {
         operation (d->dragType, Cancel);
@@ -502,6 +479,9 @@
     d->dragType = Unknown;
     d->cancelledShapeButStillHoldingButtons = true;
     setUserMessage (i18n ("Let go of all the mouse buttons."));
+
+
+    ::AssertAllTimersInactive (d);
 }
 
 // virtual
@@ -571,10 +551,6 @@
     kDebug () << "kpAbstractSelectionTool::endDraw()" << endl;
 #endif
 
-    d->createNOPTimer->stop ();
-    d->RMBMoveUpdateGUITimer->stop ();
-
-
     viewManager ()->setQueueUpdates ();
     {
         operation (d->dragType, EndDraw);
@@ -590,6 +566,9 @@
 
     if (mouseButton () == 1/*right*/)
         popupRMBMenu ();
+
+
+    ::AssertAllTimersInactive (d);
 }
 
 
--- branches/work/~dang/kdegraphics/kolourpaint/tools/selection/kpAbstractSelectionTool.h \
#696502:696503 @@ -108,7 +108,7 @@
 
     enum Operation
     {
-        Draw, Cancel, EndDraw
+        BeginDraw, Draw, Cancel, EndDraw
     };
 
     void createOperation (Operation op);
@@ -122,7 +122,8 @@
     //
     // Overridden in kpToolText.
     // REFACTOR: Not completely clean interface with subclasses due to side effects.
-    virtual DragType beginDrawInsideSelection ();
+    // Later: I just got rid of the side effects.
+    virtual DragType dragTypeInsideSelection () const;
 
 public:
     virtual void beginDraw ();
--- branches/work/~dang/kdegraphics/kolourpaint/tools/selection/kpAbstractSelectionTool_Create.cpp \
#696502:696503 @@ -84,6 +84,27 @@
 }
 
 
+void kpAbstractSelectionTool::beginDrawCreate ()
+{
+    viewManager ()->setQueueUpdates ();
+    {
+        // LOREFACTOR: I suspect some calls to viewManager() in this
+        //             file (including this) are redundant since any
+        //             code that tweaks such settings, returns them to
+        //             their original state, after the code is complete.
+        viewManager ()->setSelectionBorderVisible (true);
+
+        viewManager ()->setSelectionBorderFinished (false);
+        viewManager ()->setTextCursorEnabled (false);
+    }
+    viewManager ()->restoreQueueUpdates ();
+
+    // (single shot)
+    d->createNOPTimer->start (200/*ms*/);
+
+    setUserMessage (cancelUserMessage ());
+}
+
 // private
 void kpAbstractSelectionTool::create (const QPoint &thisPoint, const QRect \
&normalizedRect)  {
@@ -137,29 +158,6 @@
 }
 
 
-void kpAbstractSelectionTool::createOperation (Operation op)
-{
-    switch (op)
-    {
-    case Draw:
-        create (currentPoint (), normalizedRect ());
-        break;
-
-    case Cancel:
-        cancelCreate ();
-        break;
-
-    case EndDraw:
-        // Do nothing.
-        break;
-
-    default:
-        Q_ASSERT (!"Unhandled operation");
-        break;
-    }
-}
-
-
 // protected slot
 void kpAbstractSelectionTool::delayedDraw ()
 {
@@ -192,7 +190,42 @@
     kDebug () << "\twas creating sel - kill" << endl;
 #endif
 
+    d->createNOPTimer->stop ();
+
     // TODO: should we give the user back the selection s/he had before (if any)?
     document ()->selectionDelete ();
 }
 
+// private
+void kpAbstractSelectionTool::endDrawCreate ()
+{
+    d->createNOPTimer->stop ();
+}
+
+void kpAbstractSelectionTool::createOperation (Operation op)
+{
+    switch (op)
+    {
+    case BeginDrw:
+        beginDrawCreate ();
+        break;
+
+    case Draw:
+        create (currentPoint (), normalizedRect ());
+        break;
+
+    case Cancel:
+        cancelCreate ();
+        break;
+
+    case EndDraw:
+        endDrawCreate ();
+        break;
+
+    default:
+        Q_ASSERT (!"Unhandled operation");
+        break;
+    }
+}
+
+
--- branches/work/~dang/kdegraphics/kolourpaint/tools/selection/kpAbstractSelectionTool_Move.cpp \
#696502:696503 @@ -108,12 +108,16 @@
 }
 
 // protected virtual
-kpAbstractSelectionTool::DragType kpAbstractSelectionTool::beginDrawInsideSelection \
() +kpAbstractSelectionTool::DragType kpAbstractSelectionTool::dragInsideSelection ()
 {
 #if DEBUG_KP_TOOL_SELECTION
     kDebug () << "\t\tis move" << endl;
 #endif
+    return kpAbstractSelectionTool::Move;
+}
 
+void kpAbstractSelectionTool::moveBeginDraw ()
+{
     d->startDragFromSelectionTopLeft =
         currentPoint () - document ()->selection ()->topLeft ();
 
@@ -129,7 +133,7 @@
         d->RMBMoveUpdateGUITimer->start (100/*ms*/);
     }
 
-    return kpAbstractSelectionTool::Move;
+    setUserMessage (cancelUserMessage ());
 }
 
 // protected slot
@@ -260,6 +264,8 @@
     kDebug () << "\twas drag moving - undo drag and undo acquire" << endl;
 #endif
 
+    d->RMBMoveUpdateGUITimer->stop ();
+
     // NOP drag?
     if (!d->currentMoveCommand)
         return;
@@ -283,6 +289,8 @@
 
 void kpAbstractSelectionTool::endDrawMove ()
 {
+    d->RMBMoveUpdateGUITimer->stop ();
+
     // NOP drag?
     if (!d->currentMoveCommand)
         return;
@@ -312,10 +320,14 @@
     addNeedingContentCommand (renamedCommand);
 }
 
-void kpAbstractSelectionTool::createOperation (Operation op)
+void kpAbstractSelectionTool::moveOperation (Operation op)
 {
     switch (op)
     {
+    case BeginDraw:
+        beginDrawMove ();
+        break;
+
     case Draw:
         move (currentPoint (), normalizedRect ());
         break;
--- branches/work/~dang/kdegraphics/kolourpaint/tools/selection/kpAbstractSelectionTool_ResizeScale.cpp \
#696502:696503 @@ -90,6 +90,22 @@
     return i18n ("Left drag to scale selection.");
 }
 
+void kpAbstractSelectionTool::beginDrawResizeScale ()
+{
+    d->startDragFromSelectionTopLeft = currentPoint () - selectionRect.topLeft ();
+    d->resizeScaleType = onSelectionResizeHandle ();
+
+    viewManager ()->setQueueUpdates ();
+    {
+        viewManager ()->setSelectionBorderVisible (true);
+        viewManager ()->setSelectionBorderFinished (true);
+        viewManager ()->setTextCursorEnabled (false);
+    }
+    viewManager ()->restoreQueueUpdates ();
+
+    setUserMessage (cancelUserMessage ());
+}
+
 // private
 void kpAbstractSelectionTool::resizeScaleTryKeepAspect (int newWidth, int newHeight,
         bool horizontalGripDragged, bool verticalGripDragged,
@@ -315,6 +331,9 @@
 {
     switch (op)
     {
+    case BeginDraw:
+        beginDrawResizeScale ();
+
     case Draw:
         resizeScale (currentPoint (), normalizedRect ());
         break;
--- branches/work/~dang/kdegraphics/kolourpaint/tools/selection/text/kpToolText.cpp \
#696502:696503 @@ -81,10 +81,32 @@
     return i18n ("Text: Create Box");
 }
 
+void kpAbstractSelectionTool::beginDrawSelectText ()
+{
+    // This path is a bit dangerous since we don't call the base
+    // implementation.
+    //
+    // However, we are doing something unusual here in that we aren't
+    // drag-moving the selection - therefore it makes sense to not
+    // call the base.
+#if DEBUG_KP_TOOL_TEXT
+    kDebug () << "\t\tis select cursor pos" << endl;
+#endif
+
+    Q_ASSERT (document ()->textSelection ());
+    viewManager ()->setTextCursorPosition (
+        document ()->textSelection ()->closestTextRowForPoint (currentPoint ()),
+        document ()->textSelection ()->closestTextColForPoint (currentPoint ()));
+}
+
 void kpAbstractSelectionTool::selectTextOperation (Operation op)
 {
     switch (op)
     {
+    case BeginDraw:
+        beginDrawSelectText ();
+        break;
+
     case Draw:
         // Do nothing.
         break;
@@ -252,25 +274,10 @@
 
 
 // protected virtual [base kpAbstractSelectionTool]
-kpAbstractSelectionTool::DragType kpToolText::beginDrawInsideSelection ()
+kpAbstractSelectionTool::DragType kpToolText::dragInsideSelection () const
 {
     if (onSelectionToSelectText () && !controlOrShiftPressed ())
     {
-        // This path is a bit dangerous since we don't call the base
-        // implementation.
-        //
-        // However, we are doing something unusual here in that we aren't
-        // drag-moving the selection - therefore it makes sense to not
-        // call the base.
-    #if DEBUG_KP_TOOL_TEXT
-        kDebug () << "\t\tis select cursor pos" << endl;
-    #endif
-
-        Q_ASSERT (document ()->textSelection ());
-        viewManager ()->setTextCursorPosition (
-            document ()->textSelection ()->closestTextRowForPoint (currentPoint ()),
-            document ()->textSelection ()->closestTextColForPoint (currentPoint \
                ()));
-
         return kpAbstractSelectionTool::SelectText;
     }
 
--- branches/work/~dang/kdegraphics/kolourpaint/tools/selection/text/kpToolText.h \
#696502:696503 @@ -114,7 +114,7 @@
     virtual bool hasBegunShape () const;
 
 protected:
-    virtual kpAbstractSelectionTool::DragType beginDrawInsideSelection ();
+    virtual kpAbstractSelectionTool::DragType dragInsideSelection () const;
     virtual QCursor cursorInsideSelection () const;
 
     virtual void pullSelectionFromDocumentIfNeeded ();


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

Configure | About | News | Add a list | Sponsored by KoreLogic