[gui] Fixed crash. Added comments and some code style fixes.

gui/button.cpp

@@ -111,6 +111,9 @@ namespace gui
 
   void Button::cacheButtonBody(EState state)
   {
+    double const k = visualScale();
+    m2::RectD const rr = GetBoundRect();
+
     graphics::Screen * cs = m_controller->GetCacheScreen();
 
     cs->beginFrame();
@@ -122,9 +125,6 @@ namespace gui
 
     cs->setDisplayList(dl.get());
 
-    double const k = visualScale();
-    m2::RectD const rr = GetBoundRect();
-
     cs->drawRoundedRectangle(m2::RectD(-rr.SizeX() / 2,
                                        -rr.SizeY() / 2,
                                         rr.SizeX() / 2,
@@ -184,16 +184,16 @@ namespace gui
 
   void Button::draw(graphics::OverlayRenderer * r, math::Matrix<double, 3, 3> const & m) const
   {
-    if (!isVisible())
-      return;
+    if (isVisible())
+    {
+      checkDirtyLayout();
 
-    checkDirtyLayout();
+      math::Matrix<double, 3, 3> const drawM = math::Shift(math::Identity<double, 3>(), pivot());
 
-    math::Matrix<double, 3, 3> const drawM = math::Shift(math::Identity<double, 3>(), pivot());
+      r->drawDisplayList(m_dls.at(state()).get(), drawM * m);
 
-    r->drawDisplayList(m_dls.at(state()).get(), drawM * m);
-
-    m_textView->draw(r, m);
+      m_textView->draw(r, m);
+    }
   }
 
   void Button::setPivot(m2::PointD const & pv)

gui/cached_text_view.cpp

@@ -43,8 +43,6 @@ namespace gui
 
   void CachedTextView::GetMiniBoundRects(RectsT & rects) const
   {
-    checkDirtyLayout();
-
     rects.resize(m_layout->boundRects().size());
     copy(m_layout->boundRects().begin(),
          m_layout->boundRects().end(),

gui/cached_text_view.hpp

@@ -38,6 +38,8 @@ namespace gui
 
     void setText(string const & text);
 
+    /// @name Overrider from graphics::OverlayElement and gui::Element.
+    //@{
     virtual void GetMiniBoundRects(RectsT & rects) const;
     void draw(graphics::OverlayRenderer * r, math::Matrix<double, 3, 3> const & m) const;
 
@@ -48,5 +50,6 @@ namespace gui
     void setFont(EState state, graphics::FontDesc const & desc);
 
     void setPivot(m2::PointD const & pv);
+    //@}
   };
 }

gui/controller.cpp

@@ -1,13 +1,11 @@
 #include "controller.hpp"
 #include "element.hpp"
-#include "display_list_cache.hpp"
 
-#include "../map/drawer.hpp"
-
-#include "../graphics/overlay.hpp"
+#include "../graphics/screen.hpp"
 
 #include "../std/bind.hpp"
 
+
 namespace gui
 {
   Controller::RenderParams::RenderParams()
@@ -31,38 +29,29 @@ namespace gui
   Controller::~Controller()
   {}
 
-  void Controller::SelectElements(m2::PointD const & pt, elem_list_t & l, bool onlyVisible)
+  shared_ptr<Element> Controller::SelectTopElement(m2::PointD const & pt, bool onlyVisible) const
   {
-    for (elem_list_t::const_iterator it = m_Elements.begin();
-         it != m_Elements.end();
-         ++it)
+    shared_ptr<Element> res;
+
+    for (ElemsT::const_iterator it = m_Elements.begin(); it != m_Elements.end(); ++it)
     {
       shared_ptr<gui::Element> const & e = *it;
       if ((!onlyVisible || e->isVisible()) && e->hitTest(pt))
-        l.push_back(e);
+      {
+        if (!res || e->depth() > res->depth())
+          res = e;
+      }
     }
-  }
 
-  namespace
-  {
-    bool DepthGreater(const shared_ptr<Element> & e1,
-                      const shared_ptr<Element> & e2)
-    {
-      return e1->depth() > e2->depth();
-    }
+    return res;
   }
 
   bool Controller::OnTapStarted(m2::PointD const & pt)
   {
-    elem_list_t l;
-
-    SelectElements(pt, l, true);
-    l.sort(DepthGreater);
-
-    /// selecting first hit-tested element from the list
-    if (!l.empty())
+    shared_ptr<Element> e = SelectTopElement(pt, true);
+    if (e)
     {
-      m_focusedElement = l.front();
+      m_focusedElement = e;
       m_focusedElement->onTapStarted(pt);
       m_LastTapCancelled = false;
       return true;
@@ -86,7 +75,7 @@ namespace gui
           m_focusedElement->onTapMoved(pt);
       }
 
-      /// event handled
+      // event handled
       return true;
     }
 
@@ -132,7 +121,7 @@ namespace gui
 
   void Controller::RemoveElement(shared_ptr<Element> const & e)
   {
-    elem_list_t::iterator it = find(m_Elements.begin(), m_Elements.end(), e);
+    ElemsT::iterator it = find(m_Elements.begin(), m_Elements.end(), e);
 
     if (it != m_Elements.end())
       m_Elements.erase(it);

gui/controller.hpp

@@ -2,20 +2,21 @@
 
 #include "display_list_cache.hpp"
 
+#include "../graphics/defines.hpp"
+
+#include "../geometry/point2d.hpp"
+
+#include "../base/strings_bundle.hpp"
+
 #include "../std/shared_ptr.hpp"
 #include "../std/unique_ptr.hpp"
 #include "../std/function.hpp"
 #include "../std/list.hpp"
 
-#include "../geometry/point2d.hpp"
-#include "../graphics/defines.hpp"
-
-#include "../base/strings_bundle.hpp"
 
 namespace graphics
 {
   class GlyphCache;
-  class OverlayElement;
   class Screen;
 }
 
@@ -47,16 +48,15 @@ namespace gui
 
   private:
 
-    /// element that has focus.
+    /// Element that has focus.
     shared_ptr<Element> m_focusedElement;
 
-    typedef list<shared_ptr<graphics::OverlayElement> > base_list_t;
-    typedef list<shared_ptr<Element> > elem_list_t;
+    typedef list<shared_ptr<Element> > ElemsT;
 
-    elem_list_t m_Elements;
+    ElemsT m_Elements;
 
-    /// select elements under specified point
-    void SelectElements(m2::PointD const & pt, elem_list_t & l, bool onlyVisible);
+    /// Select top element under specified point for tap processing.
+    shared_ptr<Element> SelectTopElement(m2::PointD const & pt, bool onlyVisible) const;
 
     /// Invalidate GUI function
     TInvalidateFn m_InvalidateFn;

gui/element.hpp

@@ -22,7 +22,6 @@ namespace gui
   class Element : public graphics::OverlayElement
   {
   public:
-
     enum EState
     {
       EInactive = 0,
@@ -32,18 +31,15 @@ namespace gui
     };
 
   protected:
-
     Controller * m_controller;
 
   private:
-
     EState m_state;
 
     mutable map<EState, graphics::FontDesc> m_fonts;
     mutable map<EState, graphics::Color> m_colors;
 
   public:
-
     typedef OverlayElement::Params Params;
 
     Element(Params const & p);
@@ -58,32 +54,36 @@ namespace gui
     graphics::Color const & color(EState state) const;
 
     /// Implement this method to handle single tap on the GUI element.
+    //@{
     virtual bool onTapStarted(m2::PointD const & pt);
     virtual bool onTapMoved(m2::PointD const & pt);
     virtual bool onTapEnded(m2::PointD const & pt);
     virtual bool onTapCancelled(m2::PointD const & pt);
+    //@}
 
-    /// invalidate the rendering system to redraw the gui elements.
+    /// Invalidate the rendering system to redraw the gui elements.
     void invalidate();
     /// obtain @see VisualScale
     double visualScale() const;
 
-    /// this method is called to cache visual appearance of gui::Element for fast rendering.
-    /// it should be called when isDirtyDrawing is set to true(visual parameters of object is changed).
+    /// This method is called to cache visual appearance of gui::Element for fast rendering.
+    /// It should be called after layout() is calculated properly.
     virtual void cache();
-    /// this method is called upon renderPolicy destruction and should clean
+    /// This method is called upon renderPolicy destruction and should clean
     /// all rendering-related resources, p.e. displayLists.
     virtual void purge();
-    /// this method is called in each frame and should be overriden if the
+    /// This method is called in each frame and should be overriden if the
     /// element wants to update it's internal state.
     virtual void update();
-    /// this method is called after gui::Controller::SetRenderParams to
+    /// This method is called after gui::Controller::SetRenderParams to
     /// perform layout calculations which might depends on RenderParams.
+    /// It should be called when isDirtyLayout is set to true (visual parameters of object is changed).
     virtual void layout();
-    /// set the parent controller for this element.
+    /// Set the parent controller for this element. Should be called for all inner Elemen's too.
     virtual void setController(Controller * controller);
 
-    /// check if the layout of element is dirty and re-layout element if needed.
+    /// Check if the layout of element is dirty and re-layout element if needed.
+    /// Used in a "lazy" layout/cache strategy (called before actual drawing).
     void checkDirtyLayout() const;
 
     /// @name Override from OverlayElement.

gui/gui_tests/gui_tests.cpp

@@ -5,8 +5,11 @@
 #include "../image_view.hpp"
 #include "../cached_text_view.hpp"
 
+#include "../../graphics/display_list.hpp"
+
 #include "../../map/country_status_display.hpp"
 
+
 struct ButtonTest
 {
   shared_ptr<gui::Button> m_button;

gui/image_view.cpp

@@ -4,9 +4,10 @@
 #include "../graphics/screen.hpp"
 #include "../graphics/display_list.hpp"
 
+#include "../geometry/transformations.hpp"
+
 #include "../base/matrix.hpp"
 
-#include "../geometry/transformations.hpp"
 
 namespace gui
 {
@@ -34,7 +35,7 @@ namespace gui
           math::Identity<double, 3>(),
           -(int)m_image.m_size.x / 2, -(int)m_image.m_size.y / 2);
 
-    uint32_t imageResID = cs->mapInfo(m_image);
+    uint32_t const imageResID = cs->mapInfo(m_image);
     cs->drawImage(m, imageResID, depth());
 
     cs->setDisplayList(0);
@@ -71,10 +72,10 @@ namespace gui
       r->drawDisplayList(m_displayList.get(), drawM * m);
     }
   }
+
   void ImageView::setImage(graphics::Image::Info const & info)
   {
     m_image = info;
     setIsDirtyLayout(true);
-    invalidate();
   }
 }

gui/image_view.hpp

@@ -3,13 +3,14 @@
 #include "element.hpp"
 
 #include "../graphics/image.hpp"
-#include "../graphics/display_list.hpp"
 
 #include "../std/unique_ptr.hpp"
 
+
 namespace graphics
 {
   class OverlayRenderer;
+  class DisplayList;
 }
 
 namespace gui
@@ -21,10 +22,6 @@ namespace gui
     unique_ptr<graphics::DisplayList> m_displayList;
 
   public:
-
-    void cache();
-    void purge();
-
     typedef Element BaseT;
 
     struct Params : public BaseT::Params
@@ -35,9 +32,16 @@ namespace gui
 
     ImageView(Params const & p);
 
+    void setImage(graphics::Image::Info const & info);
+
+    /// @name Override from graphics::OverlayElement and gui::Element.
+    //@{
     virtual m2::RectD GetBoundRect() const;
 
     void draw(graphics::OverlayRenderer * r, math::Matrix<double, 3, 3> const & m) const;
-    void setImage(graphics::Image::Info const & info);
+
+    void cache();
+    void purge();
+    //@}
   };
 }

gui/text_view.cpp

@@ -85,8 +85,6 @@ namespace gui
 
   void TextView::cache()
   {
-    layout();
-
     cacheBody(EActive);
     cacheBody(EPressed);
   }

map/compass_arrow.cpp

@@ -129,6 +129,7 @@ void CompassArrow::CreateAnim(double startAlfa, double endAlfa, double timeInter
 
   if (m_animTask)
     m_animTask->Cancel();
+
   m_animTask.reset(new AlfaAnimationTask(startAlfa, endAlfa, timeInterval, timeOffset, m_framework));
   m_animTask->AddCallback(anim::Task::EEnded, bind(&CompassArrow::AlfaAnimEnded, this, isVisibleAtEnd));
   m_framework->GetAnimController()->AddTask(m_animTask);
@@ -202,24 +203,14 @@ bool CompassArrow::onTapEnded(m2::PointD const & pt)
   anim::Controller * animController = m_framework->GetAnimController();
   anim::Controller::Guard guard(animController);
 
-  /// switching off compass follow mode
+  // switching off compass follow mode
   m_framework->GetInformationDisplay().locationState()->StopCompassFollowing();
-
-  double startAngle = m_framework->GetNavigator().Screen().GetAngle();
-  double endAngle = 0;
-
-  m_framework->GetAnimator().RotateScreen(startAngle, endAngle);
-
+  m_framework->GetAnimator().RotateScreen(m_framework->GetNavigator().Screen().GetAngle(), 0.0);
   m_framework->Invalidate();
 
   return true;
 }
 
-bool CompassArrow::roughHitTest(m2::PointD const & pt) const
-{
-  return hitTest(pt);
-}
-
 bool CompassArrow::hitTest(m2::PointD const & pt) const
 {
   Resource const * res = GetCompassResource();

map/compass_arrow.hpp

@@ -63,7 +63,6 @@ public:
 
   void draw(graphics::OverlayRenderer * r, math::Matrix<double, 3, 3> const & m) const;
   bool isVisible() const;
-  bool roughHitTest(m2::PointD const & pt) const;
   bool hitTest(m2::PointD const & pt) const;
 
   void cache();

map/country_status_display.cpp

@@ -134,6 +134,7 @@ void CountryStatusDisplay::CountryProgress(storage::TIndex const & idx, pair<int
   if ((m_countryIdx == idx) && (m_countryStatus == storage::EDownloading))
   {
     m_countryProgress = progress;
+
     setIsDirtyLayout(true);
     invalidate();
   }
@@ -152,9 +153,7 @@ CountryStatusDisplay::CountryStatusDisplay(Params const & p)
   bp.m_depth = depth();
   bp.m_minWidth = 200;
   bp.m_minHeight = 40;
-  bp.m_pivot = m2::PointD(0, 0);
   bp.m_position = EPosCenter;
-  bp.m_text = "Download";
 
   m_downloadButton.reset(new gui::Button(bp));
   m_downloadButton->setOnClickListener(bind(&CountryStatusDisplay::downloadCountry, this));
@@ -169,8 +168,6 @@ CountryStatusDisplay::CountryStatusDisplay(Params const & p)
 
   gui::TextView::Params tp;
   tp.m_depth = depth();
-  tp.m_pivot = m2::PointD(0, 0);
-  tp.m_text = "Downloading";
 
   m_statusMsg.reset(new gui::TextView(tp));
 
@@ -253,21 +250,19 @@ void CountryStatusDisplay::setCountryIndex(storage::TIndex const & idx)
 void CountryStatusDisplay::draw(graphics::OverlayRenderer *r,
                                 math::Matrix<double, 3, 3> const & m) const
 {
-  if (!isVisible())
-    return;
+  if (isVisible())
+  {
+    checkDirtyLayout();
 
-  checkDirtyLayout();
-
-  if (m_downloadButton->isVisible())
-    m_downloadButton->draw(r, m);
-  if (m_statusMsg->isVisible())
-    m_statusMsg->draw(r, m);
+    if (m_downloadButton->isVisible())
+      m_downloadButton->draw(r, m);
+    if (m_statusMsg->isVisible())
+      m_statusMsg->draw(r, m);
+  }
 }
 
 m2::RectD CountryStatusDisplay::GetBoundRect() const
 {
-  checkDirtyLayout();
-
   m2::RectD r(pivot(), pivot());
   if (m_downloadButton->isVisible())
     r.Add(m_downloadButton->GetBoundRect());
@@ -275,7 +270,7 @@ m2::RectD CountryStatusDisplay::GetBoundRect() const
   return r;
 }
 
-void CountryStatusDisplay::setController(gui::Controller *controller)
+void CountryStatusDisplay::setController(gui::Controller * controller)
 {
   Element::setController(controller);
   m_statusMsg->setController(controller);

map/country_status_display.hpp

@@ -36,6 +36,7 @@ private:
   unique_ptr<gui::Button> m_downloadButton;
   /// country status message
   unique_ptr<gui::TextView> m_statusMsg;
+
   /// current map name, "Province" part of the fullName
   string m_mapName;
   /// current map group name, "Country" part of the fullName

map/location_state.cpp

@@ -388,11 +388,6 @@ namespace location
     }
   }
 
-  bool State::roughHitTest(m2::PointD const & pt) const
-  {
-    return hitTest(pt);
-  }
-
   bool State::hitTest(m2::PointD const & pt) const
   {
     return false;

map/location_state.hpp

@@ -91,10 +91,6 @@ namespace location
     void cachePositionArrow();
     void cacheLocationMark();
 
-    void cache();
-    void purge();
-    void update();
-
     m2::RectD m_boundRect;
 
     void CheckCompassFollowing();
@@ -155,8 +151,11 @@ namespace location
     virtual m2::RectD GetBoundRect() const;
 
     void draw(graphics::OverlayRenderer * r, math::Matrix<double, 3, 3> const & m) const;
-    bool roughHitTest(m2::PointD const & pt) const;
     bool hitTest(m2::PointD const & pt) const;
+
+    void cache();
+    void purge();
+    void update();
     //@}
   };
 }