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

Fixes/hide unneeded burg labels #1070

Conversation

EricKnudsen256
Copy link

Description

Labels seem to be a considerable drain on resources, especially when a lot of them are being rendered at once. To help alleviate this, burg labels that are no longer on the screen can be set to hidden so that they are not rendered by the browser, and thus do not take up resources to render anymore. I did a bit of checking to make sure it didn't break anything, but let me know if I've missed something or missed part of this pull request process since this is the first one I've done.

Type of change

  • Bug fix

Versioning

  • Version is updated
  • Changed files hash is updated

…screen. Results in noticable FPS increase on maps with high burg count (tested with 800+ burgs)
Copy link

netlify bot commented Apr 28, 2024

Deploy Preview for afmg ready!

Name Link
🔨 Latest commit fd47c82
🔍 Latest deploy log https://app.netlify.com/sites/afmg/deploys/662dc5d926d74f0008b8ff7a
😎 Deploy Preview https://deploy-preview-1070--afmg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Azgaar
Copy link
Owner

Azgaar commented Apr 28, 2024

Hello. Need to test it to say for sure, but browser should not render elements that are out of the screen by default. So you don't need to mark them as hidden...

@EricKnudsen256
Copy link
Author

I would think they wouldn't render them, but considering how much better the app runs when you hide those labels, I'm not sure. I admittedly don't know too much about how browsers render things, but I've tested with Chrome and Opera and the framerate is considerably better after this one change.

@Azgaar
Copy link
Owner

Azgaar commented Apr 28, 2024

Need to test it myself. In any case the code should be optimized first.

@EricKnudsen256
Copy link
Author

EricKnudsen256 commented Apr 28, 2024

Agreed, this was more of a proof of concept on the idea. Probably should have been brought up as an issue instead of a pull request, my bad.

@Azgaar
Copy link
Owner

Azgaar commented Apr 29, 2024

No, that's fine, I will try to test it today

@Azgaar
Copy link
Owner

Azgaar commented Apr 29, 2024

@EricKnudsen256, I have tested it on my machine. I don't see any performance improvement, for it makes the dragging even a bit more slow...

@EricKnudsen256
Copy link
Author

Huh, that's definitely not what I'm experiencing on my end. I'll go get some more data when I get the chance and figure out what's causing the lag for me then. It's definitely text-based, since my framerate more than doubles when I disable labels just on the main release branch right now. If you're not experiencing it though, then there's either something more going on, or I'm wrong. Either way, I'll test some more and get come back once I've got some more concrete info then "it feels faster for me".

@Azgaar
Copy link
Owner

Azgaar commented Jul 13, 2024

Closing as it was not proved to be a working solution... Please respond with some tests if it's not true.

@Azgaar Azgaar closed this Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants