Physics Engine

Dec 29, 2007 at 12:56 PM
I would like to create a Physics Engine for the QS engine so when he camera goes up a hill, the camera cant go up a hill that is 70 Degrees and if the camera goes up a hill that is say 45 degrees it will slow the camera speed, when the camera returns to from 0 to 20 degrees it will be the same speed.
Dec 29, 2007 at 1:21 PM
I believe shawmishrak is doing the physics. Maybe you could get in touch with him and work together?
Dec 29, 2007 at 4:54 PM
Terrain collision is only a very small part of the planned physics implementation.

One idea I've been meaning to bounce off everyone is the possibility of writing a physics abstraction layer (as I believe someone mentioned awhile ago). Now that Xbox support is secondary, this seems even more plausible. We could find a managed-only physics library (or roll our own, if we want to spend a lot of time on it) like JiggleX for the Xbox-version, but stick with a high-performance library like AGEIA PhysX for the Windows version. I see it breaking down like follows:

  • QuickStart.dll contains interfaces and glue code for the physics abstraction layer. All other engine systems would have no idea that the physics implementation is external.
  • QuickStart.Physics.PhysX.dll contains an implementation of the physics interfaces for the PhysX SDK. This module would be in C++/CLI to directly interact with the PhysX library. Care would need to be taken to make sure the cross-boundary calls are limited, but its better than writing/using a wrapper.
  • QuickStart.Physics.JiggleX.dll contains an implementation of the physics interfaces for JiggleX, or whatever other all-managed library we want to use.

The beauty here on Windows is that the PhysX library is a lot more optimized and a lot faster than anything you could currently do in pure-managed code. You'll take a little bit of a hit by calling through interfaces and crossing native/managed boundaries, but I think the net gain will be positive.

Comments?
Dec 29, 2007 at 6:05 PM
I back you 100% on this. I love the idea of using a commercial implementation, as I mean no offence to anyone by this, its goign to be better than one we roll ourselves, as they have funding/time/etc. I also like the idea of it being abstracted so the client can use whatever system he so chooses. I absolutely agree with your suggestion, and would love to see it work out for us.
Dec 29, 2007 at 6:25 PM
Edited Dec 29, 2007 at 6:26 PM
I see two big issues with this, and I need to make sure everyone on the team is okay with them. First, this would mark an official split between the Xbox and Windows builds. Technically, the Xbox version would be Windows compatible, just with significantly reduced functionality/performance. Second, the physics code would be written in a mix of native C++ and C++ /CLI (for the Windows PhysX version). Only the interface and some glue logic would be written in C#. I don't have a problem being the maintainer on this if no one else wants to touch C++ (blasphemers!) but I also don't want it to become an issue. Also, The source tree would need to maintain physics binaries for those that use Visual C# Express and don't want to install Visual C++ Express.
Dec 29, 2007 at 7:32 PM
All good points. I see no problem with seperating windows and xbox, as thats in the process with netowrking anyway (sockets).

As for the language, the interface must be in c# and written so that it is able to be fully managed. If this is fulfilled I dont see a problem. It could even be xbox compatible at some stage. As for you kindly volunteering to do this, I have no problem with at all. As long as the interface is c#, this is only an issue for the physx part (which is not really a feature, more an optional extra in my view)
Dec 29, 2007 at 7:39 PM
Correct, engine and client code will only ever see pure-managed code. Whether this is pure C# or exposed C++/CLI classes, it doesn't really matter. It all looks the same through Intellisense.
Coordinator
Dec 29, 2007 at 10:23 PM
Edited Dec 29, 2007 at 10:27 PM
I'm ok with C++ , I have much more experience with it than I do with C#. We just need to make sure that it is easy for a C#-only programmer to pick up the engine, and use it as-is without knowledge of C++.
Dec 31, 2007 at 10:27 PM
As I earlier suggested having a interface (or some kind of abstraction) for the physics layer in order to achieve exactly this, it's fine with me. Also the physics implementation can use any language it wishes, as long as it can interface with the mananged world.

The implementation is totally agnostig to the engine user, as they will use pure managed languages (C# perfered).
Jan 2, 2008 at 8:59 PM
Hi, You have see XnaDevru physics engine for Xna? It's a port of Bullet to C#. It's would be a good start point?

Congratulations for very good work you are doing.
Jan 2, 2008 at 9:06 PM
I've looked at BulletX, and it's definitely a nice option. I'm considering either it or JiggleX for the managed-only version of the physics integration layer for the engine. Thanks for the heads-up!
Jan 5, 2008 at 4:46 PM
Does anyone have any preference on the solution/project file layout for the physics assembly?

Like I mentioned, the interface code will be in the main QuickStart assembly, and I'll create a new QuickStart.Physics.PhysX assembly for the implementation. However, if I add this project to the main solution file, C# Express will probably not be able to load it since it contains a C++ project file. I'm planning on just dropping a new solution file into the root directory that will be VS Pro only and contain all project files, then the C# Express users can just use the current solution file. Also, that would require me to the put the physics binaries in the source tree, for the sake of C# Express users. Is that acceptable for everyone?
Jan 5, 2008 at 6:59 PM
Why must it be added to the main project? Whatabout adding the binaries, and leaving the source in a c++ solution? Then if an express user wanted to read it, they could open it with c++ express, or if not just use the binaries.
Jan 5, 2008 at 7:08 PM
That's what I meant. The C+/CLI assembly is a separate project. Right now I have two Windows solution, one with the C+ project, and one without. As long as that's fine with everyone, I'll leave it like that.
Jan 5, 2008 at 10:29 PM
LordIkon, before I go and reproduce a bunch of code that you might already have somewhere, is there an index buffer (or any easy way to get an array of indices) for the entire terrain?
Jan 6, 2008 at 4:48 AM
Edited Jan 6, 2008 at 4:49 AM
The basics of the physics interface and PhysX back-end are in place. Sphere/boxes are implemented, and arbitrary triangle meshes are somewhat working. I can set up the entire terrain as a triangle mesh, but its horribly slow since there's so many triangles. LordIkon, I need a way to extract a height field from the terrain system. Basically, a 2d array of height values. How hard would that be to implement, most likely as a by-product of the terrain loading code? With that, I would be able to generate a PhysX height field instead of a tri-mesh, which should be significantly more efficient.


And of course this post wouldn't be complete without a "teaser" video: http://www.cse.ohio-state.edu/~holewins/QuickStart/QSEngine_PhysXDemo1.avi :)

The video is Xvid encoded. Any Xvid/DivX decoder should work.

The terrain isn't rendered or part of the physics scene since the collision isn't quite working yet due to the issue mentioned above. The scene in the video contains 900 spheres and 4 boxes. On one core, I get about 120 FPS. With the PhysX multi-threading, I don't see us having speed issues. It's performing much better than any pure-managed physics engine I've seen. :)

I'll try to clean up the code and post a patch for everyone to check out within the next day or so. The native/managed transitions were much easier than I imagined. Damn I've missed C++! ;)
Jan 6, 2008 at 5:15 AM
I am having way too much fun with this!

http://www.cse.ohio-state.edu/~holewins/QuickStart/QSEngine_PhysXDemo2.avi

1600 objects: 800 spheres / 800 boxes + 5 stationary boxes

Steady 70 FPS using 3 cores: 1 for the QuickStart engine processing and 2 for the PhysX simulator.
Jan 6, 2008 at 11:57 AM
First off, cool demos, ship it :)

I have no problems with two projects, I would say put the code under Framework/code/Physics/PhysX (something like that anyway) and put the binaries under framework\content\binaries and have a post build process for the express solution which copies the binaries to the target folder.
Jan 6, 2008 at 8:20 PM
Heightfields are now working! Yay! They are working much better than triangle meshes for terrain collision.

At the moment, the entire terrain is treated as a height field, and is automatically generated by the physics back-end based on the heightData array in Terrain.cs and the terrainScale variable in Terrain.cs. Arbitrary actors can be put in the world, composed of boxes (any scale, rectangular), spheres, tri-meshes, and height fields. Actors can be composed of any combination of these shapes, so you could theoretically have a group of these shapes make up a single actor.

Everything's functional and performing very well, so I'm going to clean up the code and post a patch for everyone to review. I'll try to get that done today.

To use the patch, you will at a minimum need the PhysX System Software installed. Just to go ageia.com and download the latest system software. To compile the C++ /CLI assembly, you will additionally need the PhysX SDK. It is also available at ageia.com. You will need to go to the developer area and register before you can download it. Registration is free, but the important thing is to accept the EULA.

Environment-wise, I did not want to put the path to the Agiea SDK files within the project files since the installation path may be different between computers. If you want to compile the C++ /CLI project, you will need to set up the Ageia SDK within Visual Studio, through Tools > Options > Projects and Solutions > VC++ Directories (Include Files and Library Files).


And before I forget, height field teaser: http://www.cse.ohio-state.edu/~holewins/QuickStart/QSEngine_PhysXDemo3.avi

I get about 60-80 FPS in that video, but its pushing 3 of my 4 cores pretty hard between QuickStart and PhysX. I'm limiting PhysX to 2 simulation threads right now since dual-core machines are pretty standard right now.
Jan 6, 2008 at 9:26 PM
It looks totally awesome, I have the feeling that it performs better than the template engine, is that right?

What is actually needed in order to use the PhysX engine? What is the size of the assemblies needed, would it be possible to just add them to the depot?
Jan 6, 2008 at 9:44 PM
I never did any benchmarking with the template engine, but I would assume so. Nothing against LordIkon, it's just that PhysX is a highly-optimized native library.

In order to run a program using PhysX, you need to have the PhysX System Software installed. You can't just copy the DLLs, they won't work without the installer. In that sense, it's like DirectX. I dont really see it being a big deal. It's free and easy to install. I like Havok's static library model better, but Havok isn't free. :)

There is a part of PhysX (the geometry cooking library) that needs to be in the application folder, and all games I've seen include it in their installer, so I'm assuming it's cool to leave it with our binaries.

The SDK files we need can't be included in the source tree, that would definitely be against the EULA. But then again, it's just a one-time setup needed to be done by each developer wanting to compile the physics assembly. If you just want to use and reference the physics assembly, you dont need the PhysX SDK installed; just the System Software.


Jan 6, 2008 at 10:22 PM
Patch 662 is now available!

The Scene.cs file is now a bit of a mess, but all of the physics actor setup (excluding terrain) is in the Scene constructor. Go there to play around with the physics.

How does the PhysX integration affect the QuickStart build process?

There are now two solution files: QuickStartWindows.sln and QuickStartWindowsAll.sln. Additionally, there are two QuickStartSampleGame projects: QuickStartSampleGameWindows.csproj and QuickStartSampleGameWindowsPhysXDev.csproj. They are loaded as follows:

    QuickStart_Windows.sln loads QuickStartSampleGame_Windows.csproj
    QuickStart_Windows_All.sln loads QuickStartSampleGame_Windows_PhysXDev.csproj and QuickStart.Physics.PhysX.csproj

*What's the difference? *

The QuickStart_Windows.sln solution does not contain the PhysX C+/CLI project, and QuickStartSampleGame is configured to reference an already existing binary version of the QuickStart PhysX assembly. You do not need Visual C+ and you do not need the PhysX SDK to use this version. You do, however, still need the PhysX System Software. All of the C# engine/game code is compilable in this solution! There should be no loss of functionality if you only have Visual C#! The other solution is only needed if you want to modify the PhysX back-end assembly

The QuickStartWindowsAll.sln solution contains the PhysX C+/CLI project, and the QuickStartSampleGame project references this project. You need Visual C+ to use this solution, as well as the PhysX SDK!

How do I install the PhysX System Software?

Simple! Just download and install this: http://www.ageia.com/drivers/PhysX_7.11.13_SystemSoftware.exe

How do I install the PhysX SDK?

NOTE: Again, this is not needed if you do not wish to compile/modify the QuickStart PhysX assembly. All C# code is compilable through the QuickStart_Windows.sln solution without this installation.

This is a bit more complicated, but not by much. Go here: http://devsupport.ageia.com/ics/support/default.asp?deptID=1949, sign-in (you will need to register, it's free!), then download the latest PhyX SDK. Once you're logged in, click on the "Online Support" tab, then on the "Downloads" link. On the left nav panel, you can expand the Downloads item and get to the PhysX SDK 2.7.3. Agree to the EULA, download, and install. Make sure you remember where you installed it.

