MADEAPPS / newton-dynamics

Newton Dynamics is an integrated solution for real time simulation of physics environments.
http://www.newtondynamics.com
Other
928 stars 183 forks source link

x64 on MingW doest not work #241

Open SamCZ opened 2 years ago

SamCZ commented 2 years ago

CMAKE_CL_64 variable is only for Microsoft Visual Studio I needed to force this if to make it compile to x64 https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-3.14/CMakeLists.txt#L154 https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/CMakeLists.txt#L196

JulioJerez commented 2 years ago

I do no understand, variable CMAKE_CL_64 if under MINGW brake.

for windows the is use the pointer size. which I believe works since I use every time. for Linux or Unix systems the define is at line 85 on teh same file and using this

if (BUILD_64)

if (CMAKE_CL_64) message("build 64 bit") add_definitions(-D_POSIX_VER_64) else (CMAKE_CL_64) message("build 32 bit") add_definitions(-D_POSIX_VER) endif (CMAKE_CL_64)

I just changed to use CMAKE_CL_64 instead of pointer size BUILD_64

what platform are you building for?

SamCZ commented 2 years ago

I use GCC on windows x64 with MingW that was downloaded from MSYS2 (mingw-w64-x86_64-gcc), for me the variable CMAKE_CL_64 is not defined.

EDIT: Idk why, but some of the demos newton-3.14 are crashing on nullptr entity when iterating from DemoEntityManager, but almost every one is working on sanbox.

JulioJerez commented 2 years ago

Are you saying that the make file generated by cmake does not add the defines for 64 bit build?

There are very few places where those define are used in Newton 4.

One such area in newton 3.xx was for data serialization, where class in the engine were cast to c structure.

That should not longer be nessesary, but there may still places where union of pointer and int are used. That can be fix by adding a long long to the union.

I will check it out, because it seems newton 4.0 should not require define for 32 or 64 bit types.

On Mon, Aug 2, 2021, 9:53 PM Sam @.***> wrote:

I use GCC on windows x64 with MingW that was downloaded fromMSYS2 https://www.msys2.org/ (mingw-w64-x86_64-gcc)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MADEAPPS/newton-dynamics/issues/241#issuecomment-891523651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6EPJCMLPGAAQ7N4SNL74LT25Y3LANCNFSM5BNPGATA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

SamCZ commented 2 years ago

Are you saying that the make file generated by cmake does not add the defines for 64 bit build?

Yes because CMAKE_CL_64 is not defined, so for me it goes to this branch https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-3.14/CMakeLists.txt#L159

I forced X64 by adding -DCMAKE_CL_64=ON argument to cmake command.

I will try today run the 4.xx samples if it is working.

JulioJerez commented 2 years ago

oh you are using 3.14

I just removed the custom platforms macros in favor of the native specified by the platform line WIN32, linux, apple, arm, and MINGW32__

I did not add that to 3.14 because it is a lot harder there with the pointer size and the data serialization between c++ code and c code. This is because some classes are virtual and the size and offsets are different in 32 and 64 bit. This does not happen in newton 4 because there is not data copy from c to c++ and vice versa.

can you please try using 4.0 is far more robust than 3.14 and already has almost all the functionality and soon will surpass it. is also a lot easier to use thanm 3.xx

I build in windows with the changes. at least see if you can make compile if not I will try later with MINWG32

Julio

On Tue, Aug 3, 2021 at 8:41 AM Sam @.***> wrote:

Are you saying that the make file generated by cmake does not add the defines for 64 bit build?

Yes because CMAKE_CL_64 is not defined so for me it goes to this branch https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-3.14/CMakeLists.txt#L159

I forced X64 by adding -DCMAKE_CL_64=ON argument to cmake command.

I will try today run the 4.xx samples if it is working.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MADEAPPS/newton-dynamics/issues/241#issuecomment-891952372, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6EPJDA3CZEBT7KCWJ343TT3AEYXANCNFSM5BNPGATA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

SamCZ commented 2 years ago

ok thanks, I will try the 4.0 and write you an comment.

The reason I firstly used 3.14 its because my college from work said that the 4.0 is work in progress so he would use the 3.14, but nevertheless I will try the 4.0, thanks for the info.

JulioJerez commented 2 years ago

Yes it is a wip. But that's only for new functionality.

The reason for 4.0 is to fix some of the bottleneck imposed by 3.xx and lower C interface. Most the core funtionality is already in. 4.0 inherited all the knowledge we learned in 3.xx and build on that.

I will son make a stable release. I did not want to so that I did not have to add sub version so son.

I am of courses bias, but you can bilieve 4.0 is by far better than 3.xx

On Tue, Aug 3, 2021, 9:05 AM Sam @.***> wrote:

