Board index FlightGear Development Canvas

Fast updates of plots

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.

Fast updates of plots

Postby Thorsten » Fri Dec 30, 2016 8:38 am

Assume we have a canvas plot with a curved line which we want to re-draw - since the line changes, it's not a simple translation or rotation, but we need to re-compute the coordinates of all points that make the line.

In discussions, Hooray gave the advice a few times to not use .removeAllChildren() to remove the path element and re-draw everything but to just change the data. Previously I've had some trouble figuring out how this ought to be actually coded (largely because the .setData() method wasn't properly documented). Now I've figured out a fast way to do it. So to avoid others having to do the same experiments, here's how it goes in code:

Assume we have a path element my_graph created. Usually we'd draw a plot with

Code: Select all
.moveTo(x1, y1)
.lineTo(x2, y2)
.lineTo(x3, y3)
(...)


Now, all the canvas Nasal API really does with that command is to fill a section of the properties with two arrays - there ends up being cmd, cmd[1], cmd[2], (...) and coord, coord[1], coord[2], coord[3], (...). The coord array is twice as large as the cmd array.

The cmd array contains a number encoding what to do with the points - moveTo is a 2, lineTo is a 4.

The coord array contains the points in the sequence they arrive - coord = x1, coord[1] = y1, coord[2] = x2, and so on.

So, a fast way to update a plot without deleting it via clearAllChildren and re-plotting it is to use the setData method, a very fast way is to simply create the arrays in Nasal and to write them directly by-passing the canvas API.

Thus, create two Nasal arrays cmd and coord according to the encoding above. Create a canvas path element my_graph where you specify position, line thickness and whatever else.

Then write them using

Code: Select all
my_graph.setData(cmd, coord);


That internally goes via props.nas which is slow. If you want to utilize the faster setprop() command, instead do something like

Code: Select all
var path = my_graph._node.getPath();

for (var i = 0; i< size(cmd); i=i+1)
{
index1 = 2 * i;
index2 = index1+1;

setprop(path~"/cmd["~i~"]", cmd[i]);
setprop(path~"/coord["~index1~"]", coord[index1]);
setprop(path~"/coord["~index2~"]", coord[index2]);
}



And that's all.

(if the number of points to be plotted can vary a lot, you need some additional cleverness... but the method is very performance-friendly if you need to deal with hundreds of points).
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Fast updates of plots

Postby Hooray » Fri Dec 30, 2016 9:57 am

I haven't actually tested the following, but another possible optimization would be NOT to build the indexed property path using Nasal string concatenation (which also needs to be parsed by the corresponding SimGear code), but instead use the "new" (refined) setprop() API which supports direct indexing (implemented by Philosopher a while ago (FG 3.2+), who also posted announcements here and on the devel list):

http://wiki.flightgear.org/Nasal_library#setprop.28.29

In theory, this should use "more native code" and less Nasal code, i.e. /should/ be faster than what you're doing now (not actually tested).

Apart from that, I still think that getting rid of props.nas and reimplementing its functionality via Nasal/CppBind (native C++ code) would be the most appropriate path forward.

In the mid-term, another, and quick workaround would be providing getprop() and setprop() equivalents that directly work using Nasal vectors (arrays), i.e. which directly operate on vectors (think GLSL-like): setpropv("/foo/x", vector);

This would be the equivalent of your code, just in C/C++ space - i.e. it would be a subset of the f_setprop() API: https://sourceforge.net/p/flightgear/fl ... s.cxx#l406

The corresponding C APIs are documented at: http://wiki.flightgear.org/Howto:Extend_Nasal

However, it's pretty much copy & paste actually - i.e. a new f_setpropv() function and adapting it to directly get the size of the passed vector and use that in a for-loop to do indexed property updates directly - which should translate into N fewer C<->Nasal context switches, because it'd be all done in native C++ code at that point.

We once did that to help speed up diff'ing two Nasal vectors in a fast way, and it caused a massive speed-up - I think it was James or Stuart who helped get that commited back then.

Given your background in Nasal, C and GLSL you should have no trouble adapting f_setprop() accordingly - but feel free to let me know if you need help or if you'd like me to post a patch implementing a setprop() equivalents that directly operate on vectors with a for-loop implemented in C++

(if the number of points to be plotted can vary a lot, you need some additional cleverness... but the method is very performance-friendly if you need to deal with hundreds of points)


One possibility for a setprop_vector() equivalent might be to take an optional start offset, and to accept a boolean value specififying whether to purge/hide nodes for which the new vector does not have any matching values.
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: Fast updates of plots

Postby sanhozay » Fri Dec 30, 2016 10:10 am

Isn't bypassing the Canvas API a bad idea? Usually part of the reason for having an API is to isolate consumers from the underlying data structures? If the Canvas API isn't performant in this case, shouldn't the Canvas API be updated or extended?
sanhozay
 
Posts: 1207
Joined: Thu Dec 26, 2013 12:57 pm
Location: EGNM
Callsign: G-SHOZ
Version: Git
OS: Ubuntu 16.04

Re: Fast updates of plots

Postby Hooray » Fri Dec 30, 2016 10:23 am

The point is valid, but it's also a compromise obviously: Thorsten needs a way to do certain things faster, and these issues have been known for a long time, without much of a response from people able to review/modify or implement/commit the corresponding changes - which also has to do with the key Canvas folks not being actively involved at the moment.

Like I said, the most general way to address this would be porting props.nas to use cppbind (which would not just benefit Canvas, but also other features needing fast property access, e.g. advanced weather (?)).

The quickest way to deal with this particular situation would be adapting setprop() to support vector-based operations, too.

Apart from that, you are obviously correct in that such workarounds are problematic from a software engineer's standpoint - but it's a manpower issue, too.
James pretty much stated the same thing when we implemented certain workarounds and offered to get the position_diff commited - these days, it's kinda to be expected that peope work around these issues within the constraints that they're facing.

Besides, setData() was never intended to be use for plotting actual graphs with hundreds of points - we're abusing Canvas, and Nasal, for things that nobody thought of when they implemented those APIs.

If he was around, I'd assume that TheTom would probably prefer seeing a cppbind-based props.nas port sooner or later, and in the short term provide a dedicated C++ API to reduce the overhead from doing context switches between C++<->Nasal (we faced the exact same problem with those KNUQ taxiways a few years ago, and that was basically his thinking process back then).

Another possiblilty would be coming up with dedicated Canvas elements for plotting purposes - i.e. because OpenVG is fairly low-level (and causing massive property I/O), according to the commit logs, James is currently exploring this path was part of his FGQCanvas experiments - and that would even make sense for SVG handling in general, because it's the most common Canvas use-case: SVG drawing, and because OSG contains good SVG support.

That would help reduce the OpenVG/property overhead rather massively.

But then again, it's a manpower issue too ...

Besides, some of the folks who are enormously experienced with Nasal and Canvas are not involved in the devel list, i.e. don't have commit access (e.g. it simply got lost during the gitorious/sourceforge transition) - and those who did, may have moved on to other things for a variety of reasons (some of which can definitely be linked back to unnecessary flamewars regarding Nasal/Canvas and overlapping developments in other areas like Phi/Qt5)

So, motivation to get more involved may be another issue, even if someone actually understands how to make these changes.

It's a part of having to deal with different opinions, priorities, and different people :D

Overall, the way we're using Canvas, and especially Nasal, is problematic for a number of reasons and simply doesn't scale - so we need to figure out a way to deal with these challenges, or we will continue to see more of those - especially in the light of the core development bottleneck, i.e. people using Nasal-space workarounds to implement stuff that would better live in the core, or at least have access to existing core hooks.

In the meantime, it is to be expected that people will be doing all sorts of micro-optimizations to work around such issues. because they don't have to nag any core devs to implement/commit something that they need - but it is true that we are also circumventing the API here, and that is crippling other things (e.g. encapsulation), or even just backward compatibility (think the setprop() syntax requiring FG 3.2)
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: Fast updates of plots

Postby Thorsten » Fri Dec 30, 2016 10:40 am

Isn't bypassing the Canvas API a bad idea?


If you have a clear performance bottleneck, running fast special purpose code rather than a slower general-purpose API is usually worth it in my view. But feel free to ignore anything I wrote.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Fast updates of plots

Postby Hooray » Fri Dec 30, 2016 11:29 am

I really don't think it was intended to sound that way - most people are probably just unaware of the intrinicacies of getting such changes discussed, prototyped, tested, reviewed, refined and committed/maintained.

For instance, I haven't built FlightGear in a while, and I do remember that you were using downloaded binaries from the build server, too - which means that we are both in a bad position to come up with patches and test those, so there simply is a certain barrier to entry regardless of the circumstances, even if people know how implement the corresponding C++ changes (which in this case really is trivial because your Nasal code is close enough to adapt the existing f_setprop() routine and make it optionally accept a vector).

For the time being, it makes sense to work around the problem until it is "properly" solved - however, the previous poster is right in that we're circumventing the API, i.e. there is no encapsulation (read: protection) at all when doing this - and James recently discovered the z-indexing issue, which is a similar issue in that the APIs are doing something that isn't really exposed at the property tree level. And we do have a number of Nasal APIs meanwhile that don't translate into the corresponding property tree changes at all, which is problematic from an encapsulation standpoint, i.e. if/when things are updated (as James was alluding to), we would possibly be breaking existing code along the way.