Once installed, you will need to configure Visual Studio. Open up Visual Studio, then go to Tools > Options > Projects and Solutions > VC++ Directories.

Under "Show Directories For", select "Include Files". In the list of directories, add these directories: (NOTE: These paths may need to be adjusted based on where you installed the SDK. These paths are the defaults on a 64-bit OS. On a 32-bit OS, the default will be in "Program Files" instead of "Program Files (x86)".)

C:\Program Files (x86)\AGEIA Technologies\SDK\v2.7.3\SDKs\Physics\include
C:\Program Files (x86)\AGEIA Technologies\SDK\v2.7.3\SDKs\PhysXLoader\include
C:\Program Files (x86)\AGEIA Technologies\SDK\v2.7.3\SDKs\Foundation\include
C:\Program Files (x86)\AGEIA Technologies\SDK\v2.7.3\SDKs\Cooking\Include

Under "Show Directories For", select "Library Files". In the list of directories, add: (NOTE: Again, path may need to be adjusted)

C:\Program Files (x86)\AGEIA Technologies\SDK\v2.7.3\SDKs\lib\win32

You should now be able to build the QuickStart PhysX assembly in Visual Studio.
Jan 7, 2008 at 1:07 AM
Cool can't wait to have a look at it.
Jan 7, 2008 at 9:23 AM
Edited Jan 7, 2008 at 10:01 AM
I just saw the videos... Really cool!!! Good work! Now just imagine all those objects with shadows! What a amazing dream!
Now, I need to add some place inside the editor to add physics and set its parameters to an object. ;)

Coordinator
Jan 7, 2008 at 2:43 PM
Very cool Shaw, can't wait to look at it.
Jan 7, 2008 at 3:49 PM
The physics actors don't really have parameters yet. Density/mass and the collision shapes are the only variables right now. They all use the same default material. I will of course be adding support for materials, collision callbacks (event triggered when actor hits something or is the target of a raycast), and such.

Everyone: Let me know what you want feature-wise, and I'll prioritize accordingly!
Jan 7, 2008 at 5:10 PM
Edited Jan 7, 2008 at 5:16 PM
Just saw this on ageia.com:


In addition to AGEIA PhysX support for Windows, AGEIA's PhysX SDK is designed for use with Linux, Xbox 360 and Sony Playstation 3 (through Sony pre-purchase)

Free SDK Package:

Most Current PC Binary
Commercial & non-commercial use on PC
o Must keep registration information current
o Must agree to the EULA at the time of download (pops up prior to download)
o Available for Windows & Linux
o No PhysX hardware optimization requirement
PS3 platform (through Sony pre-purchase)
All platforms through some of our middleware partnerships, such as UE3, Gamebryo 2.2, and others


  1. This has changed since I last looked, for it to be free you had to pass the granny test of does-it-look-so-good-on-physx-ppu-youre-granny-would-notice. Guess theyve given up on that when they realised few people had the cash to buy the ppu.
  2. Does this mean we can have physx on the xbox, or because we are using xna, we are stopped due to the no-native-code restriction?

Im sold on Physx looking at the revised features now. Ive we can implemetn them all in some form or another that would be awesome. A long way off I expect!
Jan 7, 2008 at 6:38 PM
Because we are using Xna we can not use PhysX that assumption is correct. We would need to write a native application (game) in order to use physx on the XBox. Just as you need the Sony pre-purchase for developing on PS

Also note that the consoles aren't directly free, only windows/linux are available in the ree SDK.
Jan 7, 2008 at 9:08 PM
Generally you should use <see cref="" /> in comments in order to refer to other types or <paramref /> to refer to parameters

In Terrrain.cs
  • On Initialize do use this prefix for instance members
  • Is it really needed to pass PhysicsScene to initialize will there be more PhysicsScenes per game or could it just be a property on the Game class?
    • Having a look at QuickStartSampleGame it seems that this could just have been set in the initialize method, and not any real need for passing it

In StaticModel.cs
  • On added properties do place xml comments

In QSGame.cs
  • On Constructor you now pass a PhysicsSystem could you consider moving this to initialize, and what if the system is null?

In SceneManager.cs
  • On Constructor just remove the outcommented codelines as the shouldn't be used

In QuickStartSampleGame\program.cs
  • On Main move the ifdef out a here ;) Create a method and use the ConditionalAttribute instead.

In PhysicsActor.cs
  • Do split out the types into seperate files, I can see that the ShapeDesc types might be in a single file but not the rest
  • I can't see why the fields of ShapeDesc are exposed publicity, why not use properties? Is the access time to read these field so much?
  • PhysicsActor is a interface and should therefore start with a capital I to indicate this
  • Set only properties do not make sense and often indicates poor design, please explain the reason for having these

In PhysicsScene.cs
  • PhysicsScene is a interface and should therefore start with a capital I to indicate this
  • On BeginFrame you are using dt instead of GameTime why?

In PhysicsSystem.cs
  • PhysicsSystem is a interface and should therefore start with a capital I to indicate this

I don't like having binaries in the target/output folders in the source tree, they should be placed somewhere else in the tree and the move out as part of the post build process. This is safer if there will be changes to the build structure. Also minimizes the number of binaries in the source tree.
Jan 7, 2008 at 9:39 PM
Sturm, you take all the fun out of coding ;) (Just kidding!)

Seriously, these reviews are very valuable for overall code quality!

First off, all the code in Scene/SceneManager/Terrain/etc. is all temporary. That's best left up to LordIkon to decide where the terrain actor should be created, and how the scene manager should interact with the physics engine. I don't want to be stepping on his toes by making my own modifications to his scene code unless necessary.

The commented lines in SceneManager and just there for everyone to play around with. Each #if false block represents one of the test scenarios I was using, including the ones in the videos.


In StaticModel.cs

On added properties do place xml comments


I knew I was forgetting something! I added those in there to make rendering easier for the demo and forgot to comment those. Honestly, they may not even stay, I have plans for a better way to get at the rendering data.


In QSGame.cs

