GNOME.org
 

[dasher] Assign default group colours better and properly handle invisible groups

commit fdd2dfa6629f478d08c340e395b616869daf87b3
Author: Alan Lawrence <acl33 inf phy cam ac uk>
Date:   Wed Apr 13 10:46:18 2011 +0100

    Assign default group colours better and properly handle invisible groups
    
    AlphIO: assign colour to group only if none (or -1) specified, ensuring
     the chosen colour is different to both parent and previous sibling.
     Also tidy group setup code and remove m_bFirstGroup (redundant)
    
    DasherNode: add NF_VISIBLE flag, set by default in c'tor; AlphMgr clears for
     invisible groups.
    
    DasherViewSquare: fill and outline node iff NF_VISIBLE set, rather than
     comparing to parent colour. Rm parent_colour param of recursive render methods.

 Src/DasherCore/Alphabet/AlphIO.cpp  |   61 ++++++++++++++++-------------------
 Src/DasherCore/Alphabet/AlphIO.h    |    1 -
 Src/DasherCore/AlphabetManager.cpp  |    1 +
 Src/DasherCore/DasherNode.cpp       |   10 +----
 Src/DasherCore/DasherNode.h         |   57 +++++++++++++++++++-------------
 Src/DasherCore/DasherViewSquare.cpp |   41 +++++++++++------------
 Src/DasherCore/DasherViewSquare.h   |    6 ++--
 7 files changed, 88 insertions(+), 89 deletions(-)