However, I would still suggest to try the setprop(path, index, vector[element]); syntax, it should cause even less Nasal overhead (in theory), while making the code better readable too.
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: Fast updates of plots

Postby Thorsten » Fri Dec 30, 2016 11:51 am

However, I would still suggest to try the setprop(path, index, vector[element]); syntax, it should cause even less Nasal overhead (in theory), while making the code better readable too.


Yes, checks out to work a bit faster here.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Fast updates of plots

Postby Hooray » Fri Dec 30, 2016 12:20 pm

Ok, thanks for trying that and for reporting back here.

I am not sure if you are currently able to apply/test any C++ patches that would require re-building FlightGear from source ?

Regardless of that, my suggestion would be to introduce a flag to make this workaround conditional, i.e. along the lines of the original compat_layer.nas approach - so that you can continue to use the current method, but also easily stop using it by setting a variable and/or property - and maybe add a comment to the top of this workaround linking back to this topic here, so that future maintainers are aware of what you are doing there, and why - as we both know, it may take months of even years until these issues are addressed, at which point it may actually be helpful to look up the original discussion leading to the corresponding workaround (remember the ATI camera hack?).

As a matter of fact, I would even copy your whole posting and either add it as a comment or copy it into the commit message when committing the workaround.

Either way, introducing a conditional flag would seem like a good idea - e.g. imagine props.nas being ported to cppbind and/or dedicated APIs becoming available making this unnecessary, at which point disabling the workaround would be much easier by setting a corresponding flag.

However, coming up with an adapted version of setprop() that accepts a Nasal vector and runs the whole thing in C space is really trivial to do for anybody familiar with the Nasal C APIs - and I am offering to prototype the whole thing if we can get this reviewed/committed - for example, is Richard currently around to help with such matters, does he have commit access ?

I have basically edited, maintained (if not even written) all wiki articles documenting the Nasal/C APIs as well as cppbind in particular, so I do understand how to adapt f_setprop() accordingly - which should translate into seeing only one context switch instead of N depending on the size of the vector- due to switching between the C runtime and the Nasal engine.

I think the whole concept of doing should feel pretty familiar to you given your background in GLSL and operating on multi-dimensional vectors of variables, right ?

Right now, we have a Nasal loop that ends up calling 3 extension functions in its loop body - which means 3 API calls X size of the vector => which gives us the number of Nasal/C context switches, whereas the same thing could be accomplished with only 3 setprop() calls that directly operate on those 3 vectors.

Let me know what you think, and I can post a prototype that implements that behavior.
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: Fast updates of plots

Postby Thorsten » Fri Dec 30, 2016 12:46 pm

I'm (unfortunately) not out to improve the canvas framework or Nasal at this point - I was trying to solve a problem with the Shuttle ADI ball, and I'll now proceed to code the smart 'pull into flare' guidance that I wanted to code for the last days but didn't manage to because I had to read canvas API and fiddle with benchmarking property access.

I'll gladly use any 'proper' solution (fallback to .setData() is trivial since I have the arrays ready) eventually if someone codes it.

I know the world would be a better place if I'd always do patches for the C++ code, document everything and discuss my changes with others in depth - but the day has only 24 hours and I wouldn't get my own projects forward.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Fast updates of plots

Postby sanhozay » Fri Dec 30, 2016 1:22 pm

Hooray wrote in Fri Dec 30, 2016 11:29 am:I really don't think it was intended to sound that way

It wasn't intended to sound any way. It was just a question/observation.

This was my concern:
Hooray wrote:problematic from an encapsulation standpoint, i.e. if/when things are updated (as James was alluding to), we would possibly be breaking existing code along the way.

This is just a suggestion ... would it make sense to have a fast updateCoords method for the path? Something like the following (untested):

Code: Select all
var coords = [x1, y1, x2, y2, x3, y3];

var p = Path.new()
    .moveTo(coords[0], coords[1])
    .lineTo(coords[2], coords[3])
    .lineTo(coords[4], coords[5]);

for (...) { # update loop
    coords[0] = x1a;
    coords[1] = y1a;
    ... and so on
    p.updateCoords(coords);
}

There is no updateCoords method in api.nas but it's trivial using a loop and setprop. I'm assuming the commands don't change on this type of refresh operation.

Code: Select all
updateCoords: func(coords)
{
    var path = me._node.getPath();
    forindex(var i; coords)
        setprop(path~"/coord", i, coord[i]);
}
sanhozay
 
Posts: 1207
Joined: Thu Dec 26, 2013 12:57 pm
Location: EGNM
Callsign: G-SHOZ
Version: Git
OS: Ubuntu 16.04

Re: Fast updates of plots

Postby Hooray » Fri Dec 30, 2016 1:34 pm

