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
