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

Add new solvespace solver #320

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

vespakoen
Copy link
Collaborator

@vespakoen vespakoen commented Jan 13, 2023

Hi there!

I am Koen, and I am a maintainer of SolveSpace, thanks for this addon, it looks really cool!

My goal is to merge Yuan Chang's PR (solvespace/solvespace#493) that adds the python bindings to SolveSpace into the main branch, and make sure it works and keeps working in the future.

I also renamed the package from python_solvespace to solvespace, and worked on the CI a bit.
That code is over here in my fork: https://github.com/vespakoen/solvespace/tree/python

From this fork I published a test to test.pypi.org over here: https://test.pypi.org/project/solvespace/#files

I am looking for feedback, and would like the package to be tested on as many platforms as possible.

This PR is based on #268 but based on the latest main branch, it fixes an issue in solver.py, where the self.solvesys.set_group(group) was set in the wrong place.

This is not complete yet, but it seems to work a little at least, and I hope that it will move things along ;)

To try it out:

mkdir -p blender/scripts/addons
cd blender/scripts/addons
git clone -b solvespace-solver https://github.com/vespakoen/CAD_Sketcher.git
/path/to/blenders/bin/python -m pip install -i https://test.pypi.org/simple/ solvespace==3.1.0.dev43 --force-reinstall
# - add blender/scripts path to Blender Preferences -> File Paths -> Scripts
# - activate the extension
# - restart blender

For the curious, it seems to fix the angle bug?

Screenshot 2023-01-13 at 02 35 09

Breaks a bunch of other things though because it is incomplete ;)

@vespakoen
Copy link
Collaborator Author

Oops, looks like I was actually using an old fork, will rebase...

@vespakoen
Copy link
Collaborator Author

Fun fact, the angle issue is back after rebasing, so it might make sense to add a failing test case and git bisect to see where it got introduced =)

@amrsoll
Copy link
Collaborator

amrsoll commented Jan 13, 2023

Thank you for the PR, very cool :D Yeah I managed to introduce a separate bug with the angles unfortunately. Usually it makes the angles switch between the direct or the complementary angle.

The original problem was that the solver just didn't want to produce a solution, which was why I thought it was best to have an newer or a different solver :) of course, it is a lot of work to update the API endpoints.

I would suggesting converting to a "draft" PR if it is not functional yet. ;)

Copy link
Owner

@hlorus hlorus left a comment

Choose a reason for hiding this comment

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

Hey Koen,
Great work, it's nice to see effort on an official python binding! Definitely looking forward to having this PR merged.

This will probably have to wait until the package is available on the official PyPi, in the meantime we could have these changes in a branch to work on the remaining issues and to get people to test it.

model/angle.py Outdated Show resolved Hide resolved
model/symmetry.py Outdated Show resolved Hide resolved
model/vertical.py Outdated Show resolved Hide resolved
model/horizontal.py Outdated Show resolved Hide resolved
solver.py Outdated Show resolved Hide resolved
@vespakoen
Copy link
Collaborator Author

Thanks for the suggestions, and yes, I will merge the python-solvespace bindings once I am confident that they are working well, so basically I want to make sure that the compiled libraries are working on all platforms, and that all the functions are working as they should.

Thanks to @amrsoll's work, I now have a nice way to test the bindings =)

@vespakoen vespakoen marked this pull request as draft January 13, 2023 16:42
@vespakoen
Copy link
Collaborator Author

vespakoen commented Jan 14, 2023

Note: to use the latest fixes, you have to install the pre-release of the solvespace python package:

/path/to/blender/python -m pip install -i https://test.pypi.org/simple/ solvespace==3.1.0.dev43 --force-reinstall

@vespakoen
Copy link
Collaborator Author

For the horizontal and vertical points constraints, you need a newer version of the solver:

/path/to/blender/python -m pip install -i https://test.pypi.org/simple/ solvespace==3.1.0.dev44 --force-reinstall

