search

Azgaar / Fantasy-Map-Generator

Web application generating interactive and highly customizable maps
https://azgaar.github.io/Fantasy-Map-Generator
Other
4.48k stars 636 forks source link

Fixes/hide unneeded burg labels #1070

Closed EricKnudsen256 closed 1 month ago

EricKnudsen256 commented 4 months ago

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

Versioning

netlify[bot] commented 4 months ago

Deploy Preview for afmg ready!

Name Link
Latest commit fd47c8271911861c61979720dc44bc60873e32cd
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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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

EricKnudsen256 commented 4 months ago

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 commented 4 months ago

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

Azgaar commented 4 months ago

@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 commented 4 months ago

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 commented 1 month ago

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