Topological Naming (another take)

Compiling a branch and being able to start testing would be a good start. But this still would expose your work to only a handful of FreeCAD users/developers. Therefore i guess it comes down to binaries for different platform. There i guess is just no way around that.

In addition @realthunder and @ickby approach started to align. It took at least a year for that to happen. I don’t want to push you in that direction. As i feel you are entitled to work on your ideas. But from upstream point of view it would likely make sense if 3 developers would work on the same effort in FreeCAD 0.18 (and likely FreeCAD 0.19 development cycle). This is just such huge effort. And if we want to start using anything remotely useful in next two years. It would likely take 3 developers at minimum to make it happen. More developers working on the same goal would likely enable each developer to contribute something unique to the final solution. Therefore maybe it wouldn’t end up exactly as you have envisioned it. But that might end up being a good thing.

well,let me know if my instructions don’t work. if they do, then at least part one of your proposal is accomplished. regarding binaries for different platforms, I believe it is a bit premature for that. Currently this solution is still in the early phases of development, and therefore I am merely looking for input from other developers. It is not really ready for rigorous user testing.

I’m not averse to switching directions. if realthunder 's approach is the better one I can support that. however, before abandoning my own work, I’d appreciate at least a cursory review from ay least ickby and realthunder. if we’re the only folks working on this problem, and we can all agree on a single direction, then I will support that wholeheartedly.

On Ubuntu 16.04 this is the first problem (due to older CMake):

CMake 3.7 or higher is required.



I’m not averse to switching directions. if realthunder 's approach is the better one I can support that. however, before abandoning my own work, I’d appreciate at least a cursory review from ay least ickby and realthunder. if we’re the only folks working on this problem, and we can all agree on a single direction, then I will support that wholeheartedly.

Note that i personally don’t have an issue with all 3 of you developing different solutions. Especially in prototyping phase. That makes much sense. And i am not saying you shouldn’t proceed to follow your ideas. Comment was more geared towards if we are talking about upstream work. And to get something working in foreseeable future. There is where a common goal likely does make sense. And indeed @realthunder and/or @ickby will hopefully get involved. And evaluate your proposal. At some point in the future. I guess once @realthunder shares his work. Likely that will interest @ickby. As the proposal and work done should generally speaking be aligned. And you already shared your work and plans. Therefore i guess we are getting there. :wink:

What version of cmake do you have? I don’t think I use any fancy features, I can probably lower the minimum version. I just set it to the version I had installed. you can change that line in the top of the CMakeLists.txt file (the first one) and retry.

I use version 3.5.1. OK i changed the version number manually and followed the instructions:

https://forum.freecadweb.org/viewtopic.php?f=10&t=27582#p222179

Unfortunately things stop again at compiling OccWrapper step. It looks like Google Test library would be needed? I commented out the test folder part from CMakeLists altogether. Compiling OccWrapper was possible but unfortunately TopoManagers didn’t want to play along after. Therefore to first make things clearer. What is the situation with Google Test library dependency?

:blush: Im so embarrassed you’re having such problems with my code base!

You can get rid of all the googletest stuff in the CMakeLists.txt it is used for unit testing. I don’t know why you’re having problems with it, it could be I forgot to add the googletest directory to my git repository.

Is the googletest the only issue you’ve had with compilation?

It would i guess be too easy if it would just work. Where would be the fun in that? :wink:

You can get rid of all the googletest stuff in the CMakeLists.txt it is used for unit testing. I don’t know why you’re having problems with it, it could be I forgot to add the googletest directory to my git repository.

Is the googletest the only issue you’ve had with compilation?

Yes it is missing. I was able to “fix” this part by leaving the whole test folder out when compiling OccWrapper. But i guess by compiling OccWrapper without the unit tests. After when compiling TopoManagers. It looks like TopoManagers doesn’t like the OccWrapper compiled without unit tests. And TopoManagers fails to compile as a result. This is it for today. Shared my current understanding. Will likely try again tomorrow. To see if further progress can be made.

I also get the error that a cmake file is missing in the googletest dir. But it looks like the directory is empty - so maybe you just need to add the files to git?

btw:

this should be

~> git clone https://github.com/ezzieyguywuf/FreeCAD-1 -b TopoManager

when I get home tonight, I’ll fix the googletest problem. I’ll also try to download a fresh clone and compile (like I should have done to begin with!) to ensure everything works as expected. (this is what happens when you let mechanical engineers code: they make silly mistakes like this :blush:)

I’m the meantime, anyone anxious to get this going now should be able to git clone the googletest files straight from their github into the test directory

I’m shocked at triplus’ comments that TopoManagers seems to “not like” that OccWrapper was compiled without googletest. it shouldn’t matter. this suggests there’s something more nefarious wrong with the build system I’ve set up.

all the more reason for me to download a fresh clone and try compiling from scratch.

Sorry about all the bother: please know that I’m truly embarrassed and have taken this as a learning opportunity (which in fact my involvement with freecad was always intended to be). I should hopefully not make these types of errors in the future.

Mistakes are the seads of greatness, without them we would still be swinging from trees. :slight_smile:

Just noticed this thread. I’ll take a look at you code to understand a bit better about your approach first before commenting.

I’ve updated the second post as well as my git repos. I had a chance to test the OccWrapper download and it compiled on a fresh download (note the —recurse-submodules). I didn’t get a chance to test TopoManagers and FreeCAD.

I stared looking at cake Find_Package yo see if I could add something the searches the current working directory as a starting point.

