New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
StbResizeImageConverter: update to stb_image_resize v2.04 #143
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No way, what a surprise PR -- didn't expect this to come up in my notifications at 2 AM, thank you! 🤩
The ASan test failure has me worried a bit, other than that it looks alright. For the SIMD stuff I may eventually play with building it several times with different flags and then dispatching with Corrade::Cpu, the default build flags are usually not enough to pick up AVX2 or F16C.
Also, the new library is big, heh.
src/MagnumPlugins/StbResizeImageConverter/StbResizeImageConverter.cpp
Outdated
Show resolved
Hide resolved
src/MagnumPlugins/StbResizeImageConverter/Test/StbResizeImageConverterTest.cpp
Outdated
Show resolved
Hide resolved
Opened an issue regarding the ASan error: nothings/stb#1526 |
Not sure if that fix was delayed by https://securitylab.github.com/advisories/GHSL-2023-145_GHSL-2023-151_stb_image_h/ in any way, worst case if nothing happens for several more days I'll just apply the patch myself here to have this merged. |
b0e77cf
to
1bb8961
Compare
To match the behavior of assert() and avoid performance regressions in release builds
1bb8961
to
f33fc5b
Compare
I updated to v2.04 which fixes the ASan errors. For some reason CircleCI timed out, not sure what to do about that 👀 |
Oh, is it out already? I was subscribed to the issue and to the PR and of course no mention was put in any of these. Strange release management.
That's just Ninja running with 38 threads because it queried that wrong from the system, and then randomly crashing or OOMing or getting stuck due to not enough RAM being available. Everything is fine 🔥 (I restarted the build.) |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #143 +/- ##
==========================================
- Coverage 97.08% 97.07% -0.01%
==========================================
Files 144 144
Lines 16447 16461 +14
==========================================
+ Hits 15967 15980 +13
- Misses 480 481 +1 ☔ View full report in Codecov by Sentry. |
Is there anything else that needs to be done from my side? This would be nice to have, but it doesn't have high priority. |
Nothing needed from your side, sorry -- this fell through the cracks. Merged as 8ca90cc...e84a3fc. |
Amazing, thanks |
Pretty straight-forward, and comes with new things!
Disclaimer: Only tested on MSVC, might be worthing checking on a few other platforms with and without SIMD. It's enabled automatically, but libraries tend to get automated SIMD detection wrong. Also haven't done any benchmarking to verify their "oooo so fast 🥥" claims.