On Constructor you now pass a PhysicsSystem could you consider moving this to initialize, and what if the system is null?


This is part of a larger issue. QuickStart.Physics.PhysX.dll depends on QuickStart.dll for the physics interfaces. Thus, QuickStart.dll cannot hold a reference to QuickStart.Physics.PhysX.dll. The game code will control which physics implementation to use anyway, so I left it up to the game assembly to reference QuickStart.Physics.PhysX.dll. However, as you pointed out, this creates an issue with how to load the PhysicsSystem implementation into the QSGame class. The variable holding the instance in QSGame is private and read-only (by our convention), so QuickStartSampleGame is unable to directly set it.

It may make sense to eventually move this to your configuration system. Let the physics assembly name and the physics system class name be defined in the XML, then you can load the assembly and instantiate the physics system class. What do you think?


In QuickStartSampleGame\program.cs

On Main move the ifdef out a here ;) Create a method and use the ConditionalAttribute instead.


I bet you had a wonderful time looking over my C++ code and it's preprocessor macros. ;)

I do agree ConditionAttribute is probably a little nicer looking and "C#-ish"


In PhysicsActor.cs

Do split out the types into seperate files, I can see that the ShapeDesc types might be in a single file but not the rest


Fair enough. I have a habit of including helper classes/structs in the same file as the main classes if they're only used with the main classes.


I can't see why the fields of ShapeDesc are exposed publicity, why not use properties? Is the access time to read these field so much?


It really has nothing to do with access time, it's just 4x the amount of typing to make everything properties. ;) That, and I was in a C++ mindset when writing them. Does the "properties for everything" rule apply to structs as well? These things are really structs, but I made them classes in order to have a common base type and avoid excessive boxing. I have no problem creating properties for everything, it just seems like a lot of extra code for no gain.


PhysicsActor is a interface and should therefore start with a capital I to indicate this


Agreed. Oversight on my part. Ditto for all of these interfaces.


Set only properties do not make sense and often indicates poor design, please explain the reason for having these


Mass and density will have getters as well, and position/orientation will eventually have setters. I only implemented what I needed to make everything functional for this patch.


In PhysicsScene.cs

On BeginFrame you are using dt instead of GameTime why?


Old habits? I was thinking it would give you more control over the simulation. You could scale the frame time to create slow-motion physics, or scale it up to create fast physics. You could do the same with GameTime, but that requires users to know exactly which fields of GameTime are fed to PhysX. Not a huge deal if you want this changed.


I don't like having binaries in the target/output folders in the source tree, they should be placed somewhere else in the tree and the move out as part of the post build process. This is safer if there will be changes to the build structure. Also minimizes the number of binaries in the source tree.

Where you suggest these binaries be placed? Target/Windows/<build> seemed to be the most fitting. The QuickStartSampleGame has a post-build to move QuickStart.Physics.PhysX.dll (as a reference) and NxCooking.dll (as a post-build copy).

No comments on the C++ code? :)

Has anyone actually ran the code, or tried to build the C++/CLI project? I'm curious if there are any setup issues, and I'd also like to see some performance numbers, like the frame rates everyone is getting with what processor/RAM configurations.
Jan 7, 2008 at 9:41 PM


Sturm wrote:
Because we are using Xna we can not use PhysX that assumption is correct. We would need to write a native application (game) in order to use physx on the XBox. Just as you need the Sony pre-purchase for developing on PS

Also note that the consoles aren't directly free, only windows/linux are available in the ree SDK.


That is very true. On consoles, a license for PhysX is quite costly! It would be wonderful if the XNA team struck a deal with Ageia to bring a PhysX wrapper to Xbox, even with a small licensing fee, but I'm not holding my breath on that one.
Coordinator
Jan 7, 2008 at 10:29 PM
Couple things:

1.) Just because I made it, doesn't mean I own it. It's probably just safe to ask about things you're concerned about. If it concerns physics integration you may know more about it, or we could collaborate.

2.) I simply started the scene manager, because we needed a basic one just to get things rolling. I really don't know the full details of where it is headed, nor was it ever assigned to me. So that class is really free reign as we still haven't fully designed it.
Jan 7, 2008 at 10:54 PM

shawmishrak wrote:
Sturm, you take all the fun out of coding ;) (Just kidding!)


You are not the first one to state that, my usual response to why I have to write so many comments to code reviews is: Because you write sloppy code ;)


shawmishrak wrote:
First off, all the code in Scene/SceneManager/Terrain/etc. is all temporary. That's best left up to LordIkon to decide where the terrain actor should be created, and how the scene manager should interact with the physics engine. I don't want to be stepping on his toes by making my own modifications to his scene code unless necessary.


This can just be dangerous as it might actually end up in the release code, seen it more times than I should.


shawmishrak wrote:
The commented lines in SceneManager and just there for everyone to play around with. Each #if false block represents one of the test scenarios I was using, including the ones in the videos.


Would it have been possible to create a QuickStartSampleSceneManager and then have done it there?


shawmishrak wrote:
This is part of a larger issue. QuickStart.Physics.PhysX.dll depends on QuickStart.dll for the physics interfaces. Thus, QuickStart.dll cannot hold a reference to QuickStart.Physics.PhysX.dll. The game code will control which physics implementation to use anyway, so I left it up to the game assembly to reference QuickStart.Physics.PhysX.dll. However, as you pointed out, this creates an issue with how to load the PhysicsSystem implementation into the QSGame class. The variable holding the instance in QSGame is private and read-only (by our convention), so QuickStartSampleGame is unable to directly set it.

It may make sense to eventually move this to your configuration system. Let the physics assembly name and the physics system class name be defined in the XML, then you can load the assembly and instantiate the physics system class. What do you think?


Move it to the configuration file, without question.


shawmishrak wrote:
I bet you had a wonderful time looking over my C++ code and it's preprocessor macros. ;)
I do agree ConditionAttribute is probably a little nicer looking and "C#-ish"


Nope, love those macros ;)
But yes ConditionalAttribute is the more C#ish way of doing it.


shawmishrak wrote:
It really has nothing to do with access time, it's just 4x the amount of typing to make everything properties. ;) That, and I was in a C++ mindset when writing them. Does the "properties for everything" rule apply to structs as well? These things are really structs, but I made them classes in order to have a common base type and avoid excessive boxing. I have no problem creating properties for everything, it just seems like a lot of extra code for no gain.


