CreativeMD / OnlinePictureFrame

GNU Lesser General Public License v2.1
22 stars 9 forks source link

Image + Gif loading optimizations #6

Closed Gegy closed 7 years ago

Gegy commented 7 years ago

This PR implements a few different optimization to the image loading process:

CreativeMD commented 7 years ago

Hey,

Thank you sorry that it took me so long to answer. I was just trying to find to the right words to say you how thankful I am that someone takes the time to help me out. I have been modding for about 5 years now and most of my projects are open source for about 3 years, but this is the first pull request (expect of translations) so far.

THANK YOU!

Ok, let's get back to the topic.

Main problem Unfortunately most of the lag is caused by creating the texture which can be done in the main thread only since Minecraft uses an old version of the lwjgl library.

Your Changes

Summary Again thank you for all your work. I'm really happy about all your changes, although I don't think all of them are necessary, but this impression might caused by my laziness. If something works I don't see a point in changing (too much else to do).

The reason why i have not merged your pr, is because you still marked it as being WIP, so i'm not sure if it's finished already. Besides that i would like to know what kind of reasons pushed you forward to create pr?

In Regards CreativeMD

Gegy commented 7 years ago

Hey, thanks for taking your time to review this PR.

So yeah, a lot of the lag is caused by uploading the texture to GPU, which can only be done on the main thread.

As well as moving the BufferedImage loading to the download thread, the process of loading a frame to a ByteBuffer for upload has also been moved to that thread. I don't believe there is much more that can be done for this.

Uploading frames as they are requested causing lag over many frames is for sure a concern. This really is a compromise. Someone that could be done is to implement a size limit for images + gifs (dimensions, length, filesize). This would ensure nobody's cache would be filled up with large gifs and make sure no gifs produce large amounts of lag each frame.

There were already multiple threads for downloading, I've just added a limit to the maximum amount that can be run concurrently to prevent "internet hogging".

So, originally I marked this as WIP because the caching wasn't great. Currently it's still marked as WIP because I'm open to suggestions for improvement and have a few things I'm not sure about that could be beneficial to performance. As well as this, as you mentioned above, some of these do need to be tested more.

One of these would be stitched gif textures: This saves texture ids and removes the need for multiple images to be stored in memory. The downside here is the whole stitched image has to be loaded at once, which could possibly cause lag spikes as happened previously, and there may be a size limit on images that can be uploaded to the GPU, especially on older graphics cards / drivers.

This PR has had a fair amount of testing done, as the motive for creating this was due to large amounts of lag on the ModOff server from gifs. A few people have been using this to help prevent those lag spikes, and it seems to have been working good for them. (I suppose this answers your question on my motives to create this PR)

EDIT: By the way, after this PR is complete, I will likely be submitting another to work on some security issues.

tterrag1098 commented 7 years ago

A few things to note:

CreativeMD commented 7 years ago

Oh yes, totally forgot that. I have copied the loading texture code a longer time ago and never took a look at it again. Once the creation of the buffer is threaded there should be no lag-spike at all.

Thanks a lot for all your help guys! Much appreciated! Maybe i add you (gegy1000) to the project on curseforge?

PorPit commented 7 years ago

good job!

PorPit commented 7 years ago

if u do not want takes up more GPU memory, when a texture not render after some time, should be delete textures (GL11.glDeleteTextures(textureid);) XD