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

listen() halts UI if used on many controllers #35

Closed
yoavmil opened this issue Jan 19, 2022 · 6 comments
Closed

listen() halts UI if used on many controllers #35

yoavmil opened this issue Jan 19, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@yoavmil
Copy link

yoavmil commented Jan 19, 2022

I've tried lil-GUI to replace dat.GUI, following threejs.
I've noticed it kill my framerate. this happens because I use the listen() on each field.
Note that in dat.GUI it doesn't happen.
For now I'm returning to dat. keep me updated.

/*
In this code I populate the GUI with a settings structure. 
*/
  private populateGui(node: GUI, key: string, settings: any) {
    if (typeof settings[key] == typeof String()) {
      node.add(settings, key).listen();
    } else if (typeof settings[key] == typeof Number()) {
      node.add(settings, key).listen();
    } else if (typeof settings[key] == typeof Boolean()) {
      node.add(settings, key, settings[key]).listen();
    } else if (typeof settings[key] == typeof Object()) {
      let folder = node.addFolder(key);
      Object.keys(settings[key]).forEach((k) => {
        this.populateGui(folder, k, settings[key]);
      });
    }
    node.close();
  }

  public open(settings: Settings) {
    this.close();
    this.gui = new GUI();
    this.gui.domElement.parentElement.style.zIndex = "10";
    Object.keys(settings).forEach((key) => {
      this.populateGui(this.gui, key, settings);
    });
  }
@georgealways
Copy link
Owner

Oh thanks for the catch. Yes, dat.GUI consolidates all the listen loops, lil-gui makes a new loop for each controller.

Is this hosted anywhere by chance? I'm curious how many controllers it takes to make this happen.

I'll look into consolidating the loops in a future release.

@yoavmil
Copy link
Author

yoavmil commented Jan 20, 2022

not hosted. I had ~100 controllers. do you plan to solve it?
BTW, dat.gui has a npm warning , does lil-gui has it too?

dat.gui  *
Severity: high
Regular Expression Denial of Service in dat.gui - https://github.com/advisories/GHSA-chwr-hf3w-c984
No fix available

@georgealways
Copy link
Owner

Yes, I'd like to. I'll need to reproduce it first to have something to test against.

dat.gui's NPM warnings aren't related to this library. Unless you're seeing warnings when installing lil-gui you should be fine.

@georgealways georgealways changed the title listen() halts UI listen() halts UI if used on many controllers Jan 20, 2022
@georgealways georgealways added the bug Something isn't working label Jan 20, 2022
@georgealways
Copy link
Owner

Should be able to do a lot more listening if you update to 0.16.1

Let me know how that works!

@yoavmil
Copy link
Author

yoavmil commented Feb 13, 2022

No sir. Sorry, still way too slow. Down to 5 fps (!). And yes, I'm using 0.16.1

@georgealways
Copy link
Owner

I'm able to run this page https://dev.lil-gui.georgealways.com/examples/listen-stress/?100 at 50fps on a 2014 MBP (after checking animateAll). Unless you're able to upload your work I can't really help any further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants