search

s9w / binary_bakery

Translates binary information (images, fonts, shaders) into C++ source code.
MIT License
130 stars 9 forks source link

Replace const char* with string_view #9

Closed CihanSari closed 2 years ago

CihanSari commented 3 years ago

Please check carefully. I tested locally and encountered no problems. However, this PR contains a lot more changes in the application than the others.

string_view allows easy string comparison alongside other pretty features (non-null terminated strings with actual length etc.)

CihanSari commented 3 years ago

Wow, I didn't even consider compile_metrics and decoding_perf_mesaurement. Let me check them later and introduce them to cmake so we can ensure they are not getting ignored.

s9w commented 3 years ago

Both are certainly in an odd place. They really just exist to create readme plots and numbers and the code certainly didn't receive the same amount of love than the rest. We could also move them into another repo or so. The whole process of the measurements isn't entirely automated so the value of the code is limited.

CihanSari commented 3 years ago

The whole process of the measurements isn't entirely automated so the value of the code is limited.

When under cmake, we can perform measurements for each PR -- which could be a useful metric to measure modifications.

s9w commented 3 years ago

But do the CI builds come with any performance guarantees at all? I would be surprised if different results would be comparable.

Other than that: Sounds cool, but also like a lot of magic (aka cmake) work. But go ham if you feel like it. I would also be ok to axe them and keep them in another repo, manually running them when we think it's necessary.

CihanSari commented 3 years ago

It might give an idea, albeit not that easy to compare. If you move them outside the repository, then it would make this PR pretty much complete I think.

s9w commented 3 years ago

Alright, I took them out for now. I'll put them somewhere tomorrow or so. I think this is good to go then.

s9w commented 2 years ago

Hey Cihan! So I took out the decoding perf and other things, that should "resolve" the .data() issue from above. I forgot that there was apparently still a "requested changes" thing going on. I think this is all good then? Unless I misunderstood something.