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

10 files changed, 69 insertions, 73 deletions

diff --git a/gui/button.cpp b/gui/button.cpp
index 6e00af4610..0a07590aa7 100644
--- a/gui/button.cpp
+++ b/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;
-
-    checkDirtyLayout();
+    if (isVisible())
+    {
+      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)
diff --git a/gui/cached_text_view.cpp b/gui/cached_text_view.cpp
index da0b330f35..20433c2654 100644
--- a/gui/cached_text_view.cpp
+++ b/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(),
diff --git a/gui/cached_text_view.hpp b/gui/cached_text_view.hpp
index 2b35a7df8c..35feea0eec 100644
--- a/gui/cached_text_view.hpp
+++ b/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);
+    //@}
   };
 }
diff --git a/gui/controller.cpp b/gui/controller.cpp
index 8e2d3d6cfc..4a5b7ae2e7 100644
--- a/gui/controller.cpp
+++ b/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);
diff --git a/gui/controller.hpp b/gui/controller.hpp
index 6b9e2a54e9..ca370bb661 100644
--- a/gui/controller.hpp
+++ b/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;
diff --git a/gui/element.hpp b/gui/element.hpp
index e24e0c620f..decbd99c17 100644
--- a/gui/element.hpp
+++ b/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.
diff --git a/gui/gui_tests/gui_tests.cpp b/gui/gui_tests/gui_tests.cpp
index 8f10444e53..089ad19954 100644
--- a/gui/gui_tests/gui_tests.cpp
+++ b/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;
diff --git a/gui/image_view.cpp b/gui/image_view.cpp
index 142157d55d..824bba70c4 100644
--- a/gui/image_view.cpp
+++ b/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();
   }
 }
diff --git a/gui/image_view.hpp b/gui/image_view.hpp
index 97db3fbd56..86419070f7 100644
--- a/gui/image_view.hpp
+++ b/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();
+    //@}
   };
 }
diff --git a/gui/text_view.cpp b/gui/text_view.cpp
index 544ba795d6..c29224c43d 100644
--- a/gui/text_view.cpp
+++ b/gui/text_view.cpp
@@ -85,8 +85,6 @@ namespace gui
 
   void TextView::cache()
   {
-    layout();
-
     cacheBody(EActive);
     cacheBody(EPressed);
   }