ok thanks, I will try the 4.0 and write you an comment.

The reason I firstly used 3.14 its because my college from work said that the 4.0 is work in progress so he would use the 3.14, but nevertheless I will try the 4.0, thanks for the info.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MADEAPPS/newton-dynamics/issues/241#issuecomment-891970878, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6EPJGPLIYZHZITBRTHOLTT3AHTHANCNFSM5BNPGATA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

SamCZ commented 2 years ago

C:/Sam/Projects/EmberSky/lib/Aurora/lib/newton-dynamics/newton-4.00/sdk/dCore/dTypes.h: In constructor 'dAtomic<T>::dAtomic()': C:/Sam/Projects/EmberSky/lib/Aurora/lib/newton-dynamics/newton-4.00/sdk/dCore/dTypes.h:353:13: error: 'val' was not declared in this scope; did you mean 'm_val'? 353 | : m_val(T(val)) | ^~~ | m_val

https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/sdk/dCore/dTypes.h#L313

EDIT: Do you have discord or something where we could chat ?

SamCZ commented 2 years ago

Next error: image

C:\Sam\Projects\EmberSky\lib\Aurora\lib\newton-dynamics\newton-4.00\sdk\dNewton\dModels\dVehicle\ndMultiBodyVehicle.cpp:618:69: error: no matching function for call to 'dMax(double, float)' 618 | dFloat32 den = dMax (dAbs (entry1->m_speed - entry0->m_speed), 1.0f);

SamCZ commented 2 years ago

Btw I had enabled NEWTON_DOUBLE_PRECISION Rn I'm trying it without it

SamCZ commented 2 years ago

I just compiled ndSandbox, but I needed to make some fixes in the cmake files

JulioJerez commented 2 years ago

Ok the error are fixed. Please try again. any reason why you buidl single tread?

using a thread the engine will run concurrent with you app. what is mean is that as a long as you run on a system with more than one core, an dthe frame update takes less that the graphics. you get the physics for free. the method is implement similar to the way opengl implement concurrent update.

what fixes you made to the cmake file?

JulioJerez commented 2 years ago

I also fixed the errors and warning in double. I see there is a bug with collsion somewhere when using double precision.
I saw the truck pass right over the floor.

I will check it out.

JulioJerez commented 2 years ago

I set up a repro for debug the bug using double.
I will fix later, but you can keep building let us see if there are more errors.

so far it seems fine using 32 bit floats.

JulioJerez commented 2 years ago

Ok I found the bug with double.
It is wit the Simd solvers. they are some place where I do things liek this

            dInt32 temp = -1;
            m_mask = dVector(*(dFloat32*)(&temp));

as you can see with 32 bit float that code works, but in 64 bit is fail because only the lower 32 bit of the double are set.

I nee to search for all teh place were the operation Select is using and see it it is using a mask trick and replace with teh correct code. I will be fix some time today. meantime the float 32 should be working fine.

JulioJerez commented 2 years ago

ok double precision is fixed now.

SamCZ commented 2 years ago

Sorry I went to sleep last night, So first I needed to fix adding glfw subdirectory here https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/thirdParty/CMakeLists.txt#L17 because i already had loaded glfw in my engine.

The first fix is:

if (MSVC OR MINGW)
        if(NOT TARGET glfw)
        add_subdirectory(glfw)
    endif()
endif(MSVC OR MINGW)

After this it compiled properly, but when I tried to compile the samples, there was and error with wrong folder in cmake for OpenAL .lib file in https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/applications/ndSandbox/CMakeLists.txt#L74 I changed ${CMAKE_SOURCE_DIR} to ${PROJECT_SOURCE_DIR} at every place in this file (this was cause by adding newton-dynamics as a submodule to subdirectory at my project (C:\Sam\Projects\EmberSky\lib\Aurora\lib\newton-dynamics\newton-4.00)).

So when this fix was introduced and everything was built I tried to include newton in my engine, but there was an issue that some includes for core and others were missing because you dont use target_inclue_directories in your cmake, so I did another fix. Here https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/sdk/CMakeLists.txt#L107

I added these lines:

target_include_directories(ndNewton PUBLIC dCore/)
target_include_directories(ndNewton PUBLIC dNewton/)
target_include_directories(ndNewton PUBLIC dTinyxml/)
target_include_directories(ndNewton PUBLIC dCollision/)
target_include_directories(ndNewton PUBLIC dNewton/dJoints/)
target_include_directories(ndNewton PUBLIC dNewton/dModels/)
target_include_directories(ndNewton PUBLIC dNewton/dModels/dVehicle)
target_include_directories(ndNewton PUBLIC dNewton/dModels/dCharacter)