Yup PIML over everything else :) writing properties isn't more than writing fields, VS and the prop snippet which helps you with that, and a simple teak of that makes it almost perfect (simply write prop and press tab twice and you see what I mean). I think we agreed on that structs are ok with exposing fields, and I think as long as we are consistent that ok.


shawmishrak wrote:
Old habits? I was thinking it would give you more control over the simulation. You could scale the frame time to create slow-motion physics, or scale it up to create fast physics. You could do the same with GameTime, but that requires users to know exactly which fields of GameTime are fed to PhysX. Not a huge deal if you want this changed.


While true this is only the first time you will encounter this, but having GameTime gives a consistent Xna behavior that most can relate to.


shawmishrak wrote:
Where you suggest these binaries be placed? Target/Windows/<build> seemed to be the most fitting. The QuickStartSampleGame has a post-build to move QuickStart.Physics.PhysX.dll (as a reference) and NxCooking.dll (as a post-build copy).


They could be placed under framwork/content/binaries as framework/content already contains compiled resources which are copied.


shawmishrak wrote:
No comments on the C++ code? :)


No I won't review the C++ code, I could review it for code errors but it wouldn't make sense to review it for guideline as you really can't use C# guidelines on C++ code, we would need another set of guidelines.


shawmishrak wrote:
Has anyone actually ran the code, or tried to build the C++/CLI project? I'm curious if there are any setup issues, and I'd also like to see some performance numbers, like the frame rates everyone is getting with what processor/RAM configurations.


No sorry I currently only have my laptop available, I'll try to see if I can do it tomorrow.
Jan 9, 2008 at 3:03 AM
Things aren't as peachy on the managed physics front. I tried writing an implemention of the physics interfaces using the JigLibX library, and my system was crawling at about 15 FPS with just 20 boxes on the terrain. This is all on one thread since JigLibX isn't threaded, but threading it won't help that much. If we want to have an all-managed physics solution in addition to the PhysX implementation, we'd need to use a severely low-detail terrain and keep with very, very simple physics interactions, or wait until JigLibX (or another all-managed physics solution) is more mature.
Jan 9, 2008 at 7:52 AM
I think that as long as you make it possible to do just that we shouldn't worry too much about that the performance of the managed isn't as good as the native library.
Jan 9, 2008 at 4:11 PM
Edited Jan 9, 2008 at 4:11 PM
The problem isn't so much that the performance is just less then the native version, the problem is that its completely unusable for anything but tiny terrains.
Jan 9, 2008 at 4:19 PM
Yes but the problemlies in the physics lib notin our model?
Jan 9, 2008 at 4:43 PM
The problem is that our world is too big. JigLibX can't handle it, not right now. I need to work out some way of splitting the world up or just some way to get it to work acceptably.
Jan 9, 2008 at 9:22 PM
I would say that getting JigLibX to work is a target for this release not a commit, I think that you should just make a issue for integrating JigLibX and set it for the 0.20 release. I would hate to have to postpone the 0.19 release just because of that.
Jan 9, 2008 at 10:03 PM
I agree, I'm not trying to get it fully working for 0.19. The issue that the PhysX dependency might be a little much for the "casual" users (not that we really have any yet) to accept. And it also prevents us from even releasing any Xbox version of the engine (that builds, at least).

Honestly, I would be happy to just work with PhysX and not have a managed physics implementation, but I'd also like the engine to run on Xbox. Plus, using the word "native" anywhere seems to scare off a sizable chunk of the XNA community.
Jan 14, 2008 at 10:23 PM
I'm about ready to submit the final physics patch.

Sturm, do you want me to integrate the physics loader with your configuration code, or do you want me to leave that to you? If you want me to do it, should I put it with the <managers> group, or create a new <physics> group? I'm not sure how you were planning on doing the assignment to QSGame.Physics.

I'm leaving the scene manager as-is right now. It needs to be looked at and recoded for 0.20 anyway.
Jan 14, 2008 at 10:42 PM
If your manager inherits from BaseManager then you should add the manager itself to the managers section, and then create a new section for the physics options, similar to the imputmanager.
Jan 14, 2008 at 11:36 PM
Interfaces cannot inherit from abstract classes, correct? Do you want me to restructure IPhysicsSystem as an abstract class and inherit from BaseManager?

The only issue I see with this is assigning the Physics property of QSGame. Configuration reading happens in QSGame.Initialize, but by then its too late to assign to Physics since its a read-only property. Should we move the configuration stuff to the QSGame constructor?
Jan 15, 2008 at 12:22 AM
Alright, I uploaded what should be the final physics patch for 0.19. The readonly keyword on physics in QSGame has been removed to allow the configuration system to set it when loading the XML file in Initialize. We can either keep it this way, since the variable is private anyway, or move the configuration reading code to the constructor.

Jan 15, 2008 at 3:50 AM
I'll have a look at it within the next 24 hours.

It's perfectally ok to remove the readonly keywoard if you need to, it's not set in stone. You should just have the readonly attribute if you only set the variable during construction. But since the initialize manager can potentially throw exceptions (missing/incorrent file etc.) this has to be outside the constructor in initialize.
Jan 15, 2008 at 5:41 AM
In ConfigurationManager.cs
  • Remove the added using statements they are not used.
  • Nice that you disregard xml comments ;)

In Terrain.cs
  • Remember to use this prefix

In QSGame.cs
  • Remember to use this prefix
  • You should move the if ( ... PhysicsSystem) into a seperate InitializePhysics method
  • Do not use if (.. is ..) a = b as a; because you are ecentially doing 2 as casts instead do a = b as a; if (a!=null)

In Scene.cs
  • Remember to use this prefix
  • Consider making actors readonly as it's only assigned during construction
  • Move the test code into a initialize method, even better would be to do this in a QuickStartSampleScene overloading Scene
  • Using +i is sooooo C+'ish use i++ in C# as there isn't any perf improvements here
  • Move the code from QueryForRenderChunks into a QuickStartSampleScene overload

In QuickStartSampleGame.cs
  • Why did you outcommented the terrain.AddNew(LOD.Low)

In ActorDesc.cs
  • Remove the unneeded using statements (System.Text etc)

In BoxShapeDesc.cs
  • Remove the unneeded using statements (System.Text etc)

In HeightFieldDesc.cs
  • Remove the unneeded using statements (System.Text etc)

In IPhysicsActor.cs
  • Remove the unneeded using statements (System.Text etc)
  • It's nice to see that you are using the Gets/sets comment annotation for properties, I would just wish you were more consistent

In PhysicsSystem.cs
  • Remove the unneeded using statements (System.Text etc)
  • Abstract base classes are not interfaces, you should not state so in the comments

In ShapeDesc.cs
  • Remove the unneeded using statements (System.Text etc)
  • Why do you have a base class for shape's but not for actor?

In SphereShapeDesc.cs
  • Remove the unneeded using statements (System.Text etc)

In TriangleMeshShapeDesc.cs
  • Remove the unneeded using statements (System.Text etc)

In QuickStartSampleGame_Windows
  • You need to use "" around paths in post build, as they could contain spaces ;)

I couldn't get the game to run and QuickStartSampleGame builds to the incorrect directory. it builds to bin and should build to target. I don't have time now, but I'll look at it later.
Jan 15, 2008 at 6:53 AM
In ConfigurationManager.cs
  • Remember to use brackets even on single line if statements
  • There should be a single space between if and ( you can just use the VS formatting (default CTRLkd) to fix that

The Physics assmebly will not load if PhysX isn't installed, stupid me for not having that ;)

It does run, but I get a framerate of about 5-10 initially (on my Core2 2.1 Vista 32 bit), until the boxes hit the surface at which time it raises to above 60.
Jan 15, 2008 at 7:22 AM
You'll have to resubmit a new pack once my changes have been checked in as we are touching the same files.
Jan 15, 2008 at 3:18 PM
Edited Jan 15, 2008 at 3:20 PM


Sturm wrote:
In ConfigurationManager.cs
  • Remove the added using statements they are not used.
  • Nice that you disregard xml comments ;)


If I didn't ignore comments, the parser would crash.


In Terrain.cs
  • Remember to use this prefix


99% of it isn't even my code. :)


In QSGame.cs
  • Remember to use this prefix
  • You should move the if ( ... PhysicsSystem) into a seperate InitializePhysics method
  • Do not use if (.. is ..) a = b as a; because you are ecentially doing 2 as casts instead do a = b as a; if (a!=null)


That's fine, it just seems like a waste to create an Initialize* method for every XML-loaded system when you can just do it all in one loop over the managers.

Also, we need to come to an agreement on naming: System or Manager?


In Scene.cs
  • Remember to use this prefix
  • Consider making actors readonly as it's only assigned during construction
  • Move the test code into a initialize method, even better would be to do this in a QuickStartSampleScene overloading Scene
  • Using ++i is sooooo C++'ish use i++ in C# as there isn't any perf improvements here
  • Move the code from QueryForRenderChunks into a QuickStartSampleScene overload


I'll spend some time cleaning this up, but to do most of what you suggest requires significant changes to SceneManager/Scene and I thought we were going to concentrate on that for 0.20.

I have to ask: what is so unreadable about using ++i instead of i++? I know there is no performance gain unless you're using complex types with overloaded increment operators (even in C++), but that's just a personal style of mine and I don't see it impacting code quality/readability at all.


In QuickStartSampleGame.cs
  • Why did you outcommented the terrain.AddNew(LOD.Low)


Oh, that was from an old terrain physics requirement. That can be uncommented.


In ActorDesc.cs
  • Remove the unneeded using statements (System.Text etc)

In BoxShapeDesc.cs
  • Remove the unneeded using statements (System.Text etc)

In HeightFieldDesc.cs
  • Remove the unneeded using statements (System.Text etc)


You have something against System.Text? :)


In IPhysicsActor.cs
  • Remove the unneeded using statements (System.Text etc)
  • It's nice to see that you are using the Gets/sets comment annotation for properties, I would just wish you were more consistent


Consistent as in saying "Gets/sets" instead of "Retrieves"?


In PhysicsSystem.cs
  • Remove the unneeded using statements (System.Text etc)
  • Abstract base classes are not interfaces, you should not state so in the comments


Yeah, yeah.


In ShapeDesc.cs
  • Remove the unneeded using statements (System.Text etc)
  • Why do you have a base class for shape's but not for actor?


The base class for shapes is needed to treat all shapes as the same type for use in a container. There's only one type of ActorDesc, so there's no need for a base.


In QuickStartSampleGame_Windows
  • You need to use "" around paths in post build, as they could contain spaces ;)


Fair enough.


I couldn't get the game to run and QuickStartSampleGame builds to the incorrect directory. it builds to bin and should build to target. I don't have time now, but I'll look at it later.


It has always built to framework/test/bin. I thought target/ was reserved for the engine binaries, not the test programs.


The Physics assmebly will not load if PhysX isn't installed, stupid me for not having that ;)


Actually, that's one area I'm looking to improve when I get the time. PhysX is actually split across multiple DLLs, and you first make a call to PhysXLoader.dll to initialize the API, which you can include as part of your binary distribution (no PhysX install required.) If a PhysX installation is not found, this call will fail but not crash, so you could use that to find broken installations. Really, all we need to make that work is to include PhysXLoader.dll with our binaries. I'm not sure why I didn't just do that to begin with.


It does run, but I get a framerate of about 5-10 initially (on my Core2 2.1 Vista 32 bit), until the boxes hit the surface at which time it raises to above 60.


Good to know. Is that with or without a debugger attached? Running without the debugger doubles my frame rate, if not more most of the time. This is especially noticable while the boxes are initially falling.


There should be a single space between if and ( you can just use the VS formatting (default CTRLkd) to fix that


Now that's just being picky! Again, I think this is a style issue that has no impact on code readability/quality. I have Visual Studio specifically set up to not include that space, as well as some other options with switch statement formatting and math expression formatting.
Jan 17, 2008 at 12:53 AM

shawmishrak wrote:
99% of it isn't even my code. :)


I didn't ask you to do it for the whole file actually, only for the code you changed (terrainActor and heightData are all instance members)


shawmishrak wrote:
That's fine, it just seems like a waste to create an Initialize* method for every XML-loaded system when you can just do it all in one loop over the managers.


We might need a better way of doing this in the long run, but factoring this out makes the rework a lot easier


shawmishrak wrote:
Also, we need to come to an agreement on naming: System or Manager?


I like manager :)