@vespakoen
Copy link
Collaborator Author

It all seems to work pretty well now, and all the tests I added are passing =) would love some feedback from others that have used the main CAD Sketcher a lot and can maybe find differences more easily.

@hlorus
Copy link
Owner

hlorus commented Jan 15, 2023

Awesome, i'd suggest to ask people on discord for testing. You should have been added as a collaborator on this repository so feel free to create a branch to have people test it.
I'll also have another look at it in the coming days. :)

@hlorus
Copy link
Owner

hlorus commented Jan 19, 2023

There are a few things i've noticed that don't work so far:

  • Tangent between two curves
  • Equal between a line and an arc
  • Vertical/Horizontal aligned distance

Let me know if there's something i can help with.

@phkahler
Copy link

@vespakoen I haven't been following the solver branch or CAD sketcher very closely. Two major changes have happened in Solvespace and I wonder if they affect this much.

  1. Last year we (mostly Ryan via Evil Spirits work) integrated Eigen, which introduces an external dependency.
  2. Just recently we added the multi-constraint patch, fixed, and expanded it.

I'm not sure where the external solver API sits, so maybe that second item has no effect, or maybe it's really good news for CAD sketcher. You tell me ;-)

I also added faces parallel and perpendicular, but I don't think CAD sketcher is using 3D constraints is it?

Thanks!

@vespakoen
Copy link
Collaborator Author

@hlorus Thanks for the tests, I have actually also found those myself, and a couple of other problems as well.

Basically, the python-solvespace and slvs_py bindings wrap the C library that SolveSpace exposes, this C library is actually missing some good parts of SolveSpace, for example, and most notably: https://github.com/solvespace/solvespace/blob/master/src/constraint.cpp#L134-L195

Also, the C library doesn't work in the most efficient way, basically CAD Sketcher is loading all the drawings into the SolverSystem on every solve, and I guess, when dragging something, this might slow things down at some point.

The python-solvespace library adds it's own "Entity" class, that wraps a SolveSpace EntityBase, but it is not "the actual entity", this causes users to keep track of things themselves, which can actually be avoided when the SolveSpace library would be C++ based.

So the last 2 days I went down into the rabbit hole of making pybind11 bindings for SolveSpace, and for that I first had to "rip out" the core of SolveSpace, many things depend on each other, and some SolveSpace UI specific stuff is also living in the "core" parts.

The most notable offender here is the "Group" object, it contains a lot of code for UI / Mesh related things, but the Solver can work with a group that only has a couple of properties and a single method.

My goal now is to "untangle" the core from SolveSpace, make that available as a C++ library and as pybind11 python bindings, this will be a net positive for other applications using the Solver (like CAD Sketcher) because they don't have to repeat the work that is already done in SolveSpace, and also for SolveSpace, because the "core" is seperated from the UI stuff.

Basically, I have added a bunch of methods to the "Sketch" object (basically the ones that are on python-solvespace's SolverSystem object (added it to a gist for now: https://gist.github.com/vespakoen/2abe264d0f12736659cf9eb19f2daed5)

And some to the EntityBase (also "stolen" from the python-solvespace project):

    inline bool IsPoint2D() {
        return type == Type::POINT_IN_2D;
    }

    inline bool IsPoint3D() {
        return type == Type::POINT_IN_3D;
    }

    inline bool IsNormal2D() {
        return type == Type::NORMAL_IN_2D;
    }
    ... etc

This is all very experimental at the moment, but I am hoping this can work out the way I imagine it, if not, I will make sure to add the missing stuff to the python-solvespace library.

@phkahler The Eigen dependency is not a big problem, it's header only and can easily be included, the multi-constraint stuff is not exposed in the library, this is something that CAD Sketcher has to implement, and in some ways already did I think.

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

Successfully merging this pull request may close these issues.

None yet

4 participants