If I forgot something I will comment here after I get home.


any reason why you buidl single tread?

It was just because option("NEWTON_BUILD_SINGLE_THREADED" "multi threaded" OFF) is weirdly commented, the option say that if you set this to ON it will disable multithreading, but the commend say otherwise. I will use multithreading.

Last night I managed to get two cubes collide in my engine with your lib, so I'm happy now. Thanks for the help, this library is very nice and I will add your logo to my game when I have splashscreen and credits implemented.

JulioJerez commented 2 years ago

I assume you are intergeration the cmake to you project, so that you project compile the source. If that's the case you can click off building the demos so that is does not build the third parties libraries.

But also if you want, you can just build the engine and demos in some folder in you machine. Them in cmake the is and install directory. You can set that to point to some directory in your project. Them after you build the sdk, if you build install, The headers, the libraries and the dll will be copied to that destination.

Them in your engine you just use that path for the include.

Anyway, I see what is confusing with the single thread comment.

On Wed, Aug 4, 2021, 3:14 AM Sam @.***> wrote:

Sorry I went to sleep last night, So first I needed to fix adding glfw subdirectory here https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/thirdParty/CMakeLists.txt#L17 because i already had loaded glfw in my engine. The first fix is: if (MSVC OR MINGW) if(NOT TARGET glfw) add_subdirectory(glfw) endif() endif(MSVC OR MINGW)

After this it compiled properly, but when I tried to compile the samples, there was and error with wrong folder in cmake for OpenAL .lib file in https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/applications/ndSandbox/CMakeLists.txt#L74 I changed ${CMAKE_SOURCE_DIR} to ${PROJECT_SOURCE_DIR} at every place in this file (this was cause by adding newton-dynamics as a submodule to subdirectory at my project (C:\Sam\Projects\EmberSky\lib\Aurora\lib\newton-dynamics\newton-4.00))

So when this fix was introduced and everything was built I tried to include newton in my engine, but there was an issue that some includes for core and others were missing because you dont use target_inclue_directories in your cmake, so I did another fix. Here https://github.com/MADEAPPS/newton-dynamics/blob/master/newton-4.00/sdk/CMakeLists.txt#L107 I added these lines: target_include_directories(ndNewton PUBLIC dCore/) target_include_directories(ndNewton PUBLIC dNewton/) target_include_directories(ndNewton PUBLIC dTinyxml/) target_include_directories(ndNewton PUBLIC dCollision/) target_include_directories(ndNewton PUBLIC dNewton/dJoints/) target_include_directories(ndNewton PUBLIC dNewton/dModels/) target_include_directories(ndNewton PUBLIC dNewton/dModels/dVehicle) target_include_directories(ndNewton PUBLIC dNewton/dModels/dCharacter) If I forgot something I will comment here after I get home.

any reason why you buidl single tread? It was just because option("NEWTON_BUILD_SINGLE_THREADED" "multi threaded" OFF) is weirdly commented, the option say that if you set this to ON, but the commend say otherwise. I will use multithreading.

Last night I managed to get two cubes collide in my engine with your lib, so I'm happy now. Thanks for the help, this library is very nice and I will add your logo to my game when I have splashscreen and credits implemented.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MADEAPPS/newton-dynamics/issues/241#issuecomment-892538528, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6EPJFXFF42NKVCGPVJI6TT3EHGXANCNFSM5BNPGATA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

SamCZ commented 2 years ago

Hi, just updated repo and enabled doubles and this error pops out


                 from C:\Sam\Projects\EmberSky\lib\Aurora\lib\newton-dynamics\newton-4.00\sdk\dCollision\ndCollisionStdafx.h:30,
                 from C:\Sam\Projects\EmberSky\lib\Aurora\lib\newton-dynamics\newton-4.00\sdk\dCollision\ndBody.cpp:23:
C:/Sam/Projects/EmberSky/lib/Aurora/lib/newton-dynamics/newton-4.00/sdk/dCore/dSpatialVector.h:97:15: error: member 'dBigVector dSpatialVector::<unnamed union>::<unnamed struct>::m_low' with constructor not allowed in anonymous aggregate
   97 |    dBigVector m_low;
      |               ^~~~~
C:/Sam/Projects/EmberSky/lib/Aurora/lib/newton-dynamics/newton-4.00/sdk/dCore/dSpatialVector.h:98:15: error: member 'dBigVector dSpatialVector::<unnamed union>::<unnamed struct>::m_high' with constructor not allowed in anonymous aggregate
   98 |    dBigVector m_high;
      |               ^~~~~~```
JulioJerez commented 2 years ago

please see my comment in the pull request.

SamCZ commented 2 years ago

It builds fine now, thanks.