shawmishrak wrote:
I'll spend some time cleaning this up, but to do most of what you suggest requires significant changes to SceneManager/Scene and I thought we were going to concentrate on that for 0.20.


Problem about letting code survive releases is that someone might come to depend on it. Also others merging our changes into their cutsomized engine will have considerably bigger task.


shawmishrak wrote:
I have to ask: what is so unreadable about using ++i instead of i++? I know there is no performance gain unless you're using complex types with overloaded increment operators (even in C++), but that's just a personal style of mine and I don't see it impacting code quality/readability at all.


There is nothing unreadable or confusing about it (at least not for me). But for consistency of the code it does make a great difference. If someone reads our code and sees that we use i-- everywhere, then suddenly sees --i he has to wonder why was that used, what is the significance and how is it going to be affected if overridden.


shawmishrak wrote:
Oh, that was from an old terrain physics requirement. That can be uncommented.


Code review = Good!!


shawmishrak wrote:
You have something against System.Text? :)


Nope not at all, ,just hate the standard C# template for including that.


shawmishrak wrote:
Consistent as in saying "Gets/sets" instead of "Retrieves"?


I like consistency (Surprise!!). I like Gets/sets as this notion is also used by MSDN (mostly) and everyone is used to that, well if you code C# you are.


shawmishrak wrote:
It has always built to framework/test/bin. I thought target/ was reserved for the engine binaries, not the test programs.


Everything should be build to target/.... as the build environment will need one unified way of handling testing.


shawmishrak wrote:
Good to know. Is that with or without a debugger attached? Running without the debugger doubles my frame rate, if not more most of the time. This is especially noticable while the boxes are initially falling.


Can't say will have to wait till tomorrow


shawmishrak wrote:
Now that's just being picky! Again, I think this is a style issue that has no impact on code readability/quality. I have Visual Studio specifically set up to not include that space, as well as some other options with switch statement formatting and math expression formatting.


Well it might be piggy (Yes I know), but now everytime someone reruns the formatting that will show up as a change (and there might be a lot of those). Also this would require eveyone to change the standard setting in VS, and also violates a few source style checkers (ReSharper/Code Style Enforcer/ClockSharp). If you think that the team should use this standard then lets put it into the Code Guideline.
Jan 19, 2008 at 1:14 PM
Man, I just love this physics library. Look at this sample: http://www.zelsnack.com/jason/JttZ/Novodex_NET_Wrapper/
Jan 19, 2008 at 4:03 PM
It's definitely some good stuff. That wrapper is for an old version of PhysX/Novodex, though.
Jan 20, 2008 at 8:05 PM
I finally found the time to complete the work on physics, so patch 712 is now uploaded. I pulled the physics test code out of Scene.cs and created a new SampleScene.cs that derives from Scene in QuickStartSampleGame. It's quick and dirty, but it works.
Jan 20, 2008 at 8:11 PM
Cool, I'll try to review it within the next 24 hours.

It would be nice if someone could review and check in my patch so that we can get closer to closing 0.19
Jan 20, 2008 at 8:17 PM
This wrapper would be a cool contribuition to the XNA comunity as a side project also.
Jan 20, 2008 at 9:01 PM
This isn't really a wrapper. It's a higher-level abstraction from PhysX. Eventually this will lead to higher performance than a wrapper, since we can offload as much of the interfacing code as possible to the native layer instead of having a direct one-to-one mapping between PhysX and our interfaces.
Jan 20, 2008 at 9:15 PM

It would be nice if someone could review and check in my patch so that we can get closer to closing 0.19


Did you ever upload the latest patch? I commented on 682, then you said you made some changes but I don't see any new patches.
Jan 20, 2008 at 9:59 PM
Strange the last patch, of mine, I can see is 689
Jan 20, 2008 at 11:36 PM
Right, I'm dumb. I was looking under the Declined patches for some reason.
Jan 21, 2008 at 4:30 AM
LOL :p
Jan 21, 2008 at 5:42 AM
Shaw, my latest checkin invalidates your latest physics changes, you need to update your source and upload a new patch ;)

Until this has been done everyone needs to use changeset 8993 in order to use the latest physics patch
Jan 21, 2008 at 2:48 PM
New patch uploaded. I can either check it in, or wait for you to look it over. Your call.
Jan 21, 2008 at 6:18 PM
I can look at this later tonight.
Jan 21, 2008 at 6:31 PM
Alright, no rush. The only changes from the last patch (yesterday) are a few source cleanups in various files, and I added the conditional attributes in Program.cs for setting the CWD.
Jan 21, 2008 at 6:59 PM
Hi Shaw.

Listen, when I open the QuickStartWindowsAll.sln solution it gives me an error message:
"The application for project 'D\QuickStartEngine\framework\code\Physics.PhysX\QuickStart.Physics.PhysX.vcproj' is not installed."

Is this normal?
Jan 21, 2008 at 7:03 PM
Edited Jan 21, 2008 at 7:05 PM
You need the PhysX SDK in order to build the project.

You do not need the ...All.sln unless you want to work with the PhysX integration. When using the standard QuickStartEngine.sln the needed assemblies (precompiled) will be copied over automatically. Also the Phys project is a C++ application not a C#
Jan 21, 2008 at 7:16 PM
Edited Jan 21, 2008 at 7:19 PM
Thanks. I didn't notice: vcproj. ;)

But running the QuickStart_Windows.sln solution gives another error:
"Could not load file or assembly 'QuickStart.Physics.PhysX, Version=1.0.2942.17398, Culture=neutral, PublicKeyToken=null' or one of its dependencies. Unable to initiate this application because the configuration of the application is incorrect. The resettlement of the application may fix this problem. (Exception from HRESULT: 0x800736B1)"
Jan 21, 2008 at 8:05 PM
Two questions:

  1. Is that when you load the project, or run the game? The project files should have no reference to it.
  2. Do you have VS Pro installed, or just Visual C# Express?

