Advanced inventory invalidates UI too often and performance drops rapidly with more items as it repopulates adv inventory

[CoroNaut]

Describe the bug

Using the advanced inventory to move single items or even stacks of items at a time requires the advanced inventory UI to be considered invalidated after every input. This may be fine when the total amount of items the advanced inventory has to collect is low. However, with more items, this performance slowdown just increases linearly until you can feel the slowdown as your cpu bogs down trying to collect the list of items every frame.

The MOVE_ALL_ITEMS feature doesn't have this problem. The ui just disappears as it does its thing and moves the entire contents to where you want. The problem is specifically 'MOVE_SINGLE_ITEM', 'MOVE_VARIABLE_ITEM', 'MOVE_ITEM_STACK' redrawing and re-collecting the items for both panes upon any of these inputs.

https://github.com/CleverRaven/Cataclysm-DDA/blob/3f2229b8349ad4730aa46523100e0db213b597f9/src/advanced_inv.cpp#L1947-L1958

1950: exit = action_move_item( sitem, dpane, spane, action ); Going into line 1950, we get to the next shown code block:

https://github.com/CleverRaven/Cataclysm-DDA/blob/3f2229b8349ad4730aa46523100e0db213b597f9/src/advanced_inv.cpp#L1651-L1655

This enables invalidation of the UI after it went through all its work to move either 1 item, partial stack, or a full stack of items. So, when the action is taken and the item is moved, it goes in this loop; it re-draws and re-makes the advanced inventory UI, clears all the items from both panes, gets all the items back, and shows the pane to the user. All to move 1 single item.

https://github.com/CleverRaven/Cataclysm-DDA/blob/3f2229b8349ad4730aa46523100e0db213b597f9/src/advanced_inv.cpp#L734-L738

... leads to ...

https://github.com/CleverRaven/Cataclysm-DDA/blob/3f2229b8349ad4730aa46523100e0db213b597f9/src/advanced_inv.cpp#L793

As you can see from the flame graph below, once you get enough items, it becomes more than half the cpu usage to just dump the items that were already in AIM and re-grap them all.

Attach save file

Grafton-trimmed.tar.gz

Steps to reproduce

1. load save
2. Open advanced inventory (it should already have NW and SE selected in AIM)
3. Begin transferring either way by holding down enter or your equivalent 'move 1 item' key (its super slow)
4. Go to a different spot in the base with less items and use AIM (its much faster, because less items)

Expected behavior

The advanced inventory UI should not be invalidated after every key press that isn't just the 'move all' command. The pane could just redraw rather than invalidate every time to account for the moved item from one pane to another.

It should not just dump the entire contents and use add_items_from_area just to re-grab them for both panes, which is >80% of the issue.

Screenshots

This flame graph is of me following the steps to reproduce

Versions and configuration

• OS: Windows
  • OS Version: 10.0.19045.4651 (22H2)
• Game Version: cdda-experimental-2024-09-10-0339 c73e4c6 [64-bit]
• Graphics Version: Tiles
• Game Language: System language []
• Mods loaded: [ Dark Days Ahead [dda], Disable NPC Needs [no_npc_food], Portal Storms Ignore NPCs [personal_portal_storms], Slowdown Fungal Growth [no_fungal_growth] ]

Additional context

No response

[CLIDragon]

MOVE_SINGLE_ITEM being slow is unavoidable. For MOVE_VARIABLE_ITEM and MOVE_ITEM_STACK, it should be possible to create a separate function similiar to MOVE_ALL_ITEMS that avoids invalidating the UI.

[CoroNaut]

I agree with the separate functions for variable and stack move.

All 3, single, variable, and stack moves cause AIM to drop its list of items when it doesn't need to. AIM knows exactly what item was moved, so why go full brute force and just drop the list and re-make it for the redraw of the AIM window? It doesn't make sense, it could be easily copied over to the next frame for AIM to draw. I believe fixing this will have the greatest performance impact.