Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Outrageous lag caused by a full inventory #11305

Closed
deltanedas opened this issue Jun 2, 2021 · 5 comments
Closed

Outrageous lag caused by a full inventory #11305

deltanedas opened this issue Jun 2, 2021 · 5 comments
Labels
Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible

Comments

@deltanedas
Copy link

deltanedas commented Jun 2, 2021

Minetest version
Minetest 5.5.0-dev-debug-631636f91-dirty (Linux)
Using Irrlicht 1.9.0mt2
Using LuaJIT 2.1.0-beta3
BUILD_TYPE=Debug
RUN_IN_PLACE=0
USE_CURL=1
USE_GETTEXT=0
USE_SOUND=1
USE_FREETYPE=1
OS / Hardware

Operating system: Debian Sid
CPU: Intel Core 2 Duo P8400 (2) @ 2.267GHz

GPU model: Intel Mobile 4 Series Chipset
OpenGL version: 3.3 Core

Summary

When opening a unified_inventory inventory filled with bricks, draw calls rise from about 4000/frame(!) to over 18000/frame(!!!!!!)
As a result on my worst machine, my frametimes go from about 20ms to 110ms, simply by looking at bricks.
This hints at the inventory list formspec element not putting each item inside a common vertex buffer. 12000 draw calls for a handful of half-cubes (prerendered to a single icon sprite i believe) and font glyphs is simply wrong.
Not only is item rendering sluggish, but an empty inventory takes a good bit of time to render (not nearly as bad though)
I'd say most formspecs of similar complexity are like this.

Inventory list rendering seriously needs to be batched, I'd like if someone could point me to a starting point on this matter.

Steps to reproduce
  1. apitrace trace minetest
  2. fill inventory with bricks
  3. qapitrace minetest.trace
  4. look for the massive spike in draw calls

Inventory:
Screenshot_19:53:38

Draw call spike:
Screenshot_11:25:02

Trace file (opened inv, closed inv, opened again at the end):
minetest.trace.gz

@deltanedas deltanedas added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label Jun 2, 2021
@deltanedas
Copy link
Author

extra info relating to sfinv:

  • the formspec with no items shown takes about 4ms, not bad
    Screenshot_13:17:29

  • the formspec with an empty inventory but full item list on the right is half as bad as the full inventory
    Screenshot_13:17:11

  • with no items theres only about 400 draw calls for the formspec, not even close to the itemstack mess.

  • empty inventory, full item list takes 11k vs 18k draw calls, so it's roughly proportional to frametime cost.

so i'm thinking that while formspecs on their own are nicely optimised, it's just the inventory list rendering that thrashes performance

@SmallJoker
Copy link
Member

Duplicate of #6905

@SmallJoker SmallJoker marked this as a duplicate of #6905 Jun 2, 2021
@sfan5
Copy link
Member

sfan5 commented Jun 2, 2021

extra info relating to sfinv:

Not sure what you meant here, sfinv is Minetest Game's inventory implementation. What you're showing in your screenshots is unified_inventory.

@sfan5
Copy link
Member

sfan5 commented Jun 2, 2021

On my system I count 24975 drawcalls with bricks and 7623 without(*), which makes a difference of ~18k.
(*) They're still showing in the hotbar.

This hints at the inventory list formspec element not putting each item inside a common vertex buffer.

You are correct, Items are drawn individually using this method:

minetest/src/client/hud.cpp

Lines 975 to 985 in e15cae9

void drawItemStack(
video::IVideoDriver *driver,
gui::IGUIFont *font,
const ItemStack &item,
const core::rect<s32> &rect,
const core::rect<s32> *clip,
Client *client,
ItemRotationKind rotation_kind,
const v3s16 &angle,
const v3s16 &rotation_speed)
{

I'd like if someone could point me to a starting point on this matter.

Collecting matching meshes into one buffer shouldn't be much of a problem (translation, scaling and rotation would have to be done in software), all I can think if is using the same projection matrix for all of them might not be simple.

There is a different problem though, our GUIs are drawn progressively and not "layer-for-layer" as would be helpful. This should illustrate it nicely:
grafik
That means you can't just batch and move item drawing to another place without also worrying about drawing backgrounds and labels in the correct order.
Fortunately this is all contained inside hud.cpp.

@deltanedas
Copy link
Author

haha yeah i mixed up sfinv and unified_inventory
will take a look at hud.cpp, maybe i can untangle backgrounds -> items -> labels

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible
Projects
None yet
Development

No branches or pull requests

3 participants