If this happens only when you run the game, then try a Release build. If you don't have Visual Studio Pro installed, the precompiled Debug version of QuickStart.Physics.PhysX.dll will be looking for the debug C++ run-time, which only exists if you have VS Pro installed. This could be a potential issue. I may have to change the dependencies so the QuickStart_Windows.sln solution only looks at the release version of the precompiled binaries.
Jan 21, 2008 at 8:41 PM
I did the review:

In Terrain.cs
  • On Initialize you should use this prefix on instance members (heightData)

In ActorDesc.cs
  • shapes could be made readonly as it's only assigned during construction

In Program.cs
  • NICE REWORK :)

In SampleScene.cs
  • Very good rework, it's nice to see the abstraction away from the engine. We do need to go back later and do more work in the engine but I think this step helps make things more clear.

Nothing here is blocking a checkin, so I can do that now, but if you want to fixup the small issues first, I'll say do that and then just check it in :)

On a side note I like the notation of "Gets or sets ..." on getter/setter properties, though I can understand the amount of rework would be a bit much, but might be worth considering in the future

btw. I had no problems running the game+patch after installing physx on my system, worked like a charm :)
Jan 21, 2008 at 10:31 PM
Changeset 9206 has been uploaded. ActorDesc.cs and Terrain.cs have been modified according to comments. In Terrain.cs, I just went ahead and put "this." in front of every instance of a class member variable so finish off that file.

Also, for the QuickStartWindows.sln solution, the PhysX assembly will be taken from the Release directory for all builds. This should fix xnasorcerer's issue, if its what I think it is. The QuickStartWindows_All.sln solution will continue to use the proper build configuration (Debug game -> Debug PhysX, Release game -> Release PhysX).

I did an update before committing, so this shouldn't overwrite Sturm's latest change set like last time. :)
Jan 22, 2008 at 6:22 AM
It looks al ok, and I can certainly live with release build of the PhysX integration, I'm going to leave all that phys debugging up to you :)
Jan 22, 2008 at 7:29 AM
Just ran it on my firegl system and performance looks good, it initially drops to 35 fps, but after a 1 or 2 gets up around 140fps. The test was run in release mode without a debugger, and I just watched the boxes fall, no interaction.

Though later I was moving around and I noticed that some of the boxes were stuck in the terrain, I created this Entities sink into terrain issue for that.
Jan 22, 2008 at 1:27 PM
Interesting, I've never observed that on my system. I wonder if it has something to do with LOD levels on the terrain.
Coordinator
Jan 22, 2008 at 4:01 PM
Probably has to do with the LOD.

I'm leaving all the physics calculations right now using the verts of the full LOD. I can certainly change that however, and it sounds like I will have to.
Jan 22, 2008 at 6:15 PM
The physics heightfield is generated from whatever you put into heightData.
Coordinator
Jan 22, 2008 at 7:36 PM
Well the problem is that the data that we choose to use from the heightfield needs to correspond to the LOD.

Heightdata holds every vertex, but at lower LOD we don't draw every vertex, so at very low LOD or a high scaling factor, or both, we could see issues.

It sounds like at different LODs we need to give the physics heightfield different data.
Jan 22, 2008 at 9:24 PM
Ideally we'll keep the physics data at the lowest possible LOD that doesn't generate bad artifacts. This will give us large speed/memory savings.
Coordinator
Jan 22, 2008 at 11:13 PM
Maybe one LOD below whatever the terrain is at for rendering? Up to a point, maybe once terrain is at 1/4th or 1/8th detail we would just use the same for physics.
Jan 22, 2008 at 11:15 PM
Shaw you have a few patches open, could you mark them as Applied/Declined so that we can clear the list for the 0.19 release?
Jan 22, 2008 at 11:58 PM
Done.
Jan 23, 2008 at 3:46 AM
It's starting to look like a upcoming release :)
Jan 23, 2008 at 4:14 AM
Despite my best efforts to prevent it. :)

Are we punting networking until 0.20? mikelid109 seems MIA.
Jan 23, 2008 at 10:06 AM
Well, I just tried now the new changeset with the physics scene. There must be something wrong with it. Using the SampleScene the fps initially drops to 6 fps, but after the box appear to stop moving, fpg goes to 18 fps only. Without your SampleScene fps stays between 90 and 140.

I also tried this other Ageia Physx wrapper that works with XNA, and I increase the number of spheres to be created. It only starts to drop after I have lots and lots of spheres. Look here: http://img187.imagevenue.com/img.php?image=82435_teste_122_248lo.jpg
Jan 23, 2008 at 3:29 PM
What processor do you have? I'm not doing any CPU detection at the moment, so the physics code is set up to try to use two cores. If you only have one core, you may be seeing some threading overhead.

How many spheres are you getting in your demo with the wrapper? For the QuickStartSameGame demo, it has 900 boxes which are more expensive to perform collision detection than spheres, and it has the heightfield collision detection to handle. In other words, the scene in the sample game is much more complex than having spheres bouncing around.

In a Release build, I get 150 FPS with the default scene. If I change it to spheres I get over 160 FPS.

To increase the speed, just go into SampleScene.cs and change the first parameters to the BoxGrid call to something more managable like 10 or 15. That parameter is the edge size of the grid, so you will get a size x size grid of boxes.
Jan 23, 2008 at 9:35 PM
I'm running on a P4 3.4ghz with 2gig ram and I get around 60-80 fps if I just watch the boxes fall. That's using release mode no debugger attached.
Jan 23, 2008 at 10:19 PM
Sturm, if you wouldn't mind, could you run a quick test for me? In PhysXScene.cpp, comment out the following lines:

    desc.flags |= NX_SF_ENABLE_MULTITHREAD;
    desc.internalThreadCount = 2;
    desc.backgroundThreadCount = 2;

And then repost your FPS. I'm curious how much of a overhead the threading is on a single-core machine. Eventually I will do a check to see the number of available cores and scale accordingly.

Still, 60-80 FPS isn't bad considering that's 900 boxes (if you didn't change anything) on a 4096x4096 unit heightfield, with a planar configuration of boxes which implies no spatial locality on the heightfield testing.