I agree (wholeheartedly), and to be honest, I completely missed that you were talking about api.nas level changes - that really is the right place to add Thorsten's workaround, while also providing an encapsulation point, i.e. a place for updating things in a holistic fashion if/when the C++ code (setprop) is updated accordingly.

My only suggestion would be to add the updateCoords method in a fashion that runs a version check (querying /sim/version) so that the string concatenation syntax is used in versions older than 3.2 - even though I think that's a fairly unlikely scenario given the changes in the shuttle and canvas departments since then.

So, I think you have just come up with a way for us to have our cake and eat it - we just need someone with fgdata access to commit the corresponding changes to api.nas, I will help review those - and also volunteer to update the wiki accordingly.

In th meantime, I have also started prototyping vector support for the setprop() API: http://wiki.flightgear.org/Howto:Vector ... l_setprop()_API#Understanding_setprop.28.29

And the good thing is that if/when this gets committed, we'd only ever have to update api.nas and its updateCoords() method there.

So, good thinking there - let's just hope that we can make the whole thing fruitful now :D


Thorsten wrote in Fri Dec 30, 2016 12:46 pm:I'm (unfortunately) not out to improve the canvas framework or Nasal at this point [...] I know the world would be a better place if I'd always do patches for the C++ code, document everything and discuss my changes with others in depth - but the day has only 24 hours and I wouldn't get my own projects forward.


I agree, and can perfectly relate to that - so it was not necessary to make that point. However, to be fair, TheTom was never planning to create the Canvas system in the first place, had he not faced a road blocks when creating the cockpit avionics for the C130J that he was developing at the time - so there is something to be said in favor of working towards common goals every once in a while. Which is also why it is correct to not just micro-optimize the Canvas use-case, but reworks props.nas or at least setprop() accordingly. Whenever people do this sort of thing, that is usually when major accomplishments are made - no matter if that means having an effects system, a Canvas system, an advanced weather system or even the space shuttle :D
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: Fast updates of plots

Postby Thorsten » Fri Dec 30, 2016 2:00 pm

I hate to spoil the good mood, but:

(if the number of points to be plotted can vary a lot, you need some additional cleverness... but the method is very performance-friendly if you need to deal with hundreds of points).


Part of the game is a smart-wipe routine for unused notes so that they don't clutter the plot but you wipe only what you don't need because wiping/re-creating takes time. And that's where you need custom code because how to do that best depends on your problem.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Fast updates of plots

Postby Hooray » Fri Dec 30, 2016 2:49 pm

Like I said, I really do understand what you are doing there - we faced the exact same problem a few years ago when we were dealing with the KNUQ taxiways and more recently, when we came up with the MapStructure framework: we needed a way to selectively update a group of canvas elements, using the delta of two different vectors (arrays), trying to efficiently reuse existing nodes (aka canvas path elements) and hiding those not used, to keep them around for re-allocation purposes.

Thus, what is commonly done under these circumstances (generic code requiring custom heuristics) is supporting an optional callback to be passed in by the caller, this is basically IOC (inversion of control) and you are free to do whatever you want without having to touch any of the generic code - the MapStructure framework gets away with its MVC way of doing things due to this being commonly done all over the place: there are a bunch of defaults that can be freely overridden using custom code - i.e. most APIs allow optional callbacks to be passed to handle such details.

Speaking more generally, it may actually make sense for us to revisit the idea of introducing a simple GC scheme where nodes don't necessarily need to be explicitly removed or hidden, but where they can be "disabled" (i.e. kept around) while beind merely hidden - at that point, we can reduce the re-allocation overhead, e.g. by configuring a sane default value, so that a number of XXX elements are always kept around (hidden), to avoid having to remove/re-create the corresponding nodes.

These element-specific defaults could be configured via properties per group, so that coders can specify the pruning behavior when updating properties, to avoid unnecessary re-allocations in between two subsequent updates.

Equally, it would make sense to introduce dedicated APIs operating on vectors of data, at that point the overhead of switching between Nasal/C should be rather minimal.
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: Fast updates of plots

Postby Thorsten » Fri Dec 30, 2016 3:42 pm

https://sourceforge.net/p/fgspaceshuttl ... /p_pfd.nas

the function you're looking for is update_plot_data

It's a sort of 'lazy' GC which just hides nodes once when there are less requested updated than currently assigned but does a wipe if the overhead grows to more than 10%.
Thorsten
 
Posts: 12490
Joined: Mon Nov 02, 2009 9:33 am

Re: Fast updates of plots

Postby Hooray » Fri Dec 30, 2016 3:53 pm

yeah, as usual, it seems you can figure out tons of complicated stuff on your own without having to ask folks about it - you really did implement a GC scheme to keep nodes around and hide them instead of removing them entirely ... good job there :-)
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


Return to Canvas

Who is online

Users browsing this forum: No registered users and 1 guest