Revision 63362 and breaking changes

Nov 5, 2011 at 11:27 AM

This revision broke several things, some good, some not so.

Change: Transformable no longer contains BoundingBox. Now we have IBoundable instead.

Comment: Probably good, seems logical. Easy to update, I was using a dummy value anyway.

Change: PropertyExpression is now PropertyExpression<T>.

Comment: Probably good. Not sure why, but probably reasonable. Easy to update (it wasn't public before).

Change: DrawableModelContent now has Source instead of Model, and Model is not public.

Comment: Probably reasonable, but (a) requires multiple updates in xaml files and (b) making Model non-public completely breaks the method I was using to clone objects. Impossible to update, so I added the following to DrawableModel.cs.

        public Microsoft.Xna.Framework.Graphics.Model Model { get { return model; } }

Change: Major changes to all API calls to find scene objects (see example below).

Comment: I assume this is some kind of optimisation, although I don't see any improvement on my testing. The new API calls are far less convenient. The updates are mechanical, but each single line is now 2-4 lines of code. I see no reason why some or all of the previous calls could not be implemented as a layer on top, for those who prefer to use them.

// from this
        IEnumerable<T> FindAll(Ray ray);
// to this
        void FindAll(ref Ray ray, ICollection<T> result);

There may be other breaking changes, but this is all that affected me.

Coordinator
Nov 5, 2011 at 4:01 PM
Most of these changes are related to garbage allocation.

Transformable -> No BoundingBox:
The previous implementation is a bit hacky to deal with infinite objects like skybox. In addition, removing ISpatialQueryable from Transformable also reduces the pressure on scene manager, as each ISpatialQueryable will have to be managed by the scene manager, now DisplayObject don't need to be added to scene manager too.

PropertyExpression -> PropertyExpression<T>:
Getting and setting the Value property (which is an object) would cause boxing and unboxing for value types, which in turn allocate a piece of garbage everytime you access the value property. The generic version avoids that.

DrawableModel.Model:
I did an experiment to remove the Drawable prefix for surfaces, models but forgot to revert that change. Thanks for pointing out :D

IEnumerable<T> -> ICollection<T>:
It requires a whole blog to fully describe why using IEnumerable<T>, Linq, yield, lamba will alway create garbage and why foreach on a List<T> would not. I agree to build a layer on top of that to simulate the existing behavior for convinence, just didn't got the time, maybe you can help that out :D

I'll continue to work on performance optimization recently, some hotspot that I discovered haven't been fixed yet.
Nov 6, 2011 at 9:00 AM
Edited Nov 6, 2011 at 9:00 AM

OK, I'm fine with all that.  I would encourage you to mention breaking changes in checkins -- it all came as a bit of a surprise. One approach I have used is to have a set of unit tests that simply call everything in the public API.

Re FindAll: The only use case I currently have is to use a Ray to find a specific visible object, usually the one closest to the camera or a weapon. The reason I liked the previous API is that it was possible to write a simple Linq query to match that use case. It seems to me now that the only way I can do that is as follows (commented out code is the old way).

 

    DrawableModel PickModel(Scene scene, Ray ray) {
      var results = new List<DrawableModel>();
      scene.FindAll<DrawableModel>(ref ray, results);
      var model = results.OrderBy(dm => dm.Intersects(ray)).FirstOrDefault();
      //var model = scene.FindAll<DrawableModel>(ray).OrderBy(dm => dm.Intersects(ray)).FirstOrDefault();
      return model;
    }

As far as I can see that's still much the same amount of garbage. I only want one result, but I have to retrieve a collection, sort it and pick one. Ideally you would have a lower-level function that could do that without ever building or returning a collection. Perhaps you can think of some other use cases, but that's all I have.