Switch to LibPNG

Reporter: faissaloo  |  Status: closed  |  Last Modified: February 16, 2019, 03:33:06 PM
Is there any particular reason why we're using LodePNG? It's abandoned, slow and is often not on package managers. I'm currently trying to get LibEGM and Emake compiling on MacOS and this is one of the things making it difficult.
RobertBColton  
Ok, I'll do it, from testing, libPNG is faster too and fundies has been asking me to do this so we don't have to check in lodepng in the repo (even though we do have it in shared). I've just been lazy about doing it, sorry. I will get to it!
RobertBColton  

Alright, #1442 switches emake and libEGM to libpng. That just leaves LodePNG being used in the engine. Try that out for me and let me know if it makes the setup easier. I mean we already have our reasons for switching which you reiterated, but there's no rush until everybody is sure.

I don't know how performance will be different yet, I will do a benchmark in a second. I am actually doing the reading differently now by decoding a single row of the image at a time, so it may be faster or use a different amount of RAM.

faissaloo  

libEGM compiles successfully now.
RobertBColton  

Nice, huehue, I'll include the engine change in that pr too.
RobertBColton  

Ok so I made some tweaks to emake in order to do a benchmark so we can prove to the whole wide hello world that we're doing this scientifically. My table below will compare the average time master and the pull request take, in milliseconds, to load several projects five successive times.

    } else if (ext == "gm81" || ext == "gmk" || ext == "gm6" || ext == "gmd") {
      buffers::Project* project;
      size_t runs = 5;
      double avgTime = 0.0;
      for (size_t i = 0; i < runs;) {
        auto start = std::chrono::high_resolution_clock::now();
        if (!(project = gmk::LoadGMK(input_file))) return 1;
        auto elapsed = std::chrono::high_resolution_clock::now() - start;
        long long milliseconds = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed).count();
        std::cout << "time " << i << " " << milliseconds << " ms" << std::endl;
        double delta = milliseconds - avgTime;
        avgTime += delta/++i;
      }
      std::cout << "benchmark completed avgTime " << avgTime << " ms" << std::endl;

      return 0;

Having reviewed the results of my own benchmark, I am now going to proceed to switching the engine to use libpng in my open pull request. I think we've shown well here the practicality of libpng in a real world use case for loading projects in a game engine and that it scales to the task much better.

Master/LodePNG (ms) Pull Request/libpng (ms)
fps6.gmk 0199.8 0093.0 ✔️
Wild Racing.gmk 2271.6 0734.0 ✔️
life.gm6 0031.0 0031.0
platform_engine.gmk 0165.0 0055.6 ✔️
destructible_blocks.gmk 0034.0 0030.4 ✔️
RobertBColton  
Alright, #1442 now switches the engine to libpng too. Only remnants of LodePNG at this point is in the makefiles, which I can do next. We can still use the shared sources to abstract libpng helpers so it's not as many additions. Overall, this looks like a pretty great change.
RobertBColton  
Now that #1533 has been merged and we've finished the transition to Google Protocol Buffers, I am coming back and finally going to get this into master. Won't take me much effort to redo it, and this is the best time now that we've eliminated a lot of the redundant places PNGs are handled.
RobertBColton  
Going to redo the benchmark first so we can see how it shifted as a result of moving the protos directly inside the compiler. I am also going to perform the benchmark differently this time, to include the compile time of the game since that's where the png stuff moved to.
Master/LodePNG (ms) Pull Request/libpng (ms)
fps6.gmk 07381.8 07220.2 ✔️
Wild Racing.gmk 16700.8 14153.4 ✔️
life.gm6 06704.4 07537.4
platform_engine.gmk 07266.6 07186.0 ✔️
destructible_blocks.gmk 06436.6 06359.6 ✔️
RobertBColton  
Ok, once again, I decided to redo the pr a totally different way, and this time with great results. #1548 is not only faster, it's also cleaner and more complete with LodePNG totally removed. I am now going to redo the benchmark one more time with the build times included.
Master/LodePNG (ms) Pull Request/libpng (ms)
fps6.gmk 07381.8 06820.0 ✔️
Wild Racing.gmk 16700.8 13587.4 ✔️
life.gm6 06704.4 06173.4 ✔️
platform_engine.gmk 07266.6 06651.4 ✔️
destructible_blocks.gmk 06436.6 05855.4 ✔️
RobertBColton  
Ok so after Josh pointed something out, I made another optimization in 9076bb3 that avoids additional allocations. This had another positive impact on the performance, so I am going to redo the benchmark again.
Master/LodePNG (ms) Pull Request/libpng (ms)
fps6.gmk 07381.8 05979.4 ✔️
Wild Racing.gmk 16700.8 12989.6 ✔️
life.gm6 06704.4 05442.8 ✔️
platform_engine.gmk 07266.6 05813.0 ✔️
destructible_blocks.gmk 06436.6 04890.8 ✔️

Then I later realized b63338e myself and believed it should speed up loading GMKs from GM8 and GM8.1 since those are the two versions that have transparency baked into the image. The only game that was in my original benchmark that fits this description is the Wild Racing GMK, so I redid the benchmark for it alone.

Master/LodePNG (ms) Pull Request/libpng (ms)
Wild Racing.gmk 16700.8 12796.6 ✔️
RobertBColton  
Closing as resolved by #1548. Cheers!
Please sign in to post comments, or you can view this issue on GitHub.