-
-
Couldn't load subscription status.
- Fork 17
vtfpp: byte order portability #79
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
base: main
Are you sure you want to change the base?
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.
It would be really nice if this library worked properly on big-endian devices like consoles/quirky computers, not much of a concern for me because I have so few big-endian devices (one, PS3) but yes I will take any fixes you deign to implement, thanks!
33ebaec to
68905dc
Compare
|
here we go, force pushed a rough example of what doing it at storage level would look like. can confirm that this produces correct primary texture data on either architecture (thumbnails and a few other things are still inconsistent), at the expense of exposing users to some wacky wrapper types. at a cursory glance it takes clang 19 |
|
I have some tests to visually test basic sdk2013 formats, L155 of |
…ator builds on linux
milestone: entire vtfpp test suite passes!
b0327c2 to
6206f0d
Compare
|
steady progress: entire vtfpp suite (incl extended) ostensibly passes on ppc & s390x; anything that depends on bitfields is still off visually edit: ok yeah upon further examination, the combination of (byte order portability) and (memory representation matching disk representation by default) simply is not accommodated by bitfields. so continuing to make this library portable would require a breaking api change concerning the pixel format structs (rip cute .r .g .b member access) whether the new underlying implementation be shifting and masking onto an appropriately-sized scalar type, or RFC, but barring any objections i'll do up a fresh range of v2 pixel formats, with uniform constexpr callable accessors, explicit constructors of initializer list, etc, use those under the hood, and declspec/attr deprecated the old ones. |
dadbea2 to
bb5d864
Compare
|
ok! |
| set(CMAKE_CXX_STANDARD 20) | ||
| if(MSVC) | ||
| # secret second flag to *actually* make msvc a c++20-compilant compiler | ||
| add_compile_options($<$<COMPILE_LANGUAGE:CXX>:/Zc:preprocessor>) |
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.
cmake/AddPrettyParser.cmake does this too, is this necessary to pull to a larger scope?
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.
i don't think it was applying in all the places ImageConversion.h was getting included, but i can double check
|
Holy Moly |
name sat better; reinterpret_le reads too much like pointer-to-pointer
0bbcacb to
f5bde62
Compare
|
ok |
938b80e to
8cbcea6
Compare
|
threw in some simple tests to make sure images parse properly and sure enough, significant work remains on BE :V at least the happy path from stock-standard 8 bit rgba png to just about any vtf pxfmt is perfect now |
|
after much messing about, it seems compressonator has weirder and subtler byte order dependencies than can be solved with a simple 32-bit swizzle before/after, when compressing. makes sense since those formats love crossing byte boundaries so much. genuinely a miracle that decompression is flawless. i am fully on board with your wish to nuke compressonator and will write an according reimplementation of appropriate formats, but it's beyond the scope of this pr. at the very least, none of the test cases segfault or throw under ppc64 or s390x. mips64 is boned but lol lmao. and a bunch of useful bonus test cases!! also, since i started, the test image for ARGB8888 has looked the wrong way round and the associated conversion case seems to reflect a typo, but i wasn't sure if it was accounting for some engine quirk where ARGB8888 isn't treated as what it says it's treated as, or something. if not i can fix that too anyway here's wonderwall |
|
Hot damn this is a lot thank you! I'll try to review over the weekend Also I'm pretty sure that format looks weird because I used "known good" VTFEdit to generate those back when I used Windows and I think that format might be busted in vtflib or something |
ok so in the process of fitting distance stuff into
ImageConversioni became quite wary of the manyreinterpret_cast<float *>kicking around, and sure enough, a small test program:resulted in a nonsense vtf full of nans on my powerpc box. at time of writing, 33ebaec represents the absolute minimum it took to roundtrip correctly from img->tex on a big-endian machine to tex->img on a little-endian machine, by just invoking bufferstream's templates with enough information for its own byte order stuff to kick in.
question: in your opinion, is this something worth caring about? ot1h, who's running sourcepp on a BE machine in 2025 besides me for shits and giggles; otoh, it is technically billed as a general-purpose library, and depending on host byte order feels icky in such a setting. if it is something worth caring about, i could go up and down making sure any float/u16/etc reinterpretations do the appropriate shift/mask song and dance routine (ime, compilers are smart enough to emit equivalent assembly to
reinterpret_caston LE targets, and put registers in LE mode on bi-endian targets) so intermediate structures are always pc vtf output order by default, and chuck down some appropriate unit tests; if not, that's cool too.