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

Bullet memory #2176

Closed
wants to merge 2 commits into from

Conversation

@jrenner
Copy link
Member

jrenner commented Jul 29, 2014

I've been doing lots of raycasting in bullet (see: http://www.java-gaming.org/topics/fps-engine/33931/view.html) and ran into problems where the bullet wrapper was creating a lot of garbage when getting raycast ray positions and hit normals due to btVector3 new object allocation.

It's currently not possible to get the hit normals without creating a new btVector3 java object. So I have implemented a static temporary object, and also made it publicly available with the appropriate java-doc warning that it is for low-level use.

Then I went through some raycast callbacks and modified their get methods to return the temporary variable.

If we don't feel good about returning the temporary variable, I'd like to keep the static btVector3.getTemp(), and then change the get methods to something like this:

public btVector3 getHitNormal(btVector3 out) {
    ...set values and return
}

then it's easy enough to grab btVector3.getTemp() and pass it to the method.

Both solutions solve my garbage collection problem.

@xoppa, let me know what you think.

@jrenner

This comment has been minimized.

Copy link
Member Author

jrenner commented Jul 29, 2014

I now see there is a problem with returning the temp object: it should not be disposed. The user won't know which methods are returning the temp object, so this is bad. I think the best solution is the alternative I suggested at the bottom of the pr description. I'll wait to hear what @xoppa says before I make any changes, though.

@xoppa

This comment has been minimized.

Copy link
Member

xoppa commented Jul 29, 2014

The bullet java files are generated using swig. If you modify them directly, then the modification will be removed when swig is run the next time.

The wrapper should use Vector3 instead of btVector3, where the Vector3 object is either always the same (for return values) or pooled (for callback arguments), see also: https://github.com/libgdx/libgdx/wiki/Bullet-physics#common-classes. So I guess that the real issue is here that the wrapper didn't use this conversion. I'd have to look into this, but it's probably the method signature (e.g. a const pointer). In that case it might, indeed, be better to add a signature accepting a Vector3 (not btVector3) instance, like this https://github.com/libgdx/libgdx/blob/master/extensions/gdx-bullet/jni/swig/collision/btCollisionObject.i#L148

Note that this is related to #2174

@xoppa xoppa closed this in 65d8a95 Jul 29, 2014
xoppa added a commit that referenced this pull request Jul 29, 2014
@jrenner jrenner deleted the jrenner:bullet-memory branch Jul 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.