Modifying Geom Vertices

Topics: Developer Forum
Aug 8, 2009 at 9:08 AM
Edited Aug 8, 2009 at 9:11 AM

It should be possible to modify Geom vertices on the fly (at least in small increments) if using the SAT narrow phase collider. I don't know what the recommended way of doing this is, but my current best solution goes something like this:

private void GrowGeom(Geom geom, Vector2 scale)
{
    geom.LocalVertices.Scale(ref scale);
    //HACK: Doing this so that the private Update() method is called on the geom.
    geom.Matrix = geom.Matrix;
}

 

Notice the hack I have in there. The issue is that I'm modifying the geometry's local vertices list directly, but that is invalidating other state in the geom, such as its world vertices and its AABB. In order to fix the geom state, as far as I can tell, all that needs to be done is to make a call to the geom's private Update() method. Since it's private, I cannot access it directly, which is why I have the last line in there. The last line indirectly calls Update(). This works, technically, but is obviously not the way it should be done. I'd love to be shown "the right way" to do it, but in case there is no better way, here is my proposed solution:

Make Update() public, and rename it to something like RebuildWorldVertices(). Comment description could be something like "Reconstructs the geometry based on its current LocalVertices. Call this whenever the LocalVertices have been modified, in order to synchronize the WorldVertices."

On a similar note, I believe there is an issue with SetVertices as well. Below is the current implementation:

/// <summary>
/// Sets the vertices of the geom.
/// </summary>
/// <param name="vertices">The vertices.</param>
public void SetVertices(Vertices vertices)
{
    vertices.ForceCounterClockWiseOrder();
    localVertices = new Vertices(vertices);
    worldVertices = new Vertices(vertices);

    AABB.Update(ref vertices);
}
This is probably how it should be:
 
/// <summary>
/// Sets the vertices of the geom.
/// </summary>
/// <param name="vertices">The vertices in local coordinates.</param>
public void SetVertices(Vertices vertices)
{
    vertices.ForceCounterClockWiseOrder();
    localVertices = new Vertices(vertices);
    worldVertices = new Vertices(vertices);    //Line 5
    Update();
}

Note the slightly more explicit vertices comment.

Update() both fixes the worldVertices relative to the localVertices as well as calling AABB.Update(ref vertices).

Line 5 could also potentially be removed if Update() was made to initialize the worldVertices. No need to iterate the list twice.

Something else I just stumbled upon is that Geom's private void ConstructClone(...) method does not appear to clone the localVertices, so if one were to use this method and then attempt to deform one of the geometries, all clones would have their localVertices changed (but we would get strange results since we wouldn't be calling Update() on all of the clones).

I'm new to participating on CodePlex, but I'm up for helping out with any of this if you'd like. I just don't want to be too bold and post this as a work item right away without some reassurance that I'm interpreting the code properly. Thanks!

Coordinator
Aug 8, 2009 at 5:43 PM

I appreciate you took the time to digg into the implementation and look around. Farseer Physics was not designed to support growing/shrinking geometries. We only have the potential since we got SAT implemented.

To scale the geometry you should use SetVertices() - I know you might think that there is a overhead from giving it the localvertices in a scaled version instead of just scaling the localvertices directly. But the current system is created for performance, and even the smallest change can make the whole thing become very unstable or very bad in performance.

Example: If you were to move the line you noted with Line 5 into Update, it would get called each time you create a new geometry (with DistanceGrid) and each timestep (because of Update(vector2,float) ). That would create a huge amount of garbage and the engine would slow down to a crawl on slow systems.

As for the ConstructClone, it does indeed clone the localVertices. The line SetVertices(geometry.localVertices); copies the localvertices from the original geometry to the localvertices in the clone. Using SetVertices() we explicitly copy over the data instead of referencing it.

I appreciate you dig around and find code like this. I hope I don't come out looking like someone who think the code is perfect - because it is not. There are a lot of areas that could be better, but the engine is originally created for simplicity. We are working on 3.0 that is a complete rewrite, it will still aim for simplicity, but all the troublesome areas are going to be removed.

Aug 8, 2009 at 8:22 PM

Hi genbox, thanks for the reply. That's fair to say that Farseer wasn't meant to support modifying geometries on the fly, and I am aware that SAT is a relatively recent addition to the engine.

I wasn't aware that Update() was called every timestep. Thank you for pointing that out, and I agree that my suggestion would have given the GC lots to play with.

Sorry, I remember now that SetVertices() makes a copy of the vertex list passed in; ConstructClone is fine. :p

For my game, I still think it is best to resize certain geometries on the fly (the next alternative would be to recreate geometries every frame), and I am happy to do this with the understanding that Farseer doesn't intentionally support this, and that I will need to be careful in how I do things. All geometries that need to be resized are part of the terrain, so I won't have to recalculate the body's mass/MOI/etc. But for the SetVertices method, are you sure that it is calling the correct Update method? Calling SetVertices on its own leaves localVertices and worldVertices equal (worldVertices doesn't take into account world positiom). That isn't a problem elsewhere in the code, because everywhere SetVertices is called, SetBody is called immediately afterward (which performs an Update). The SetBody method has some additional side effects that I am not interested in, so for the time being, I think the best workaround for myself is still something like geom.Matrix = geom.Matrix; as in my first code sample. Tell me what you think.

Coordinator
Aug 8, 2009 at 9:24 PM

The Update() method also gets called by the Update(Vector2, float) method. And the Update(Vector2,float) method gets called every time a body changes position or rotation. So unless it is stationary or static, the Update() method will get called on each timestep.

You could just change the line AABB.Update() to Update() inside SetVertices(), that will update the world vertices.

Aug 8, 2009 at 9:53 PM

Yeah, these bodies were static, which is why Update() was never called. I will go with just changing AABB.Update(ref vertices) to Update(). Thanks!