On Sun, Jan 3, 2016 at 10:11 AM, Adam Wulkiewicz wrote:

- The identifiers should be made more verbose. This is true both for function names as well as identifers containing "q"/"v"/"m" parts. The former should be replaced with full words, e.g. "transp" -> "transpose". The latter should be replaced at least with "qua"/"quat"/"vec"/"mat" respectively, so e.g. "v_traits" -> "vector_traits"/"vec_traits", "col_m" -> "col_mat", etc.

Does this include header file names? If you look in the boost/qvm directory, you'll see that currently the q, v and m prefix naming convention is followed for file names as well (q and v prefixes by analogy): m.hpp: any size matrix operations m2.hpp: 2D matrix operations m3.hpp: 3D matrix operations m4.hpp: 4D matrix operations m_access.hpp: matrix element access functions m_traits.hpp: m_traits template definition m_traits_array.hpp: m_traits specialization for C arrays Also: qv.hpp: operations between quaternions and vectors vm.hpp: operations between vectors and matrices map_mm.hpp: matrix-to-matrix view proxies map_mv.hpp: matrix-to-vector view proxies map_vm.hpp: vector-to-matrix view proxies Thanks, Emil

I completed the refactoring based on the reviewers' feedback, except for providing function-style syntax for swizzling, which requires more thought. Note that this refactoring breaks almost any existing code that uses Boost QVM, since a lot of functions now use more verbose names. See http://zajo.github.io/boost-qvm/. Thanks! Emil On Sun, Jan 3, 2016 at 10:11 AM, Adam Wulkiewicz wrote:

Dear Boost community,

The review period for Emil Dotchevski's QVM library has concluded. I'd like to thank everyone who participated in the discussions, esspecially those who sent formal reviews.

Only four votes were casted: - 1 vote for acceptance * Rajaditya Mukherjee - 2 votes for conditional acceptance * Rainer Deyke * Oswin Krause - 1 vote for rejection * Phil Endecott

However more people commented on issues on the mailing list. Comments received during the course of the review were generally positive, with a number of technical issues identified that need to be addressed. The review identified no fundamental problems in the design or implementation of the library that would entail rejection. The final version of the library can be fully accepted once Emil has addressed the issues identified during the review. Therefore I'm happy to say:

Emil Dotchevski's Boost.QVM library is CONDITIONALLY ACCEPTED.

Below is a summary of technical concerns:

- The identifiers should be made more verbose. This is true both for function names as well as identifers containing "q"/"v"/"m" parts. The former should be replaced with full words, e.g. "transp" -> "transpose". The latter should be replaced at least with "qua"/"quat"/"vec"/"mat" respectively, so e.g. "v_traits" -> "vector_traits"/"vec_traits", "col_m" -> "col_mat", etc.

- Many participants found the swizzling syntax using comma operator confusing or not good enough. There were many suggestions about the possible replacements: * v%XY - the old operator% syntax * (v,XY) - the comma operator syntax currently used by QVM * (v,asXY) - in order to limit potential conflicts with macros * XY(v) - function syntax, MOST PREFERED * v.XY() - probably the best but currently not possible (n4165, n4174) * ref(v).XY() * ref(v)[xy] * swizzle(v, xy) * ref(v)->xy The function syntax, i.e. XY(v), was the most prefered solution. Some of the participants appreciated the comma operator as well because of the similarity with shader languages syntax. The library could provide a few alternatives that could be picked by the user, e.g. both a comma and function syntax.

- There were concerns regarding deduce_xx set of traits, in particular that traits taking two arguments (deduce_q2, deduce_v2 and deduce_m2) can cause ODR issues in a case when e.g. third-party libraries depending on QVM specialized these traits differently and those libraries were used together. There were no consensus if this is indeed a library issue or how this issue could be handled in general. Taking into account that the specialization of these traits is optional it'd be sufficient to describe the issue in the documentation and discourage the users from specializing those traits for types defined outside of their own code.

- There was also a suggestion to use the same names that are already being used in other well-known libraries to make the interface more intuitive, e.g. mag() could be replaced by norm() (used in Eigen and Armadillo libraries), mag2() could be replaced by norm_squared() or norm_sqr(), etc. This would also make the latter more verbose. I'd let the decission how to deal with it to Emil.

- Another suggestion was to replace read/write semantics with get/set semantics or at least to make the r/w/ir/iw identifiers more verbose. Here, I'd let the decission how to deal with it to Emil as well.

- Aside from the issues described above, there were also suggestions that the scope of QVM library could/should be broadened to include functionalities from domains other than computer graphics, e.g. linear algebra, geometry, physics, robotics, etc. Oswin Krause requested that an consensus on the scope of the library should be made before the acceptance of the library. Phil Endecott expressed that the library is too small to be a standalone library. However as a Boost.Geometry developer I may say that the scopes, design rationales, internal structures, etc. of both libraries are different. AFAIK it's also true for Boost.Polygon. Furthermore, if my understanding is correct after reading various discussions the community finds the library useful even in the current state, some of the participants already used the library in their projects. Furthermore I'm sure that future users of QVM will suggest additions to the QVM library if they think it's necessary, so the scope will be broadened in a natural way. Though of course I encourage Emil to start a discussion with the community regarding this issue.

- Regarding the documentation, various people suggested the following improvements: * it should start with a "Quick Start" page showing how to use the QVM built-in types to do some high-school linear algebra, * make it more obvious which functions are classified as "operations" and which are "view proxies", * contain more examples of every function as well as some bigger real-life examples.

Please let me know if you think something is wrong.

Regards, Adam

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost