Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lambda modifier mesh #3590

Merged
merged 19 commits into from Dec 18, 2016
Merged

Lambda modifier mesh #3590

merged 19 commits into from Dec 18, 2016

Conversation

@lordofhyphens
Copy link
Member

@lordofhyphens lordofhyphens commented Nov 27, 2016

Two related items. One is the ability to slide modifier meshes around in the Settings dialog, the other is the ability to make generic/anonymous (or lambda) meshes available to the user. Currently there's just a box available. Anything from Slic3r::Test is available for inclusion through appending them to the ComboBox list.

UI needs some more work for it (porting to use the UI builder code).

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

Revised the menu code to be nicer to look at (and largely use OptionGroup). I'm unsure how to get a combobox that isn't driven by the XS config going, couldn't find any Perl examples in the current codebase.

@alexrj if you remember how it'd be great if you could whip up a snippet I could learn from.

Loading

@alranel
Copy link
Member

@alranel alranel commented Nov 27, 2016

Hey, I do really appreciate this effort - that's something needed.
But I don't think we should use Slic3r::Test at all. Those are really meant just for the test suite... What about a simple button "Add Cube Modifier" which prompts for parallelepiped sizes (can be three Wx::GetNumberFromUser dialogs)? I think that's the only useful thing for starters. What do you think?

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

Works for me, I used Slic3r::Test because it was there and I didn't feel like duplicating code. I can pull that out either into its own package or otherwise. I think it could go into its own package primarily to facilitate a few other simple solids (sphere, cylinder) getting added in.

I made a point of committing the movement code separately from the generics code if you want to grab just that via cherry-pick.

Loading

@bubnikv
Copy link
Contributor

@bubnikv bubnikv commented Nov 27, 2016

I love the idea, though it is not yet refined enough for my taste.

The LambdaObjectDialog crashes on Windows just at ->new if it is missing in the list at the start of GUI.pm The same applies to Test. I agree with @alexrj, the Test should not be loaded into the application.

Regarding the mesh creation, I think it would best be part of the XS module, object TriangleMesh. I would love something like Slic3r::TriangleMesh->new_box, new_sphere etc. I can prepare that.

Also the modifier mesh should not be moved to @{$self->{model_object}->origin_translation}
as it shares the coordinate system of the main object.

Also I believe the shift sliders shall only be active for the modifier meshes, not for the main mesh. Ideally, these sliders would be hidden if no modifier mesh is selected.

I would love a "slab" modifier mesh, which would shrink in XY to the size of the slab. I would also love a sphere and a cylinder.

Loading

@bubnikv
Copy link
Contributor

@bubnikv bubnikv commented Nov 27, 2016

One more thing. IMHO there should be a new set of transformation attributes (transformation, scale, rotation) stored for the modifier mesh, so when you open the dialog, you could reset the transformation by filling in the zeros. Your implementation just moves the coordinates of the mesh. When implementing the transformation attributes of the modifier meshes, these need to be stored into the AMF file and restored from the AMF file to be really useful.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

I'm not sure about putting it in TriangleMesh myself, although the basic code would probably be cribbable from ReadFromPerl in the xsp if you wanted to push it into XS.

Perhaps a specialized Model instead?

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

I'm not trying to implement scale or rotation. I agree that being able to store/recall the modifier mesh setup into formats that support it is useful, but the rest of it isn't quite there yet.

I'm more in favor of getting something more limited like this in while leaving it flexible enough to support more work later.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

@bubnikv Unless you were talking about putting make_cube and such into the xsp interface and not libslic3r's TriangleMesh.cpp?

Loading

@bubnikv
Copy link
Contributor

@bubnikv bubnikv commented Nov 27, 2016

Unless you were talking about putting make_cube and such into the xsp interface and not libslic3r's TriangleMesh.cpp?

That is really a matter of taste. A free function at TriangleMesh.cpp/hpp or a static function of TriangleMesh class would be possible. Then I would bind the static or free functions to the TriangleMesh XS object.

Loading

@alranel
Copy link
Member

@alranel alranel commented Nov 27, 2016

I agree with @bubnikv about creating TriangleMesh constructors for specific geometric shapes. Why don't you like that @lordofhyphens?

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

@alexrj I mostly was trying to avoid non-specific "kitchen sink" functions, aka functions that do one thing and then sit around in the interface for all time (see: PHP). It's mostly an aesthetics thing to me, but if we made some specific constructors for it I'd be alright with it.

One thing I'd like to see in a constructor is the ability to pass in a STL vector of STL tuples for the facets and vertices as a generic, from there we can have static functions that make/return the simple shapes and then bind them to the interface via the XSP or w/e. If someone's calling it through libslic3r that isn't us then I think that would be cleaner and a bit easier to use.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

Actually, we don't need tuples, we have Point3 and Pointf3 for facet IDs and positions, currently putting together a TriangleMesh constructor that takes a std::vector of Pointf3 and Point3 for the vertices and facets and generates a mesh from that.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

While I'm in here, is there any objection to overloading [] for Point (and their respective descendent classes) to refer to x/y/z numerically? Or maybe something we can use with std::get<>?

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 27, 2016

There's the new constructor and make_cube utility function.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Nov 28, 2016

Cylinder mesh has been added (via make_cylinder in TriangleMesh.cpp and exposed as a clone function in the XSP).

I've updated the generation UI as well; do we want to be able to generate these anonymous objects on the main plater as well? Seems like it'd be handy for a purge/prime tower (at least for making the tower).

Loading

bubnikv
Copy link
Contributor

bubnikv commented on 2171d6a Nov 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new methods cube & cylinder in xs/xsp/TriangleMesh.xsp should somehow be made static, so they could be called on a Perl package not on an object instance. Otherwise great, thanks!

Loading

lordofhyphens
Copy link
Member

lordofhyphens commented on 2171d6a Nov 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I just don't know how to wield that particular black magic yet.

I have the beginnings of a proper sphere as well, but it's being derpy.

Loading

bubnikv
Copy link
Contributor

bubnikv commented on 2171d6a Nov 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loading

lordofhyphens
Copy link
Member

lordofhyphens commented on 2171d6a Nov 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My kingdom for an icosahedron :)

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Dec 6, 2016

I'll check to see if I can work out the canvas size generated for the parts dialog, in which case slab will be fairly straightforward (take a Z from the dialog, etc).

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Dec 6, 2016

Behold, for it is a sphere. I defaulted the step angle to 1 degree (actually 2pi / 360).

Rotation (and preserving it) only makes a lot of sense if you expect to be able to (ab)use a file format to save the result in a way that only Slic3r knows how to replicate. I think that instead performing any translation/rotation directly and exporting is a better idea as it lets the user pull the mesh out with the object and manipulate it in an exterior program without breaking something.

I know that @bubnikv (or someone else he works with) is working on extending the AMF reader and making it suitable for a project file through special comments.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Dec 6, 2016

Slab is done as a UI function, the same elements get the bounding box for model_object and call TriangleMesh::Cube() with those attributes and then shifts it so that it is centered properly.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Dec 6, 2016

I wasn't calling repair() on the produced mesh, nor was I marking it as fixed when I generated it, hence the call to repair(). Not doing so makes vertices() very unhappy.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Dec 15, 2016

I feel this is feature complete at this point, if someone else wants to do a PR later to add more functionality fine.

Loading

@alranel
Copy link
Member

@alranel alranel commented Dec 18, 2016

Yay! I'm going to merge this.

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Dec 18, 2016

If you feel like messing with it as you do so, feel free :)

Loading

@alranel alranel merged commit a26a60f into slic3r:master Dec 18, 2016
1 check passed
Loading
@alranel
Copy link
Member

@alranel alranel commented Dec 18, 2016

I'm doing some minor adjustments, but this new feature is really great to have :)

Loading

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Dec 18, 2016

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants