-
-
Notifications
You must be signed in to change notification settings - Fork 699
fix some int32 limits on windows #3935
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
Conversation
422ea5f
to
0964d60
Compare
Perhaps we should just use |
This looks good! I'll test this out on Windows later.
I think clang-tidy's Though, that could also generate false-positives, for example: libvips/libvips/foreign/archive.c Line 180 in f1e1407
|
It looks like libvips/libvips/iofuncs/generate.c Lines 626 to 629 in 0964d60
Applying the same fix on that function (see e.g. commit kleisauke@af7c7bc) fixes the test case mentioned in #3918 (reply in thread) on my Windows 11 PC. $ vips black x.v 1400000 10000 --bands 3 --vips-progress
vips x.v: 1400000 x 10000 pixels, 24 threads, 1400000 x 1 tiles, 512 lines in buffer
vips x.v: done in 18.5s
$ (Get-Item x.v).Length
42000002024 BTW, should this PR target the |
write() on windows uses int for byte counts, leading to errors with very wide (more than 1.5m pixels wide) images
0964d60
to
4af2e39
Compare
Targeted against the @pdbourke, I credited you in the changelog, I hope that's OK. |
Ah thank you for fixing this Kleis. It looks great, let's merge. |
I merged to master as well. |
write() on windows uses int for byte counts, leading to errors with very wide (more than 1.5m pixels wide) images.
libvips was assuming posix:
But windows has:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/write?view=msvc-170
We probably have some other int32 limits still :( we should make some test cases and do more investigation, especially on windows.
Referring discussion:
#3918 (comment)