Cherry / 3D2D-Textscreens

3D2D Textscreens Garry's Mod Workshop Addon
https://jross.me/3d2d-textscreens/
MIT License
27 stars 20 forks source link

why are you making so many fonts #63

Closed Potatosayno closed 4 years ago

Potatosayno commented 4 years ago

Hey I noticed you are making insane amounts of fonts with different sizes. This is NOT needed and may cause client crashes/issues. Instead what you should be doing is using the 3D2D scale modifier (Which in your addon is set to .25 and is not used for an odd reason).

mcNuggets1 commented 4 years ago

Since when does creating fonts crash? Any source?

Potatosayno commented 4 years ago

https://github.com/Cherry/3D2D-Textscreens/blob/master/lua/textscreens_config.lua#L3-L12

Also might've been the cause for this crash (Not proven) https://pastebin.com/fcW957bN

Loading a bunch of fonts on the client every frame is idiotic, it even loads in the font for those without text, this addon literally creates more fonts than the server itself with all the other addons combined.

https://github.com/Cherry/3D2D-Textscreens/blob/master/lua/entities/sammyservers_textscreen/cl_init.lua#L57-L76

Cherry commented 4 years ago

I'd be open to a PR to improve this, but the addon has been this way for 5+ years, and there's been no significant report of any issues for how I'm doing it. The fonts are only created once, when the addon starts up - not every frame.

If you've got a "smarter" way to do this that is less "idiotic", please feel free to submit a PR.

mcNuggets1 commented 4 years ago

The fuck are you talking about. The fonts are created ONCE not every frame.

This shouldn't crash your server, nor the clients, fix your server instead.

Potatosayno commented 4 years ago

Sorry about the every frame thing, I was just getting confused a bit.

Just to clear things up. Cherry your check on https://github.com/Cherry/3D2D-Textscreens/blob/master/lua/entities/sammyservers_textscreen/cl_init.lua#L63 should include (#data[i][TEXT] <= 0). This will improve performance as the client will have less fonts to load every frame (Which the addon does, even for lines without any text on them, this should fix that too).

Also regarding the 'bunch of fonts' - I could suggest using multiple cam.Start3D2D instead to save memory. I've also asked the Garry's Mod development community and many would agree using a bunch of fonts to change the size of text is stupid.

Potatosayno commented 4 years ago

Not very aware on how to use Matrix but that could be a possibility too?

Cherry commented 4 years ago

Please submit a PR with these changes and some kind of benchmark or analysis to show this solution is indeed better. I still maintain this addon, but I don't really have the time to test and investigate these claims I'm afraid, sorry, especially if this is going to be such a marginal performance improvement.

Potatosayno commented 4 years ago

No problem! :) I'll do it once I have time as I too would want these fixed on my own server.

robotboy655 commented 4 years ago

You really should fix this, people are crashing left and right on servers that use this mod, case and point the issue mentioned just above this comment.

Even creating the fonts only when necessary, i.e. when they are about to be displayed but only once would be miles better.

If you want a proof this causes issues for people, try increasing the size range from 0-100 to 0-255 and/or add more font files and just try to load into a map. This may depend on your RAM/VRAM size/hardware in general.

Cherry commented 4 years ago

Even creating the fonts only when necessary, i.e. when they are about to be displayed but only once would be miles better.

Correct me if I'm wrong, but there's no current way in gmod to test if a font already exists before creating it. It's for this main reason that when the addon is loaded, the fonts are created in https://github.com/Cherry/3D2D-Textscreens/blob/master/lua/textscreens_config.lua#L3-L12.

The best solution here would likely be to create a single font for each font family, and then scale the 3D2D render appropriately to get the wanted font sizes. I'm unlikely to be able to look at this any time soon though, so would be very happy to accept a PR.

robotboy655 commented 4 years ago

You can always keep a local cache of names you already created and check that, but yeah, scaling it like already mentioned would be best solution.

Cherry commented 4 years ago

Fixed in #64