Board index FlightGear Development Canvas

Canvas display property I/O

Canvas is FlightGear's new fully scriptable 2D drawing system that will allow you to easily create new instruments, HUDs and even GUI dialogs and custom GUI widgets, without having to write C++ code and without having to rebuild FlightGear.

Canvas display property I/O

Postby Thorsten » Sat Mar 18, 2017 2:01 pm

For anyone suffering from canvas avionics performance impacts, the following might be interesting.

Disclaimer: I guess I'll generally suggest things which are not considered 'elegant' coding - I do this because they're fast.

Fairly typical situation, we have a display and run an update loop to adapt it to the internal state of the simulation. The basic structure of the individual updates is something like

Code: Select all

var my_variable = getprop("/sim/systems/my_variable");
my_canvas_element.setText(my_variable);


(could also be setTranslation() for a scale, or setRotation() for a gauge,..., anyway, you fetch a number, you update a canvas element with it.

Now, most of the performance impact of this is not in how canvas assembles the texture or how that texture gets rendered, it's in how Nasal interacts with the property tree. Fast canvas code has to minimize property I/O.

To gauge that, take the cost of a setprop/getprop to be 1. Then the relative cost of using props.nas if you have a pointer to the node via

Code: Select all
var val = node.getValue();


is about 3. The relative cost of props.nas if you need to get a pointer to the node via

Code: Select all
var val = props.globals.getNode(my_node_name).getValue();


is about 10. In contrast, the relative cost of fetching something from Nasal, for instance

Code: Select all
var val = MyAircraft.guidance.target_array[1];


is pretty much zero - it's unmeasurably small.

So the first thing to do is that your getter method of data should always use getprop(), then the relative cost of the update code snippet above is 4 (1 for the getter, and 3 for setText which uses props.nas internally). Avoid props.nas for getter methods if you don't need to (usually you do not), and never discard the pointers to the node once you have them.

As a side note, if you have something in Nasal already (because the system you simulate happens to be implemented in Nasal) never go via the property tree to pass it to a canvas display, always pass it directly.

This leaves the setter. If you have a sizable performance issue, it is possible to by-pass the canvas API to use setprop() also for writing, cutting the total performance cost of the update down to 2.

I'd not recommend this generally though. In many instances, you can do other speedups.

1) If the getter refers to a value which you use in many places in the same frame (things like attitude or airspeed come to mind...), use a data provider - have a dedicated loop execute all getter functions you need once per frame and store the result in a Nasal data structure, use that Nasal data structure inside the canvas update loops.

Say you use airspeed in 5 different locations, then the cost of the getter function is no longer 1 but effectively 1/5.

2) If you have a status update, like an AP mode label, update it only when needed. For instance you can store a string with it

Code: Select all
my_label.lasttext = "";


and then, rather than setText, do

Code: Select all

var updateText = func (node, text) {

if (text == node.lasttext) {return;}

node.lasttext = text;
node.setText(text);
}



and in most frames, you'll never pay the cost for the setter function. Of course that performs worse with any elements which do change every frame, so you have to know what kind of element it is you're updating.

