Canvas ADI ball (shuttle) / circular clipping

[Hooray]

Note: I haven't actually read beyond your first sentence in your last response, because I didn't really get the impression that you were trying to be particularly helpful - so to prove my good will, I have done what you asked me to do, and adapted one of the Canvas snippets to randomly draw 3000-6000 lines to a GUI dialog.

http://wiki.flightgear.org/Howto:Canvas ... nchmarking


I am hopeful that we can adapt this to turn this into a stress-test/benchmark to see if, what and how this can be optimized (and yes, I am willing to also look at the C++ side of this).

For starters, it would be great if you could take a look and adapt this so that this makes sense in terms of the approach/use-case you have in mind (e.g. numbers, primitives added to the scene, and workload created by this).

Next, my suggestion would be that you provide your own test function that implements the logic you are currently using - which would tell us if there are any low-hanging fruits or not, and if you are already using the fastest method.

[Thorsten]

I haven't actually read beyond your first sentence in your last response

Maybe you should have? Because the 'what are we trying to achieve here' is not still really clear to me.

[Hooray]

I have meanwhile, sorry - your response was more constructive than I anticipated after the introductory "banter"

With that being said, I have taken the code posted in the wiki and reworked it a little (so far without any of your feedback) and have also executed the whole thing using the built-in profiler, which confirms that the property I/O is the bottleneck - and in fact, the problem is that it all goes through props.nas largely, which adds several layers of indirection (unnecessarily) - the setValues() and setChildren() logic can be easily expressed in C++ space using ~50 lines of code (all of which already exists in nasal-props.cxx).

I think that this would be a particularly low-hanging fruit to optimize this particular use-case - in the long-term, it would however make sense to port all/most of props.nas and move it back into C++ space for all the reasons that James, TheTom and Philosopher originally suggested (using Nasal/CppBind).

Apart from that, there are also Canvas/Element-specific optimizations possible, such as reusing the Canvas.Path node/element when resetting those paths.

Overall, it would also make sense not to set hundreds/thousands of properties, but really just build a single SVG path/command stream and set a single property and take it from there.
In summary, I don't think we've hit any "hard" limitations so far.

http://wiki.flightgear.org/Howto:Canvas ... rking#Code


PS: I was trying to come up with a reproducible test case to easily benchmark this and profile the underlying code, to be sure that we don't just know that this is "property related", but that there are no bugs like the leaking effects listeners contributing to this.

[Hooray]

We don't want to hide it - we want it gone and re-load a new path.
[...]
Is there then any particular reason why you tell me in some length how to hide groups?

Like I said previously, with the remove[All]Children() idiom being so unfriendly to the way the Canvas system works behind the scenes and in the light of the way the props.nas stuff is adding to the workload (as per the diagram shown above), it may be worthwhile to simply hide the previous "plot" group, and then re-assign the Nasal variable referencing it - at that point, it would be invisible, i.e. not processed by the Canvas system (it should simply ignore those groups/paths), and once the variable is reassigned, there should not be any GC references anymore, at that that point, the removeAllChildren() logic should no longer be necessary, because the Nasal garbage collector could take care of things by removing the naGhosts/SGPropertyNode handles directly.

[Thorsten]

and in fact, the problem is that it all goes through props.nas largely,

props.nas is pure evil for high throughput. I'm surprised it's being used at all.

[Hooray]

it is being used unnecessarily actually - the Nasal code is basically turning hashes into properties - it's roughly 30 lines of Nasal code that could be easily ported to C++ at the mere cost of reviewing nasal-props.cxx:

When we call setData(), it will do this:

Code: Select all

setData: func(cmds, coords)
  {
    me.reset();
    me._node.setValues({cmd: cmds, coord: coords});
    me._last_cmd = size(cmds) - 1;
    me._last_coord = size(coords) - 1;
    return me;
  }


The reset() call will do this:

Code: Select all

