Commit: acf50b21f29b5797344cbf61cda5843df7e37173
Author: Hugo Sales
Date:   Thu Jun 6 14:25:09 2019 +0100
Branches: soc-2019-fast-io
https://developer.blender.org/rBacf50b21f29b5797344cbf61cda5843df7e37173

[Fast import/export] Review and documentation, and bug fixes

===================================================================

M       source/blender/editors/io/intern/common.hpp
M       source/blender/editors/io/intern/obj.cpp

===================================================================

diff --git a/source/blender/editors/io/intern/common.hpp 
b/source/blender/editors/io/intern/common.hpp
index dd8219588b9..99d22e8d86e 100644
--- a/source/blender/editors/io/intern/common.hpp
+++ b/source/blender/editors/io/intern/common.hpp
@@ -91,6 +91,9 @@ namespace common {
        // --- TEMPLATES ---
 
        // Adapt a pointer-size pair as a random access iterator
+       // This makes use of `boost::iterator_facade` and makes it possible to 
use
+       // for each style loops, as well as cleanly hiding how the underlying 
Blender
+       // data structures are accessed
        template<typename SourceT, typename Tag = 
std::random_access_iterator_tag>
        struct pointer_iterator :
                public boost::iterator_facade<pointer_iterator<SourceT, Tag>, 
SourceT &, Tag> {
@@ -101,9 +104,9 @@ namespace common {
                explicit pointer_iterator(SourceT *p, size_t size) : it(p), 
first(p), size(size) {}
                operator SourceT *() const { return it; }
                pointer_iterator & operator=(const pointer_iterator<SourceT, 
Tag> &p) {
-                       this->it = p.it;
-                       this->first = p.first;
-                       this->size = p.size;
+                       // Placement new: construct a new object in the 
position of `this`
+                       // Doesn't actually allocate memory
+                       new (this) pointer_iterator(p);
                        return *this;
                }
                // pointer_iterator & operator=(pointer_iterator<SourceT, Tag> 
&&p) = default;//  {
@@ -121,11 +124,15 @@ namespace common {
                bool equal(const pointer_iterator &other) const { return it == 
other.it; }
                SourceT & dereference() const { return *this->it; }
                SourceT *it;
-               SourceT *first;
+               SourceT * const first;
                size_t size;
        };
 
-       // TODO soemeone Document CTRP
+       // This is another iterator, whcih makes use of 
`boost::iterator_adaptor` to change  how the underlying
+       // iterator behaves. Specifically, it uses the derived class' 
`dereference` method.
+       // This makes use of CRTP, the curiously recurring template pattern
+       // This means that the base class has, as a template paramater, the 
type of the derived class
+       // which means that it can hold a pointer to said derived class and 
call it's `dereference` method.
        template<typename SourceT, typename ResT, typename CRTP,
                 typename Base = pointer_iterator<SourceT>,
                 typename Tag = typename 
std::iterator_traits<Base>::iterator_category>
@@ -145,25 +152,25 @@ namespace common {
                        : dereference_iterator(Base{p, (size_t) size}, crtp) {}
                explicit dereference_iterator(SourceT *p, CRTP *crtp) // For 
list_iterator
                        : dereference_iterator(Base{p}, crtp) {}
-               dereference_iterator       begin()  const { return 
{this->base().begin(), crtp}; }
+               dereference_iterator       begin()  const { return 
{this->base().begin(),  crtp}; }
                const dereference_iterator cbegin() const { return 
{this->base().cbegin(), crtp}; }
-               dereference_iterator       end()    const { return 
{this->base().end(), crtp}; }
-               const dereference_iterator cend()   const { return 
{this->base().cend(), crtp}; }
+               dereference_iterator       end()    const { return 
{this->base().end(),    crtp}; }
+               const dereference_iterator cend()   const { return 
{this->base().cend(),   crtp}; }
                friend class boost::iterator_core_access;
                ResT dereference() const { return 
crtp->dereference(this->base()); }
                CRTP *crtp;
        };
 
+       // Another iterator that iterates over doubly linked lists
        template<typename SourceT, typename ResT = SourceT &,
                 typename Tag = std::bidirectional_iterator_tag>
        struct list_iterator : public 
boost::iterator_adaptor<list_iterator<SourceT, ResT, Tag>,
                                                              
pointer_iterator<SourceT, Tag>, ResT, Tag, ResT> {
                list_iterator() : list_iterator::iterator_adaptor_() {}
-               list_iterator(const pointer_iterator<SourceT, Tag> & other)
+               list_iterator(const pointer_iterator<SourceT, Tag> &other)
                        : list_iterator::iterator_adaptor_(other), 
first(other.first) {}
                explicit list_iterator(SourceT *first)
-                       : 
list_iterator::iterator_adaptor_(pointer_iterator<SourceT, Tag>{first}),
-                         first(first) {}
+                       : 
list_iterator::iterator_adaptor_(pointer_iterator<SourceT, Tag>{first}), 
first(first) {}
                list_iterator       begin()  const { return 
list_iterator{first}; }
                list_iterator       end()    const { return 
list_iterator{nullptr}; }
                const list_iterator cbegin() const { return 
list_iterator{first}; }
@@ -175,16 +182,19 @@ namespace common {
                SourceT * const first;
        };
 
+       // Iterator over the polygons of a mesh
        struct poly_iter : pointer_iterator<MPoly> {
                poly_iter(const Mesh * const m) : pointer_iterator(m->mpoly, 
m->totpoly) {}
                poly_iter(const pointer_iterator &p) : pointer_iterator(p) {}
                poly_iter(pointer_iterator &&p) : 
pointer_iterator(std::move(p)) {}
        };
 
+       // Iterator over the vertices of a mesh
        struct vert_iter : pointer_iterator<MVert> {
                vert_iter(const Mesh * const m) : pointer_iterator(m->mvert, 
m->totvert) {}
        };
 
+       // Iterator over the vertices of a polygon
        struct vert_of_poly_iter : public dereference_iterator<MLoop, MVert &, 
vert_of_poly_iter> {
                // TODO someone What order are the vertices stored in? 
Clockwise?
                explicit vert_of_poly_iter(const Mesh * const mesh, const MPoly 
&mp)
@@ -193,6 +203,7 @@ namespace common {
                const Mesh * const mesh;
        };
 
+       // Iterator over the UVs of a mesh (as `const std::array<float, 2>`)
        struct uv_iter : public dereference_iterator<MLoopUV, const 
std::array<float, 2>, uv_iter> {
                explicit uv_iter(const Mesh * const m)
                        : dereference_iterator(m->mloopuv, m->totloop, this) {}
@@ -205,12 +216,14 @@ namespace common {
                }
        };
 
+       // Iterator over all the edges of a mesh
        struct edge_iter : pointer_iterator<MEdge> {
                edge_iter(const Mesh * const m) : pointer_iterator(m->medge, 
m->totedge) {}
                edge_iter(const pointer_iterator<MEdge> &pi) : 
pointer_iterator(pi) {}
                edge_iter(pointer_iterator<MEdge> &&pi) : pointer_iterator(pi) 
{}
        };
 
+       // Iterator over the edges of a mesh whcih are marked as loose
        struct loose_edge_iter : public 
boost::iterator_adaptor<loose_edge_iter, edge_iter,
                                                                MEdge, 
std::bidirectional_iterator_tag> {
                explicit loose_edge_iter(const Mesh * const m, const edge_iter 
&e)
@@ -231,6 +244,7 @@ namespace common {
                const Mesh * const mesh;
        };
 
+       // Iterator over all the objects in a `ViewLayer`
        // TODO someone G.is_break
        struct object_iter : dereference_iterator<Base, Object *, object_iter, 
list_iterator<Base>> {
                explicit object_iter(const ViewLayer * const vl)
@@ -238,18 +252,29 @@ namespace common {
                inline static Object * dereference(const list_iterator<Base> 
&b) { return b->object; }
        };
 
+       // Iterator over the modifiers of an `Object`
        struct modifier_iter : list_iterator<ModifierData> {
                explicit modifier_iter(const Object * const ob)
                        : list_iterator((ModifierData *) ob->modifiers.first) {}
        };
 
+       // Iterator over the `MLoop` if a `MPoly` of a mesh
        struct loop_of_poly_iter : pointer_iterator<MLoop> {
                explicit loop_of_poly_iter(const Mesh * const mesh, const 
poly_iter &poly)
                        : pointer_iterator(mesh->mloop + poly->loopstart, 
poly->totloop - 1) {}
+               explicit loop_of_poly_iter(const Mesh * const mesh, const MPoly 
&poly)
+                       : pointer_iterator(mesh->mloop + poly.loopstart, 
poly.totloop - 1) {}
                loop_of_poly_iter(const pointer_iterator &p) : 
pointer_iterator(p) {}
                loop_of_poly_iter(pointer_iterator &&p) : 
pointer_iterator(std::move(p)) {}
        };
 
+       // Iterator over the normals of mesh
+       // This is a more complex one, because there are three possible sources 
for the normals of a mesh:
+       //   - Custom normals
+       //   - Face normals, when the face is not smooth shaded
+       //   - Vertex normals, when the face is smooth shaded
+       // This is a completely separate iterator because it needs to override 
pretty much all behaviours
+       // It's only a bidirectional iterator, because it is not continuous
        struct normal_iter :
                public boost::iterator_facade<normal_iter, std::array<float, 3>,
                                              std::bidirectional_iterator_tag, 
const std::array<float, 3>> {
@@ -259,8 +284,7 @@ namespace common {
                normal_iter(normal_iter &&) = default;
                explicit normal_iter(const Mesh * const mesh, const poly_iter 
poly, const loop_of_poly_iter loop)
                        : mesh(mesh), poly(poly), loop(loop) {
-                       loop_no = 
static_cast<float(*)[3]>(CustomData_get_layer(&mesh->ldata, CD_NORMAL));
-                       // ++this->loop;
+                       custom_no = 
static_cast<float(*)[3]>(CustomData_get_layer(&mesh->ldata, CD_NORMAL));
                }
                explicit normal_iter(const Mesh * const mesh)
                        : normal_iter(mesh, poly_iter(mesh), 
loop_of_poly_iter(mesh, poly_iter(mesh))) {}
@@ -268,58 +292,48 @@ namespace common {
                normal_iter end()   const { return normal_iter(mesh, 
poly.end(), loop.end()); }
                friend class boost::iterator_core_access;
                void increment() {
-                       // if (loop != loop.end()) {
-                       //      if (poly->flag & ME_SMOOTH)
-                       //              ++loop;
-                       // } else {
-                       //      loop = loop_of_poly_iter{mesh, poly};
-                       // }
-
-                       // if (poly != poly.end()) {
-                       //      ++poly;
-                       //      loop = loop_of_poly_iter{mesh, poly};
-
-                       // } else {
-                       //      loop = loop.end();
-                       // }
-                       // If not in the last of the loop and face is smooth 
shaded, each vertex has it's
-                       // own normal, so increment loop, otherwise go to the 
next poly
-                       //  && (loop_no != nullptr || (poly->flag & ME_SMOOTH) 
!= 0)
+                       // If we're not at the end of the loop, we increment it
                        if (loop != loop.end()) {
                                ++loop;
                        } else if (poly != poly.end()) {
+                               // if we're not at the end of the poly, we 
increment it
                                ++poly;
                                if (poly != poly.end())
+                                       // if incrementing the poly didn't put 
us past the end,
+                                       // use the new poly to generate the new 
loop iterator
                                        loop = loop_of_poly_iter{mesh, poly};
                        }
-                       //  else {
-                       //      loop = loop.end();
-                       //      poly = poly.end();
-                       // }
                }
                void decrement() {
-                       if ((loop != loop.begin()) && ((poly->flag & ME_SMOOTH) 
!= 0)) {
+                       if (loop != loop.begin()) {
                                --loop;
                        } else if (poly != poly.begin()) {
                                --poly;
-                               loop = loop_of_poly_iter(mesh, poly);
-                       } else {
-                               loop = loop.end();
-                               poly = poly.end();
+                               loop = loop_of_poly_iter{mesh, poly};
                        }
                }
                bool equal(const normal_iter &other) const {
-                       return poly 

@@ Diff output truncated at 10240 characters. @@

_______________________________________________
Bf-blender-cvs mailing list
[email protected]
https://lists.blender.org/mailman/listinfo/bf-blender-cvs

Reply via email to