(It's also possible to set these with dedicated functions and pull them out of the update loop altogether, but that makes for rather messy code imo).

So, a well-organized property I/O might in an ideal case run some 4 times faster than a non-optimized code. It's unlikely that you'll always get that much, but it's not negligible either.

Enjoy speeding up your displays.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Canvas display property I/O

Postby Hooray » Sat Mar 18, 2017 4:15 pm

Yeah, as usual you are figuring out stuff on your own that other people have been using already for the exact same reasons - I think I wrote a handful of wiki tutorials using this technique for optimization purposes - e.g. referring to:

http://wiki.flightgear.org/Howto:Reset/ ... leshooting
Image
Code: Select all
if (currentText == neededLabel) continue;
        test.status.setText(neededLabel);


The point being, it is unfortunate to encourage people to use such workaround - even though I do think your approach of encapsulating this is much better than my hackish code.
What would be better, if someone with commit access to fgdata would edit api.nas and either extend setText() accordingly to support an optional parameter doing this properly, or simply adding a new updateText() API that would do this transparently behind the scenes.

The rationale being that we should definitely encapsulate such coding patterns using a single point/place, one that is maintained by the FlightGear project (i.e. fgdata side) - so that potential future updates/optimizations can be more easily propagated without requiring aircraft/fgdata developers to manually edit tons of their existing code.

If/when this is done, we would only ever have to edit api.nas and locate updateText() to map directly to a corresponding native C++ call and/or property update.

A while ago when we covered property I/O being the bottleneck when it comes to Canvas performance, I specifically mentioned how this should not go through Nasal space at all, and how this could be easily optimized by edit CanvasText to directly support labels that are retrieved using other properties - e.g. by having an element-specific "text-mode" property, where the value would either be an ASCII string representign the actual text or a property path.

Equally, for pretty much the same reasons, I suggested that we introduce a mode that directly supports sprintf-style format strings so that CanvasText updates would never have to do this sort of thing in Nasal space, which can quickly add up (unnecessarily).

Background quoted below:

Subject: Space Shuttle

Hooray wrote:The Avidyne code is using such an abstraction - personally, I am more inclined to think in terms of building blocks, which does not mean that any of your existing code needs to be rewritten from scratch, but should be encapsulated in a fashion so that common coding constructs (such as sprintf/getprop idiom) are put into a helper function, which can later on be re-implemented/optimized, without having to touch tons of files/functions.


Subject: [SOLVED] Canvas and properties
Hooray wrote:The most efficient method would be using a simple helper-framework like the one Thorsten mentioned (e.g. using Richard's MFD work), but preparing it to be replaced/augmented through native hooks eventually - i.e. it would be relatively simple to hack the Canvas:Element code to also support property values, i.e. values retrieved from other properties - that would minimize the degree of required Nasal code rather significantly, especially in conjunction with supporting sprintf-style format strings - but that would require some c++ changes, for the corresponding background info, see:

viewtopic.php?f=71&p=266721#p266721


Subject: Space Shuttle
Hooray wrote:Part of the reason why the extra500 is/was (haven't checked in a while) such a resource hog is indeed the way its massive display is organized and the way it is updated - while some things could certainly be optimized, the typical shuttle display will have more or less a 1:1 mapping from a sprintf/getprop() call to a setText() equivalent which maps to OSGText nodes, in the case of most modern MFDs, i.e. those on the extra500, but also the PFD/ND on an airlienr, that is no longer the case - which I think you can see when looking at some of the Canvas.Path-heavier "pages", or the trajectory map.


Additional info to be found by searching for "Canvas + sprintf": search.php?st=0&sk=t&sd=d&sr=posts&keywords=sprintf+canvas
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 12707
Joined: Tue Mar 25, 2008 9:40 am
Pronouns: THOU

Re: Canvas display property I/O

Postby Thorsten » Sat Mar 18, 2017 4:53 pm

What would be better, if someone with commit access to fgdata would edit api.nas and either extend setText() accordingly to support an optional parameter doing this properly, or simply adding a new updateText() API that would do this transparently behind the scenes.


I think the important point is that much is special purpose code. You're creating overhead in one place to reduce workload in another. If your use case is structured such that you update rarely, you gain. If your use case is something that actually changes per frame, you lose.

Which is why I'm somewhat hesitant to commit such things more generally (I'm also working on a setTextFast() method which uses a stored property path and setprop() for the job) because people need to know what they're doing when using these.

(One simple example is that the current api.nas seems to do some sanity checks - which I'd dispense with in high-performance code).

Well, having said that, I don't mind adding these to the canvas API if people think that's a good idea.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Canvas display property I/O

Postby Hooray » Sat Mar 18, 2017 4:56 pm

I see, but introducing a new updateText() method with said behavior should be a safe thing, because it would be unused - i.e. we could document it after providing it, and before people start using it to explicitly make it clear what it is all about -as a matter of fact, even the update semantics could be encoded as part of a sufficiently high-level API, using either a timer or listener that will do this transparently, while also being aware of the actual update frequency.

Obviously, we would also review the ::Element update() routine to specifically look for problematic use-cases, i.e. frequent and/or redundant updates and notify the user/aircraft developer accordingly, which is touching on the whole idea of having a "developer mode".
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 12707
Joined: Tue Mar 25, 2008 9:40 am
Pronouns: THOU

Re: Canvas display property I/O

Postby Thorsten » Sun Mar 19, 2017 8:17 am

Okay, after sleeping over it, I propose to do the following:

Both updateText(string) and setTexFast(string) need to store additional strings - the former the last text that was written, the latter the path to the node to set.

Since I don't want to have any checks for existence of strings inside the methods for performance reasons, I would supply two methods

enableUpdate() and enableFast()

which create the infrastructure (i.e. attach a lasttext and nodepath key to the text hash) which need to be called after creating the element.

The actual setter methods will then rely on this being there (and you will get a crash if you use them without enabling then first).

Does that sound feasible to you?
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Canvas display property I/O

Postby Hooray » Sun Mar 19, 2017 11:11 am

note that if you do that, you still need to check if enableUpdate/Fast have previously been executed to set up the hash accordingly unless I am missing an important implementation detail.

However, what you could do is to procedurally add those methods and otherwise remove those hash members or keep an empty placeholder (untested code below):

Code: Select all

var Foo = {

new: func() {
var m = {parents:[Foo]};
return m;
},  # new

enableUpdate(): {
me.updateText = func() {
}; # updateText()
}, # enableUpdate

updateText: func() {
die("updateText() requires enableUpdate() to be called first");
},

}; # Foo


(hope that makes sense, if I missed something, we may need call/compile here - but that would also work to do away with unnecessary conditionals)
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 12707
Joined: Tue Mar 25, 2008 9:40 am
Pronouns: THOU

Re: Canvas display property I/O

Postby Thorsten » Sun Mar 19, 2017 11:22 am

Currently I have (as proposed methods for the text class)

Code: Select all
  # enable reduced property I/O update function
  enableUpdate: func
  {
    me._lasttext = "INIT_BLANK";
  },
  # Update text, this is run only if the function argument is different from the pre-set one
  updateText: func (text)
  {
    if (text == me._lasttext) {return;}
    me._lasttext = text;
    me.set("text", typeof(text) == 'scalar' ? text : "");
  },
  # enable fast setprop-based text writing
  enableFast: func
  {
    me._node_path = me._node.getPath()~"/text";

  },
  # fast, setprop-based text writing
  setTextFast: func(text)
  {
    setprop(me._nodepath, text);
  },


If you try to e.g. setTextFast without calling enableFast before, you'll get

Code: Select all
Nasal runtime error: No such member: _nodepath


Basically I don't want to have a validity check anywhere inside setTextFast because that's supposed to be performance-critical. I guess your solution is somewhat clearer on the error messages it creates.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Canvas display property I/O

Postby Thorsten » Sun Mar 19, 2017 11:41 am

Okay, this seems to be working as expected:

Code: Select all
 # enable reduced property I/O update function
  enableUpdate: func ()
  {
    me._lasttext = "INIT_BLANK";
    me.updateText = func (text)
    {
      if (text == me._lasttext) {return;}
      me._lasttext = text;
      me.set("text", typeof(text) == 'scalar' ? text : "");
    };
  },

  updateText: func (text)
  {
    die("updateText() requires enableUpdate() to be called first");
  },
 
  # enable fast setprop-based text writing
  enableFast: func ()
  {
    me._node_path = me._node.getPath()~"/text";
    me.setTextFast = func(text)
    {
      setprop(me._node_path, text);
    };

  },
  # fast, setprop-based text writing
  setTextFast: func (text)
  {
    die("setTextFast() requires enableFast() to be called first");
  },


It has descriptive errors, so I like it. If you don't have any objections, i'd go with that then.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Canvas display property I/O

Postby Hooray » Sun Mar 19, 2017 12:24 pm

thanks for trying this, I am not able to test any of this currently, but it's looking good - so if it's working, I don't see any problems.
My suggestion would be that you add a link to this topic, or maybe copy your posting into the commit message so that if/when TheTom sees those changes, he can see immediately what this is all about - also, this would be the right place for us to review/optimize if/when the Canvas::Text element is updated accordingly (e.g. to support sprintf-style format strings for labels that may need to look up/format multiple properties to create a corresponding string).

We could also add a link to the corresponding docs so that these can be updated/maintained once those APIs are touched: http://wiki.flightgear.org/Canvas_Text

note though that you ended up using enableFast() whereas I named the whole thing enableUpdate() apparently.

EDIT: Looking at your proposed changes, I would suggest to explore adding a defaulted argument (or new method) to provide a vector of properties to be fetched and mapped into the sprintf-style string - that way, all the MFD use-cases using multiple getprop() calls to populate a formatted string could be encapsulated using a single method, that is part of api.nas - so that this can be much better isolated and optimized in the future.

(in theory, returning a function wrapped in a closure may be cheaper than doing the me.field lookup, but cannot test this atm)
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 12707
Joined: Tue Mar 25, 2008 9:40 am
Pronouns: THOU

Re: Canvas display property I/O

Postby sanhozay » Sun Mar 19, 2017 12:55 pm

Just suggestions ...

Is there a reason for "INIT_BLANK" rather than nil?

Could the updateText function that gets replaced call enableUpdate to replace itself, or does it recurse infinitely if you do that?
sanhozay
 
Posts: 1207
Joined: Thu Dec 26, 2013 12:57 pm
Location: EGNM
Callsign: G-SHOZ
Version: Git
OS: Ubuntu 16.04

Re: Canvas display property I/O

Postby Hooray » Sun Mar 19, 2017 1:24 pm

Using this technique, of dynamically replacing methods after running the heuristics/conditionally, you could obviously obtain pretty much any desired behavior - including methods that may do self-setups, the question is how far you want to go with this - for instance, at some point you are approaching the situation where just calling an API has very different parameter requirements compared to setting it up in the first place (imagine the sprintf/getprop use-case where you need to fetch a vector of properties via getprop to populate a formatted string)
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 12707
Joined: Tue Mar 25, 2008 9:40 am
Pronouns: THOU

Re: Canvas display property I/O

Postby Thorsten » Sun Mar 19, 2017 2:33 pm

Is there a reason for "INIT_BLANK" rather than nil?


It's a string, whereas nil is not. Since Nasal has no variable type declaration, I habitually init things with a placeholder of the right type to set the variable type early on.

And "" doesn't work, because that's a string that actually needs to be written fairly often (I had a problematic case immediately), so I just picked a string that's unlikely to occur in any real use case.

Could the updateText function that gets replaced call enableUpdate to replace itself, or does it recurse infinitely if you do that?


Possibly... Though it'd somehow first replace itself and then call itself after it has replaced itself (otherwise the first call will never be made) and that may not be unproblematic. The current setup rather forces people to sit and think whether they need it.

I would suggest to explore adding a defaulted argument (or new method) to provide a vector of properties to be fetched and mapped into the sprintf-style string


I actually don't think having the getter function canvas-side is a good idea. What is needed there/works best depends on what system you're trying to display - you may want to run unit conversions, interpolations, string conversions, data providers,...

note though that you ended up using enableFast() whereas I named the whole thing enableUpdate() apparently.


They're two rather different methods - one for properties which update rarely, one for properties which update basically every frame. You're supposed to use only one, doesn't make overly much sense to use both.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Canvas display property I/O

Postby Hooray » Sun Mar 19, 2017 2:41 pm

ok, sorry I missed that (only been skimming) - regarding your point on keeping the update semantics separate, that is a valid concern - we were facing the same issue with the ND code that ends up calling MapStructure as its back-end, which ultimately means passing tons of structures/hashes with embedded callbacks around to implement IoC (Inversion of Control) - if you have ever used Nasal's sort() or cmp() functions you should be familiar with the concept:

Code: Select all

var someCheck = func() return 0;


var updateSomething(condition, value) {
# call the callback contraining the condition:
if( condition() ) {
print(value);
}
else {
print("condition failed");
}
}

updateSomething(someCheck, "Hello World");



(again, hope that makes sense - this coding pattern is basically what "delegates" (the design pattern) is all about - you delegate control out of the code implementing it, so that the caller needs to pass callbacks implementing the behavior, no matter if that means conditionals/heuristics or other stuff)
Please don't send support requests by PM, instead post your questions on the forum so that all users can contribute and benefit
Thanks & all the best,
Hooray
Help write next month's newsletter !
pui2canvas | MapStructure | Canvas Development | Programming resources
Hooray
 
Posts: 12707
Joined: Tue Mar 25, 2008 9:40 am
Pronouns: THOU

Re: Canvas display property I/O

Postby Thorsten » Sun Mar 19, 2017 3:27 pm

Okay, I've just pushed it 'as is' and will re-write the Shuttle PFD code to use the new methods as an example (and canvas seems to be still working...) - expect that to be pushed by tomorrow into the Shuttle devel branch.

I think if needed the functions can always be extended and made more sophisticated.

If upon testing there's no further problems emerging, I'll add the documentation to the Wiki.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am


Return to Canvas

Who is online

Users browsing this forum: No registered users and 1 guest