[PATCH] Don't select units while dragging selection bandbox

[Alex]

Don't select units while dragging selection bandbox which causes vast performance improvements.

During busy game situations, trying to select many units can cause critical performance issues because the game keeps calculating which units shall be selected again and again during the drag operation.

Practically, there's no advantage in seeing/knowing what entities are getting selected with my selection rectangle - I just tested the gameplay and really adored the overall improved performance coming with that solved issue!

[Alex]

Not sure whether this is the proper patch-file format - but you should get the things which changed.

[sanderd17]

As dicussed on IRC, since this patch removes useful functionality (seeing which units get selected while selecting), we first expect:

• An attempt at profiling the difference, and finding the point of major slowdown in the code.
• If the major point of slowdown can be optimised without crippling the functionality too much, that should be done.
• If the major slowdown is caused by a method that's hard to optimise, but leaving it out does show a big performance improvement, then turning this into a setting would be better.

[Alex]

Alright, I just modified the 0ad/binaries/data/mods/public/gui/session/input.js:565 so that it dumps out the time it required to get all entities beneath the selected rectangle:

var now = new Date(); var ents = Engine.PickFriendlyEntitiesInRect(rect[0], rect[1], rect[2], rect[3], Engine.GetPlayerID()); warn("PickEnts: "+(new Date()-now)+"\n");

Now I generated a random 6-player dark forest map with me as a player + 5 AIs. Even after just having started the match it spits out something around 30ms. Of course, with even more units overall, this value gets even bigger.

I might not need to give further explanation why >30ms * >60 times per second in which the mouse is moved (i.e. SDL mouse-move-event) to grow up the selection rectangle is a bad and (at least for me who rather depends on quick interaction with the game rather than knowing which items become selected half a second before I release the mouse button) unsettling circumstance.

I'm on Linux, my notebook has both an integrated Intel HD 4600 as well as a GeForce 750M chip. I've tested playing the same map with both graphic devices: the Intel one led to a selection duration around 40ms, the GeForce around said 30ms. Presuming that one has enabled VSync, one frame should take 16ms to be rendered. It's more than frustrating to fear selecting units only to prevent these fps drops - I bet probably noone would even notice the difference of not being able to see which units get selected but rather enjoy the speed-up :)

[Alex]

Of course, the duration value is way lower if the map is smaller and I used the entirely zipped gamedata folder public.zip. Still, having huge maps is a possible game state and thus should be supported as well - or, just have a small map and get massive amounts of units on it as it appears to be in each late-game. Even if the function alone only took 15ms, it's still a major interference which caused massive fps drops.

[Alex]

I've done a normal-sized map game now with 3 AIs and the method's execution time went down to 4-7ms. Still way too long to be executed dozens of times during rectangle drag&drop.

Is it possible that the iteration itself is the inefficient part of the method, not the translation calculation/position check? Perhaps all units should be ordered by their owner or via some creative hashing algorithm, preferably as soon as the unit becomes registered in the system. :)

[sanderd17]

Since you get huge slowdowns at the start on a big map, and much smaller slowdowns on small maps, it's most likely that it iterates over all trees (or at least does too much work for trees).

These are my thoughts so far:

At ​http://trac.wildfiregames.com/browser/ps/trunk/source/simulation2/helpers/Selection.cpp#L124, the trees are most likely to fail at the ownership test, so that test could be move higher (avoiding two unneccesary tests for trees)

In case it's not enough, and Gaia has the majority of entities, your idea could indeed be implemented. It's easier to avoid the player test in the for loop at all, and pre-create maps with entities per player. Either the methods from RangeManager (and the data) could be reused (the cached player data likely makes it faster). Or the Player or Playermanager component could keep track of a list of its entities (in the same way as RangeManager, by listening to the ownershipChanged messages).

Btw, the zipped data only has an effect on loading speed, not on execution speed.

[Alex]

Furthermore, the check for the owner-filter to be valid and/or whether the engine runs in Atlas must then be done earlier and could then iterate through all entities and only look for their positions. [Edit] Why not have a second method for this?

[Alex]

Philip: It's okay to use CmpPtr's as parameters as long as you're not making them member variables. sanderd17: 'CheckEntityVisibleAndInRect' as function name is okay.

owner == INVALID_PLAYER most exclusively means usage in Atlas.

[Alex]

Rework of the PickEntitiesInRect? method - It takes ~0ms until it returns now, even on huge maps in late-game!

[Itms]

In 16068:

Use the range manager to pick entities when drawing a selection box during a game. This allows for a significant performance improvement.

Fixes #2961, patch by @aBothe.

[Itms]

Thanks for the patch! :)