---
diff --git a/Src/DasherCore/Alphabet/AlphIO.cpp b/Src/DasherCore/Alphabet/AlphIO.cpp
index 9f360d4..4e41ebc 100644
--- a/Src/DasherCore/Alphabet/AlphIO.cpp
+++ b/Src/DasherCore/Alphabet/AlphIO.cpp
@@ -430,7 +430,6 @@ void CAlphIO::XML_StartElement(void *userData, const XML_Char *name, const XML_C
     Me->InputInfo->Mutable = Me->LoadMutable;
     Me->ParagraphCharacter = NULL;
     Me->SpaceCharacter = NULL;
-    Me->bFirstGroup = true;
     Me->iGroupIdx = 0;
     while(*atts != 0) {
       if(strcmp(*atts, "name") == 0) {
@@ -506,17 +505,11 @@ void CAlphIO::XML_StartElement(void *userData, const XML_Char *name, const XML_C
   if(strcmp(name, "group") == 0) {
     SGroupInfo *pNewGroup(new SGroupInfo);
     pNewGroup->iNumChildNodes=0;
-    pNewGroup->iColour = (Me->iGroupIdx % 3) + 110;
-    ++Me->iGroupIdx;
+    pNewGroup->iColour = -1; //marker for "none specified"; if so, will compute later
     if (Me->m_vGroups.empty()) Me->InputInfo->iNumChildNodes++; else Me->m_vGroups.back()->iNumChildNodes++;
 
-    if(Me->bFirstGroup) {
-      pNewGroup->bVisible = false;
-      Me->bFirstGroup = false;
-    }
-    else {
-      pNewGroup->bVisible = true;
-    }
+    //by default, the first group in the alphabet is invisible
+    pNewGroup->bVisible = (Me->InputInfo->m_pBaseGroup!=NULL);
 
     while(*atts != 0) {
       if(strcmp(*atts, "name") == 0) {
@@ -527,39 +520,41 @@ void CAlphIO::XML_StartElement(void *userData, const XML_Char *name, const XML_C
 //         atts--;
       }
       if(strcmp(*atts, "b") == 0) {
+        pNewGroup->iColour = atoi(*(atts+1));
+      } else if(strcmp(*atts, "visible") == 0) {
         atts++;
-	pNewGroup->iColour = atoi(*atts);
-        atts--;
-      }
-      if(strcmp(*atts, "visible") == 0) {
-	atts++;
-	if(!strcmp(*atts, "yes") || !strcmp(*atts, "on"))
-	  pNewGroup->bVisible = true;
-	else if(!strcmp(*atts, "no") || !strcmp(*atts, "off"))
-	  pNewGroup->bVisible = false;
-	atts--;
-      }
-      if(strcmp(*atts, "label") == 0) {
-        atts++;
-	pNewGroup->strLabel = *atts;
+        if(!strcmp(*atts, "yes") || !strcmp(*atts, "on"))
+          pNewGroup->bVisible = true;
+        else if(!strcmp(*atts, "no") || !strcmp(*atts, "off"))
+          pNewGroup->bVisible = false;
         atts--;
+      } else if(strcmp(*atts, "label") == 0) {
+        pNewGroup->strLabel = *(atts+1);
       }
       atts += 2;
     }
 
+    SGroupInfo *&prevSibling(Me->m_vGroups.empty() ? Me->InputInfo->m_pBaseGroup : Me->m_vGroups.back()->pChild);
+    
+    if (pNewGroup->iColour==-1 && pNewGroup->bVisible) {
+      //no colour specified. Try to colour cycle, but make sure we choose
+      // a different colour from both its parent and any previous sibling
+      for (;;) {
+        pNewGroup->iColour=(Me->iGroupIdx++ % 3) + 110;
+        if (!Me->m_vGroups.empty() && Me->m_vGroups.back()->iColour == pNewGroup->iColour)
+          continue; //same colour as parent -> try again
+        if (prevSibling && prevSibling->iColour == pNewGroup->iColour)
+          continue; //same colour as previous sibling -> try again
+        break; //different from parent and previous sibling (if any!), so ok
+      }
+    }
+    
     pNewGroup->iStart = Me->InputInfo->m_vCharacters.size()+1;
 
     pNewGroup->pChild = NULL;
 
-    if(Me->m_vGroups.size() > 0) {
-      pNewGroup->pNext = Me->m_vGroups.back()->pChild;
-      Me->m_vGroups.back()->pChild = pNewGroup;
-    }
-    else {
-      pNewGroup->pNext = Me->InputInfo->m_pBaseGroup;
-      Me->InputInfo->m_pBaseGroup = pNewGroup;
-    }
-
+    pNewGroup->pNext = prevSibling;
+    prevSibling = pNewGroup;
 
     Me->m_vGroups.push_back(pNewGroup);
 
diff --git a/Src/DasherCore/Alphabet/AlphIO.h b/Src/DasherCore/Alphabet/AlphIO.h
index ac01426..d501ffa 100644
--- a/Src/DasherCore/Alphabet/AlphIO.h
+++ b/Src/DasherCore/Alphabet/AlphIO.h
@@ -91,7 +91,6 @@ private:
   // Data gathered
   std::string CData;            // Text gathered from when an elemnt starts to when it ends
   CAlphInfo *InputInfo;
-  bool bFirstGroup;
   int iGroupIdx;
 
   // Callback functions. These involve the normal dodgy casting to a pointer
diff --git a/Src/DasherCore/AlphabetManager.cpp b/Src/DasherCore/AlphabetManager.cpp
index dc1c72e..f123879 100644
--- a/Src/DasherCore/AlphabetManager.cpp
+++ b/Src/DasherCore/AlphabetManager.cpp
@@ -176,6 +176,7 @@ CAlphabetManager::CGroupNode::CGroupNode(CDasherNode *pParent, int iOffset, unsi
             pGroup ? (pGroup->bVisible ? pGroup->iColour : iBkgCol)
             : (iOffset&1) ? 7 : 137, //special case for root nodes
             pGroup ? strEnc+pGroup->strLabel : strEnc, pMgr), m_pGroup(pGroup) {
+  if (m_pGroup && !m_pGroup->bVisible) SetFlag(NF_VISIBLE, false);
 }
 
 CAlphabetManager::CAlphNode *CAlphabetManager::GetRoot(CDasherNode *pParent, unsigned int iLower, unsigned int iUpper, bool bEnteredLast, int iOffset) {
diff --git a/Src/DasherCore/DasherNode.cpp b/Src/DasherCore/DasherNode.cpp
index ef247fe..5f918fe 100644
--- a/Src/DasherCore/DasherNode.cpp
+++ b/Src/DasherCore/DasherNode.cpp
@@ -43,20 +43,14 @@ int Dasher::currentNumNodeObjects() {return iNumNodes;}
 
 //TODO this used to be inline - should we make it so again?
 CDasherNode::CDasherNode(CDasherNode *pParent, int iOffset, unsigned int iLbnd, unsigned int iHbnd, int iColour, const string &strDisplayText)
-: m_pParent(pParent), m_iOffset(iOffset), m_iLbnd(iLbnd), m_iHbnd(iHbnd), m_iColour(iColour), m_strDisplayText(strDisplayText) {
+: m_pParent(pParent), m_iFlags(DEFAULT_FLAGS), onlyChildRendered(NULL), m_iLbnd(iLbnd), m_iHbnd(iHbnd), m_iOffset(iOffset), m_iColour(iColour), m_strDisplayText(strDisplayText) {
   DASHER_ASSERT(iHbnd >= iLbnd);
 
   if (pParent) {
     DASHER_ASSERT(!pParent->GetFlag(NF_ALLCHILDREN));
     pParent->Children().push_back(this);
   }
-
-  onlyChildRendered = NULL;
-
-  // Default flags (make a definition somewhere, pass flags to constructor?)
-  m_iFlags = 0;
-
-  m_iRefCount = 0;
+	
   iNumNodes++;
 }
 
diff --git a/Src/DasherCore/DasherNode.h b/Src/DasherCore/DasherNode.h
index f503208..3c2767c 100644
--- a/Src/DasherCore/DasherNode.h
+++ b/Src/DasherCore/DasherNode.h
@@ -37,14 +37,40 @@ namespace Dasher {
 #include <vector>
 
 // Node flag constants
+/// NF_COMMITTED: The node is 'above' the root, i.e. all onscreen parts of it
+/// are covered by the root; this is when we train the language model etc
 #define NF_COMMITTED 1
+
+/// NF_SEEN - Node is under the crosshair and has (already) been output
 #define NF_SEEN 2
+
+/// NF_CONVERTED - Node has been converted (eg Japanese mode)
 #define NF_CONVERTED 4
+
+/// NF_GAME - Node is on the path in game mode
 #define NF_GAME 8
+
+/// NF_ALLCHILDREN - Node has all children. TODO Since nodes only
+/// ever have all their children, or none of them, can we not
+/// just check it has children, and get rid of this flag?
 #define NF_ALLCHILDREN 16
+
+/// NF_SUPER - Node covers entire visible area (and so is eligible
+/// to be made the new root)
 #define NF_SUPER 32
+
+/// NF_END_GAME - Node is the last one of the phrase in game mode
 #define NF_END_GAME 64
 
+/// NF_VISIBLE - an invisible node is one which lets its parent's
+/// colour show through, and has no outline drawn round it (it may
+/// still have a label). Note that this flag is set (i.e. the node
+/// is drawn and outlined) by default in the constructor.
+#define NF_VISIBLE 128
+
+///Flags to assign to a newly created node:
+#define DEFAULT_FLAGS NF_VISIBLE
+
 /// \ingroup Model
 /// @{
 
@@ -80,11 +106,14 @@ class Dasher::CDasherNode:private NoClones {
 
   /// @brief Constructor
   ///
-  /// @param pParent Parent of the new node
+  /// Note the flags of the new node are initialized to DEFAULT_FLAGS.
+  ///
+  /// @param pParent Parent of the new node; automatically adds self to children
+  /// @param iOffset index into text box of character being/last entered
   /// @param iLbnd Lower bound of node within parent
   /// @param iHbnd Upper bound of node within parent
-  /// @param pDisplayInfo Struct containing information on how to display the node
-  ///
+  /// @param iColour colour to render node; for invisible nodes, should be same as parent.
+  /// @param strDisplayText label to render upon node
   CDasherNode(CDasherNode *pParent, int iOffset, unsigned int iLbnd, unsigned int iHbnd, int iColour, const std::string &strDisplayText);
 
   /// @brief Destructor
@@ -98,24 +127,8 @@ class Dasher::CDasherNode:private NoClones {
 
   /// @brief Set a node flag
   ///
-  /// Set various flags corresponding to the state of the node. The following flags are defined:
-  ///
-  /// NF_COMMITTED - Node is 'above' the root, so corresponding symbol
-  /// has been added to text box, language model trained etc
-  ///
-  /// NF_SEEN - Node has already been output
-  ///
-  /// NF_CONVERTED - Node has been converted (eg Japanese mode)
-  ///
-  /// NF_GAME - Node is on the path in game mode
-  ///
-  /// NF_ALLCHILDREN - Node has all children (TODO: obsolete?)
-  ///
-  /// NF_SUPER - Node covers entire visible area
-  ///
-  /// NF_END_GAME - Node is the last one of the phrase in game mode
-  ///
-  ///
+  /// Set various flags corresponding to the state of the node. 
+  /// 
   /// @param iFlag The flag to set
   /// @param bValue The new value of the flag
   ///
@@ -269,8 +282,6 @@ class Dasher::CDasherNode:private NoClones {
   unsigned int m_iLbnd;
   unsigned int m_iHbnd;   // the cumulative lower and upper bound prob relative to parent
 
-  int m_iRefCount;              // reference count if ancestor of (or equal to) root node
-
   ChildMap m_mChildren;         // pointer to array of children
   CDasherNode *m_pParent;       // pointer to parent
 
diff --git a/Src/DasherCore/DasherViewSquare.cpp b/Src/DasherCore/DasherViewSquare.cpp
index f2e4004..a08c9bd 100644
--- a/Src/DasherCore/DasherViewSquare.cpp
+++ b/Src/DasherCore/DasherViewSquare.cpp
@@ -99,14 +99,6 @@ CDasherNode *CDasherViewSquare::Render(CDasherNode *pRoot, myint iRootMin, myint
   VisibleRegion(iDasherMinX, iDasherMinY, iDasherMaxX, iDasherMaxY);
   //
 
-  screenint iScreenLeft;
-  screenint iScreenTop;
-  screenint iScreenRight;
-  screenint iScreenBottom;
-
-  Dasher2Screen(iRootMax-iRootMin, iRootMin, iScreenLeft, iScreenTop);
-  Dasher2Screen(0, iRootMax, iScreenRight, iScreenBottom);
-
   m_iRenderCount = 0;
 
   CDasherNode *pOutput = pRoot->Parent();
@@ -127,11 +119,19 @@ CDasherNode *CDasherViewSquare::Render(CDasherNode *pRoot, myint iRootMin, myint
     DasherDrawRectangle(0, iDasherMinY, iDasherMinX, iDasherMaxY, 0, -1, 0);
 
     //and render root.
-    DisjointRender(pRoot, iRootMin, iRootMax, NULL, policy, std::numeric_limits<double>::infinity(), 0, pOutput);
+    DisjointRender(pRoot, iRootMin, iRootMax, NULL, policy, std::numeric_limits<double>::infinity(), pOutput);
   } else {
     //overlapping rects/shapes
-    Screen()->DrawRectangle(0, 0, Screen()->GetWidth(), Screen()->GetHeight(), 0, -1, 0);
-    NewRender(pRoot, iRootMin, iRootMax, NULL, policy, std::numeric_limits<double>::infinity(), 0, pOutput);
+    if (pOutput) {
+      //LEFT of Y axis, would be entirely covered by the root node parent (before we render root)
+      // (getColour() gives the right colour, even if pOutput is invisible - in that case it gives
+      // the colour of its parent)
+      DasherDrawRectangle(iDasherMaxX, iDasherMinY, 0, iDasherMaxY, pOutput->getColour(), -1, 0);
+      //RIGHT of Y axis, should be white.
+      DasherDrawRectangle(0, iDasherMinY, iDasherMinX, iDasherMaxY, 0, -1, 0);
+    } else //easy case, whole screen is white (outside root node, e.g. when starting)
+      Screen()->DrawRectangle(0, 0, Screen()->GetWidth(), Screen()->GetHeight(), 0, -1, 0);
+    NewRender(pRoot, iRootMin, iRootMax, NULL, policy, std::numeric_limits<double>::infinity(), pOutput);
   }
 
   // Labels are drawn in a second parse to get the overlapping right
@@ -465,7 +465,7 @@ bool CDasherViewSquare::IsSpaceAroundNode(myint y1, myint y2) {
 
 void CDasherViewSquare::DisjointRender(CDasherNode *pRender, myint y1, myint y2,
 					CTextString *pPrevText, CExpansionPolicy &policy, double dMaxCost,
-					int parent_color, CDasherNode *&pOutput)
+					CDasherNode *&pOutput)
 {
   DASHER_ASSERT_VALIDPTR_RW(pRender);
 
@@ -534,8 +534,8 @@ void CDasherViewSquare::DisjointRender(CDasherNode *pRender, myint y1, myint y2,
 
         if (newy2-newy1 < iDasherMaxX) //fill in to it's left...
           DasherDrawRectangle(std::min(Range,iDasherMaxX), std::max(y1,iDasherMinY), newy2-newy1, std::min(y2,iDasherMaxY), myColor, -1, 0);
-        DisjointRender(pChild, newy1, newy2, pPrevText,
-                        policy, dMaxCost, myColor, pOutput);
+        DisjointRender(pChild, newy1, newy2, pPrevText, 
+                        policy, dMaxCost, pOutput);
         //leave pRender->onlyChildRendered set, so remaining children are skipped
       }
       else
@@ -557,7 +557,7 @@ void CDasherViewSquare::DisjointRender(CDasherNode *pRender, myint y1, myint y2,
           pRender->onlyChildRendered = pChild;
           if (newy2-newy1 < iDasherMaxX)
             DasherDrawRectangle(std::min(Range,iDasherMaxX), std::max(y1,iDasherMinY), newy2-newy1, std::min(y2,iDasherMaxY), myColor, -1, 0);
-          DisjointRender(pChild, newy1, newy2, pPrevText, policy, dMaxCost, myColor, pOutput);
+          DisjointRender(pChild, newy1, newy2, pPrevText, policy, dMaxCost, pOutput);
           //ensure we don't blank over this child in "finishing off" the parent (!)
           lasty=newy2;
           //all remaining children are offscreen. quickly delete, avoid recomputing ranges...
@@ -574,7 +574,7 @@ void CDasherViewSquare::DisjointRender(CDasherNode *pRender, myint y1, myint y2,
           if (std::max(lasty,iDasherMinY)<newy1) //fill in interval above child up to the last drawn child
             DasherDrawRectangle(std::min(Range,iDasherMaxX), std::max(lasty,iDasherMinY),0, std::min(newy1,iDasherMaxY), myColor, -1, 0);
           lasty = newy2;
-          DisjointRender(pChild, newy1, newy2, pPrevText, policy, dMaxCost, myColor, pOutput);
+          DisjointRender(pChild, newy1, newy2, pPrevText, policy, dMaxCost, pOutput);
         } else {
           // We get here if the node is too small to render or is off-screen.
           // So, collapse it immediately.
@@ -594,7 +594,7 @@ void CDasherViewSquare::DisjointRender(CDasherNode *pRender, myint y1, myint y2,
     //end rendering children, fall through to outline
   }
   // Lastly, draw the outline
-  if(GetLongParameter(LP_OUTLINE_WIDTH) && (!pRender->Parent() || pRender->getColour()!=pRender->Parent()->getColour())) {
+  if(GetLongParameter(LP_OUTLINE_WIDTH) && pRender->GetFlag(NF_VISIBLE)) {
     DasherDrawRectangle(std::min(Range,iDasherMaxX), std::max(y1,iDasherMinY),0, std::min(y2,iDasherMaxY), -1, -1, abs(GetLongParameter(LP_OUTLINE_WIDTH)));
   }
 }
@@ -635,7 +635,7 @@ bool CDasherViewSquare::CoversCrosshair(myint Range, myint y1, myint y2) {
 
 void CDasherViewSquare::NewRender(CDasherNode *pRender, myint y1, myint y2,
                                   CTextString *pPrevText, CExpansionPolicy &policy, double dMaxCost,
-                                  int parent_color, CDasherNode *&pOutput)
+                                  CDasherNode *&pOutput)
 {
 	//when we have only one child node to render, which'll be the last thing we
 	// do before returning, we make a tail call by jumping here, rather than
@@ -679,7 +679,7 @@ beginning:
   // _supposed_ to be the same colour as their parent, will have no outlines...
   // (thankfully having 2 "phases" means this doesn't happen in standard
   // colour schemes)
-  if (myColor!=parent_color) {
+  if (pRender->GetFlag(NF_VISIBLE)) { 
 	//outline width 0 = fill only; >0 = fill + outline; <0 = outline only
 	int fillColour = GetLongParameter(LP_OUTLINE_WIDTH)>=0 ? myColor : -1;
 	int lineWidth = abs(GetLongParameter(LP_OUTLINE_WIDTH));
@@ -737,7 +737,6 @@ beginning:
 	    (newy1 < iDasherMinY && newy2 > iDasherMaxY)) { //covers entire y-axis!
          //render just that child; nothing more to do for this node => tail call to beginning
          pRender = pChild; y1=newy1; y2=newy2;
-         parent_color = myColor;
          goto beginning;
     }
     pRender->onlyChildRendered = NULL;
@@ -753,7 +752,7 @@ beginning:
     if (newy1<=iDasherMaxY && newy2 >= iDasherMinY) { //onscreen
       if (newy2-newy1 > GetLongParameter(LP_MIN_NODE_SIZE)) {
         //definitely big enough to render.
-        NewRender(pChild, newy1, newy2, pPrevText, policy, dMaxCost, myColor, pOutput);
+        NewRender(pChild, newy1, newy2, pPrevText, policy, dMaxCost, pOutput);
         if (newy2 > iDasherMaxY) {
           //remaining children offscreen.
           if (newy1 < iDasherMinY) pRender->onlyChildRendered = pChild; //...and previous were too!
diff --git a/Src/DasherCore/DasherViewSquare.h b/Src/DasherCore/DasherViewSquare.h
index f514a02..bfb4319 100644
--- a/Src/DasherCore/DasherViewSquare.h
+++ b/Src/DasherCore/DasherViewSquare.h
@@ -147,14 +147,14 @@ private:
   /// (i.e. appropriate for LP_SHAPE_TYPE==0). Each call responsible for rendering
   /// exactly the area contained within the node.
   /// @param pOutput The innermost node covering the crosshair (if any)
-  void DisjointRender(CDasherNode * Render, myint y1, myint y2, CTextString *prevText, CExpansionPolicy &policy, double dMaxCost,int parent_color, CDasherNode *&pOutput);
-
+  void DisjointRender(CDasherNode * Render, myint y1, myint y2, CTextString *prevText, CExpansionPolicy &policy, double dMaxCost, CDasherNode *&pOutput);
+  
   /// (Recursively) render a node and all contained subnodes, in overlapping shapes
   /// (according to LP_SHAPE_TYPE: 1=rects, 2=triangles, 3=truncated triangles,
   /// 4=quadrics, 5=semicircles)
   /// Each call responsible for rendering exactly the area contained within the node.
   /// @param pOutput The innermost node covering the crosshair (if any)
-  void NewRender(CDasherNode * Render, myint y1, myint y2, CTextString *prevText, CExpansionPolicy &policy, double dMaxCost,int parent_color, CDasherNode *&pOutput);
+  void NewRender(CDasherNode * Render, myint y1, myint y2, CTextString *prevText, CExpansionPolicy &policy, double dMaxCost, CDasherNode *&pOutput);
 
 #ifdef _WIN32
   ///
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]