# Remove all existing path data
  reset: func
  {
    me._node.removeChildren('cmd', 0);
    me._node.removeChildren('coord', 0);
    me._node.removeChildren('coord-geo', 0);
    me._first_cmd = 0;
    me._first_coord = 0;
    me._last_cmd = -1;
    me._last_coord = -1;
    return me;
  },


The setValues() stuff is doing this:

Code: Select all

##                                                                             
# Useful utility.  Sets a whole property tree from a Nasal hash               
# object, such that scalars become leafs in the property tree, hashes         
# become named subnodes, and vectors become indexed subnodes.  This           
# works recursively, so you can define whole property trees with               
# syntax like:                                                                 
#                                                                             
# dialog = {                                                                   
#   name : "exit", width : 180, height : 100, modal : 0,                       
#   text : { x : 10, y : 70, label : "Hello World!" } };                       
#                                                                             
Node.setValues = func(val) {                                                   
    foreach(var k; keys(val)) { me._setChildren(k, val[k]); }                 
}                                                                             
                                                                               
##                                                                             
# Private function to do the work of setValues().                             
# The first argument is a child name, the second a nasal scalar,               
# vector, or hash.                                                             
#                                                                             
Node._setChildren = func(name, val) {                                         
    var subnode = me.getNode(name, 1);                                         
    if(typeof(val) == "scalar") { subnode.setValue(val); }                     
    elsif(typeof(val) == "hash") { subnode.setValues(val); }                   
    elsif(typeof(val) == "vector") {                                           
        for(var i=0; i<size(val); i+=1) {                                     
            var iname = name ~ "[" ~ i ~ "]";                                 
            me._setChildren(iname, val[i]);                                   
        }                                                                     
    }                                                                         
}

It would not take much effort to get rid of the overhead caused by this and move everything happening here to C++ space - nasal-props.cxx already has all the code we need to do this sort of thing, it's just not used at all. Besides, Philosopher actually implemented support for more efficient indexing lookups to get rid of the concatenation/string-building overhead, and not even that is currently being used by props.nas

When it comes to property tree access performance, getting rid of props.nas, or lateast re-implementing the workhorses there, is one of the lowest hanging fruits, that would not even be specific to the Canvas system, i.e. all systems going through props.nas would benefit from a reimplementation using cppbind.

When it comes to Canvas, it would be possible to specifically cache Path elements and their nodes and reuse the instead of using the removeChildren() - i.e. a simple form of resource management, by keeping a configurable average of nodes around before deleting them entirely.

[Hooray]

You'll see all the invariant parts of the ADI (including those that can be animated by translations/rotations) created in the ondisplay part and only the parts which can't be done that way in the update part of the display methods.

Not being familiar with the shuttle and its ADI ball: is there any way you could tackle the problem in such a way that you merely project a set of textures onto the Canvas by using transformations that are specific to the current position/attitude etc (e.g. imagine having 1..n stacked textures with their own z-index) ?

The background being that fully rendering the sphere's grid once to a set of background textures and picking (extracting textures from) those would be much cheaper in comparison to the property I/O that is currently created - which is why we originally wanted to use actual UV-mapped 3D models and merely change the attitude of the 3D model.

There's also code in the wiki to use actual OSG "shapes" as Canvas elements:

http://wiki.flightgear.org/Canvas_Sandbox#CanvasShape


In other words, this could load an actual texture from disk - so you would not even need to use the Canvas system to create it (even though you could).

(Admittedly, the 3D model based patch would be much more generic and much more useful than just having the capability to render/reposition a textured sphere, but it would be comparatively straightforward from a C++ standpoint ...)

[Thorsten]

Not being familiar with the shuttle and its ADI ball: is there any way you could tackle the problem in such a way that you merely project a set of textures onto the Canvas by using transformations that are specific to the current position/attitude etc (e.g. imagine having 1..n stacked textures with their own z-index) ?

Not without making it very jumpy or requiring some 300+ textures (which someone would need to generate in the first place...)

I'm open to any contributions from people who want to test that, but I have absolutely no time right not to work on the ADI myself and try work-intensitve things.