Argh. Those mechanical engineer coders! There should be a law to make more of them! As how could a non-mechanical engineer coder understand what is the topology issue all about! :slight_smile:

I’ve updated the second post as well as my git repos. I had a chance to test the OccWrapper download and it compiled on a fresh download (note the —recurse-submodules). I didn’t get a chance to test TopoManagers and FreeCAD.

I stared looking at cake Find_Package yo see if I could add something the searches the current working directory as a starting point.

I can confirm compiling OccWrapper now following the instructions is successful. Will proceed with the rest of instructions.

P.S. Setting lower CMake version requirement and uncommenting #TKShHealing was needed.

No joy with TopoManagers. A bunch of errors returned when trying to compile. Will wait for you to confirm first. If everything is working on your side.

P.S. Note that there is no rush involved. Make your tests whenever you will have time (doesn’t have to be today).

Thanks for testing triplus! my wife and I are looking at some houses today, I’m going to try to look at this tonight I’m anxious to have some more people look at my stuff :slight_smile:

ps. can you list the errors you got?

Sure. At this point it is much more important for the house to have a good topology (or issues will occur). Therefore take your time!

I can build it, with some fixes of the library names:

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 2e2fa55..7b62c37 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -1,9 +1,9 @@
-AddLib(ISolidManager OccModifiedSolidd)
+AddLib(ISolidManager OccModifiedSolid)
 AddLib(PrimitiveSolidManager ISolidManager)
-AddLib(CompoundSolidManager ISolidManager OccBooleanSolidd)
+AddLib(CompoundSolidManager ISolidManager OccBooleanSolid)
 
 #add_library(PrimitiveSolidManager STATIC ISolidManager PrimitiveSolidManager.cpp)
-#target_link_libraries(PrimitiveSolidManager ${OCC_LIBS} OccSolidd)
+#target_link_libraries(PrimitiveSolidManager ${OCC_LIBS} OccSolid)
 
 #add_library(CompoundSolidManager STATIC CompoundSolidManager.cpp)
-#target_link_libraries(CompoundSolidManager ${OCC_LIBS} ISolidManager OccSolidd OccModifiedSolidd)
+#target_link_libraries(CompoundSolidManager ${OCC_LIBS} ISolidManager OccSolid OccModifiedSolid)
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 2f8e71c..17dd0b1 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -5,15 +5,15 @@ include_directories(${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR})
 
 set(NEEDED_LIBS
     ${OCC_LIBS}
-    OccEdged
-    OccFaced
-    OccSolidd
-    OccShaped
-    OccCylinderd
-    OccBooleanSolidd
-    OccSolidModifierd 
-    OccModifiedSolidd
-    OccSolidMakerd )
+    OccEdge
+    OccFace
+    OccSolid
+    OccShape
+    OccCylinder
+    OccBooleanSolid
+    OccSolidModifier
+    OccModifiedSolid
+    OccSolidMaker )
 
 AddTest("PrimitiveSolidManagerTester" ${NEEDED_LIBS} PrimitiveSolidManager)
 AddTest("CompoundSolidManagerTester" ${NEEDED_LIBS} CompoundSolidManager)

Trying to build freecad now :wink:

reox, you beat me there. I just finished pushing an update to my github with these library name fixes :stuck_out_tongue:

I don’t know why, but originally for some reason my libraries had that ‘d’ on the end of the name. I thought it was because they were “debug” version. shrug.

I’m also trying the FreeCAD build now. And I still want to try to improve my cmake-fu so that it can Find_Package (at least in the same dir).

:mrgreen: I have a fast machine on hand :wink:

But I’m still compiling FreeCAD because there are a ton of dependencies to install first… (this is always a PIA if you like to start building in a clean environment) I’ll probably continue tomorrow with that.

I’ve browsed through your code. Interesting approach, I’d say, and is quite different from mine. Here are my comments. I think it will save you a lot of trouble in the future if you can find a way to integrate your manager into TopoShape instead of the property. As you may have already find out, any transformation or copy operation may change the underlying TopoDS_Shape, causing failed match. If your manager lives inside TopoShape, it is easy to know when to update.

Have you considered how to deal with extrusions, which mostly comes from sketch. With your approach, you’ll probably need special manager for that to keep track of the face changes in extrusion due to sketch changes. And that is kind of against your whole idea of tracking edges using faces.

Faces are not always generated from other faces. For example, the extrusion mentioned above, and also in the offset operation, a face can be generated from an edge, or even a vertex.

Now the more crucial points (or not, depending on whether I understand your code correctly) is the potential ambiguity in your mapping scheme. You encode the face with three indices, i the index of base solid, j the origin face index from the base solid, and k the modification index. However, a new face can be originated from multiple faces from multiple base solids, and that origins may change as well, meaning new origin being added, or existing origining removed, and I don’t see any logic dealing with that in your updateMappedFaces(). Your approach demands some complex data structure to capture the shape evolution, which your current implementation does not have. But I do see in your code comments that it is on your to do list.

Even if we ignore the update issue for now, the ambiguity problem I mentioned will occur at your CompoundSolidManager::getFaceIndex(int). You use the j index to obtain the origin face, and then obtain a list of new faces modified from this origin face, and return this list as the result. So how can user unambiguously refer to any particular faces in that list? You didn’t use the k index. And since you encode the edge by two face reference, it means that there are edges that can’t be referenced unambiguously, too.

I wouldn’t say there is anything fundamentally wrong in your approach, because I can (vaguely) see a way around each problem I mentioned above. But then my thoughts are kind of biased to my own implementation. I am curious to see your take.