Merge lp:~3v1n0/nux/callgrind-improvements into lp:nux

[Marco Trevisan (Treviño)]

WindowThread: return a const& in GetDrawList

This will reduces a lot the copies that unity has to do in very-often called functions

Returning a const& we have practically no cost when running this in unity, see this pre/after comparison:

http://ubuntuone.com/3ukWhqzPdcBreLxTNayzBv

[lp:~3v1n0/nux/callgrind-improvements]

778. By Marco Trevisan (Treviño) on 2013-04-10

configure.ac: increase ABI version

[Michi Henning (michihenning)]

Geometry const& geo = view->GetAbsoluteGeometry();
parent = view->GetToplevel();

As far as I can see, if this actually works, it works only by accident. This binds a C++ reference to the return value of a function. The return value is a temporary that will disappear the instant the function completes.

Further down in the function, we have:

m_dirty_areas.push_back(geo);

This looks like it pushes a dangling reference.

review: Needs Fixing

[Andrea Azzarone (azzar1)]

> Geometry const& geo = view->GetAbsoluteGeometry();
> parent = view->GetToplevel();
>
> As far as I can see, if this actually works, it works only by accident. This
> binds a C++ reference to the return value of a function. The return value is a
> temporary that will disappear the instant the function completes.
>

Are you sure? The standards says:

«...
C obj1 ;
const C & cr = C (16)+ C (23);
C obj2 ;

the expression C(16)+C(23) creates three temporaries. A first temporary T1 to hold the result of the expression C(16), a second temporary T2 to hold the result of the expression C(23), and a third temporary T3 to hold the result of the addition of these two expressions. The temporary T3 is then bound to the reference cr. It is unspecified whether T1 or T2 is created first. On an implementation where T1 is created before T2, it is guaranteed that T2 is destroyed before T1.
The temporaries T1 and T2 are bound to the reference parameters of operator+; these temporaries are destroyed at the
end of the full expression containing the call to operator+. **** The temporary T3 bound to the reference cr is destroyed at the end of cr’s lifetime, that is, at the end of the program... *** »

> Further down in the function, we have:
>
> m_dirty_areas.push_back(geo);
>
> This looks like it pushes a dangling reference.

[Andrea Azzarone (azzar1)]

*standard

[Michi Henning (michihenning)]

Oops, me bad, you are right. The const makes it legal.

review: Approve

[Andrea Azzarone (azzar1)]

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

Il giorno 11/apr/2013 03:19, "Michi Henning" <email address hidden>
ha scritto:
>
> Review: Approve
>
> Oops, me bad, you are right. The const makes it legal.

Yep, in any case the move ctor that rect now has would make this implicit.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'Nux/WindowThread.cpp'
--- Nux/WindowThread.cpp	2013-02-01 09:14:07 +0000
+++ Nux/WindowThread.cpp	2013-04-10 22:45:26 +0000
@@ -1352,11 +1352,10 @@
   void WindowThread::AddToDrawList(View *view)
   {
     Area *parent;
-    Geometry geo, pgeo;
-    
-    geo = view->GetAbsoluteGeometry();
+
+    Geometry const& geo = view->GetAbsoluteGeometry();
     parent = view->GetToplevel();
-    
+
     if (parent && (view != parent))
     {
 //       pgeo = parent->GetGeometry();
@@ -1385,7 +1384,7 @@
     m_dirty_areas.clear();
   }
   
-  std::vector<Geometry> WindowThread::GetDrawList()
+  std::vector<Geometry> const& WindowThread::GetDrawList() const
   {
     return m_dirty_areas;
   }

=== modified file 'Nux/WindowThread.h'
--- Nux/WindowThread.h	2013-02-01 09:14:07 +0000
+++ Nux/WindowThread.h	2013-04-10 22:45:26 +0000
@@ -323,7 +323,7 @@
 
     void ClearDrawList();
 
-    std::vector<Geometry> GetDrawList();
+    std::vector<Geometry> const& GetDrawList() const;
 
 #ifdef NUX_GESTURES_SUPPORT
     /*!

=== modified file 'configure.ac'
--- configure.ac	2013-04-10 22:45:26 +0000
+++ configure.ac	2013-04-10 22:45:26 +0000
@@ -23,7 +23,7 @@
 # e.g.: december 5th, 2011 is: 20111205
 # To make more than one API change in a day, add a number to the date. Like 20111205.xx
 
-m4_define([nux_abi_version], [20130408.1])
+m4_define([nux_abi_version], [20130411.0])
 
 m4_define([nux_version],
           [nux_major_version.nux_minor_version.nux_micro_version])

=== modified file 'debian/changelog'
--- debian/changelog	2013-03-25 04:02:27 +0000
+++ debian/changelog	2013-04-10 22:45:26 +0000
@@ -1,3 +1,9 @@
+nux (4.0.1-0ubuntu1) UNRELEASED; urgency=low
+
+  * Buming version to 4.0.1 as per recent ABI changes
+
+ -- Marco Trevisan (Treviño) <[e-mail]>  Mon, 08 Apr 2013 17:21:37 +0200
+
 nux (4.0.0daily13.03.25-0ubuntu1) raring; urgency=low
 
   [ Brandon Schaefer ]