Model.hpp Doxygen Documentation #3958

Samir55 · 2017-05-17T00:29:12Z

No description provided.

alranel

Good work! Just a few notes.

alranel · 2017-05-20T17:25:42Z

xs/src/libslic3r/Model.hpp

    ModelObjectPtrs objects;
-
+    ///< Objects are owned by a model. Each model may have multiple instances


Each object may have multiple instances.

alranel · 2017-05-20T17:27:07Z

xs/src/libslic3r/Model.hpp

    ~Model();
+
+    /// Read a model from file.
+    /// This function supports the following formats (STL, OBJ, AMF).


It's worth documenting that this method auto-detects the file format by looking at the filename suffix. It's also useful to state that the file path supplied to this method must be expressed in UTF-8.

alranel · 2017-05-20T17:29:54Z

xs/src/libslic3r/Model.hpp

    void repair();
+
+    /// Center each model object instance around a point.


No! It centers the total bounding box of the instances, not each single instance. Otherwise they would placed in the same spot.

It's worth specifying that this only works in the XY plane and no transformation in Z is performed.

alranel · 2017-05-20T17:32:50Z

xs/src/libslic3r/Model.hpp

    void center_instances_around_point(const Pointf &point);
+
+    /// Center each ModelObject instance around the origin point.
    void align_instances_to_origin();


I realize this method is badly named (you described its behavior correctly) and you can remove it. Also remove it from Model.xsp please, otherwise it doesn't compile.

alranel · 2017-05-20T17:34:55Z

xs/src/libslic3r/Model.hpp

    void translate(coordf_t x, coordf_t y, coordf_t z);
+
+    /// Flatten all ModelObjects of the current model to a single mesh
+    /// after performing extra transformations (if the model was rotated or translated).


This has to be worded better. It flattens all ModelInstances to a single mesh. Beware of expressions like "extra transformations", as users who read this API docs might wonder what they are. They are instance transformations, so let's call them that way without introducing a new term.

alranel · 2017-05-20T18:11:07Z

xs/src/libslic3r/Model.hpp

    void scale_to_fit(const Sizef3 &size);
+
+    /// Rotate the current ModelObject.


Specify whether this rotates the volume meshes or just alters the instance attributes.

alranel · 2017-05-20T18:15:22Z

xs/src/libslic3r/Model.hpp

    void mirror(const Axis &axis);
+
+    /// Transform the current ModelObject by a certain ModelInstance attributes.


It's important to document that the inverse transformation is applied to all the object instances, so that the final size/position/rotation of the transformed objects doesn't change. This method is useful for flattening the instance transformations (of one instance) to the mesh.

alranel · 2017-05-20T18:16:13Z

xs/src/libslic3r/Model.hpp

    size_t facets_count() const;
+
+    /// Know whether there exists a TriangleMesh object that needs repair or not.


...that needed repair.

alranel · 2017-05-20T18:16:58Z

xs/src/libslic3r/Model.hpp

    bool needed_repair() const;
+
+    /// Cut (Slice) the current ModelObject at a certain axis at a certain magnitude.


Just English notes: along a certain axis at a certain coordinate.

alranel · 2017-05-20T18:18:32Z

xs/src/libslic3r/Model.hpp

+    /// Cut (Slice) the current ModelObject at a certain axis at a certain magnitude.
+    /// \param axis Axis the axis to slice at (X = 0 or Y or Z)
+    /// \param z coordf_t the point at the certain axis to cut(slice) the Model at
+    /// \param model the owner Model


This is the Model which will get the resulting objects added. It can be the same model as the input one, or another one.

lordofhyphens · 2017-05-21T03:48:31Z

As a FYI, Markdown syntax is supported with Doxygen. Don't feel as if you have to use it.

Put back align_instances_to_origin. Not sure why this was removed to start with.

put back removed function (this should be a doc-only change)

Re-added function that should have not been removed in a doc-only change (re: scope)

lordofhyphens

I resolved my own comments.

lordofhyphens · 2017-05-17T00:36:37Z

xs/src/libslic3r/Model.hpp

    ModelMaterial* add_material(t_model_material_id material_id, const ModelMaterial &other);
+
+    /// Get theModelMaterial having a certain material id.


Typo: Need a space between "the" and "ModelMaterial"

lordofhyphens · 2017-06-04T03:17:43Z

xs/src/libslic3r/Model.cpp

-    this->center_instances_around_point(new_center);
-}
-
-void


So where did this go?

@alexrj told me to remove it from Model.

Changes were made to address review.

Samir55 and others added 8 commits May 12, 2017 04:13

Documenting Model Class (Still un finished functions)

91d557b

Add Model Material Documentation

f87eb91

Documenting ModelInstance

736453c

Documenting ModelVolume

7e3b13a

Documenting the ModelObject Class

4e1fa25

Applying some enhancement to Model.hpp Documentation

3d8abef

Removed some todos in the code

34ebca1

Remove Todos in Model.hpp

efc0d72

alranel previously requested changes May 20, 2017

View reviewed changes

lordofhyphens added the Code Cleanup/Refactoring This is a cleanup/refactor effort that should not affect functionality. label May 21, 2017

lordofhyphens added this to the GSOC 2017 milestone May 21, 2017

Samir55 and others added 6 commits May 24, 2017 15:57

Fix some functions dcumentation.

bf0f84d

Removed todo found in Model.hpp

180c1e8

Update Model.hpp

5894d06

Update Model.cpp

0bb6870

Put back align_instances_to_origin. Not sure why this was removed to start with.

Update Model.xsp

99eb6c3

put back removed function (this should be a doc-only change)

Update Model.hpp

9002405

Re-added function that should have not been removed in a doc-only change (re: scope)

lordofhyphens approved these changes Jun 16, 2017

View reviewed changes

lordofhyphens merged commit 1e8cd3f into slic3r:master Jun 16, 2017

Samir55 mentioned this pull request Aug 21, 2017

[GSoC 2017] Support 3MF Format Read/Write. #4046

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Model.hpp Doxygen Documentation #3958

Model.hpp Doxygen Documentation #3958

Samir55 commented May 17, 2017

alranel left a comment

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

alranel May 20, 2017

lordofhyphens commented May 21, 2017

lordofhyphens left a comment

lordofhyphens May 17, 2017

lordofhyphens Jun 4, 2017

Samir55 Jun 16, 2017

		ModelObjectPtrs objects;

		///< Objects are owned by a model. Each model may have multiple instances

		void repair();

		/// Center each model object instance around a point.

		void scale_to_fit(const Sizef3 &size);

		/// Rotate the current ModelObject.

		void mirror(const Axis &axis);

		/// Transform the current ModelObject by a certain ModelInstance attributes.

		size_t facets_count() const;

		/// Know whether there exists a TriangleMesh object that needs repair or not.

		bool needed_repair() const;

		/// Cut (Slice) the current ModelObject at a certain axis at a certain magnitude.

		ModelMaterial* add_material(t_model_material_id material_id, const ModelMaterial &other);

		/// Get theModelMaterial having a certain material id.

Model.hpp Doxygen Documentation #3958

Model.hpp Doxygen Documentation #3958

Conversation

Samir55 commented May 17, 2017

alranel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lordofhyphens commented May 21, 2017

lordofhyphens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment