[chief1983]

Basically, since you have with textures, 3 for the extension, 1 for the period, and up to 7 for the modifier (-normal is the longest so far I think?), you're already restricting people to 21 character base names.  And since many have hit that mark, suddenly having a 4 character extension could potentially break a lot.

[DarkBasilisk]

So hey kids, i've solved this mystery. The problem is because FSO code used to use it's own libjpeg from the SVN which was customized and modified for use. And then it switched over to dynamic linking, which would have been fine if someone way back hadn't changed the libjpeg library itself.

So suddenly we're using a libjpeg with different settings. What settings are different? The offset of Red and Blue when reading files I did a diff between the 6b libjpeg in the repositories people used to use and the actual libjpeg6b and found this :

Code: [Select]

> #include "jconfig.h" /* auto configuration options */
diff -r scpjpeg/jmorecfg.h stockjpeg/jmorecfg.h
314c314
< #define RGB_RED 2 /* Offset of Red in an RGB scanline element */
---
> #define RGB_RED 0 /* Offset of Red in an RGB scanline element */
316c316
< #define RGB_BLUE 0 /* Offset of Blue */
---
> #define RGB_BLUE 2 /* Offset of Blue */
Even with libjpeg8c if you fix the offset settings to match what FSO is expecting you get perfectly accurate colors. I'll get back to this thread soon with a patch if i can find a way to fix it on the FSO side, since we should stay compatible with the standard libraries unless we switch back to static linking.

-Cheers

[chief1983]

Wow.  This is a complete guess, but is this related to the colorspace FSO uses, being BGRA instead of RGBA?  I noticed that those offset changes match the locations of those colors in those strings.  The OpenGL BGRA extension isn't available on VirtualBox, and that's why I can't run it on my Linux VM.

[DarkBasilisk]

That'd do it. It'd explain why the original coder doing the libjpeg stuff uses these settings, since i looked it up and the alternatives to the matter get a bit complicated. Course looking that up prompted me to find out that libjpeg-turbo gives you more control and we could specify stuff like this without locking it in at compile time (which necessitates us doing statically linked libs since the user's own libraries wouldn't have these alternative settings)

[DarkBasilisk]

And we have a patch. At least provided I still remember how to make these. For right now it modifies the libjpeg stored in the svn to get the right #defines, and sets the config/compile process to static link the library. I can't think of many other graceful ways around this problem, the library doesn't allow you any other option for changing the color space, and flipping the bits on FSO side for each jpeg read in seems silly.

[attachment deleted by ninja]

[Zacam]

(Inception)We should probably go deeper(/Inception)

Meaning, that rather than having FSO color space BGRA, we should see if it's possible to RGBA instead. This would resolve future update issues and make VM operations a sane possibility.

[chief1983]

When he said "alternatives to the matter get a bit complicated", I took it that using RGBA instead was one of those complicated alternatives.  Would be nice though.

[DarkBasilisk]

I meant the alternatives to fixing it on the FSO level are :

1) Changing the jpeg library we're using to something else that does let you switch colorspace (a little involved and some foot work to make sure that doesn't break something, there's a few i could look into)
2) Implementing a workaround in jpegutils that flips all the red bits with the blue bits (That won't be massively slow, but it will slow things down a little bit)
3) Changing freespace's colorspace, which i have no idea what entails, but even if it was a mater of just flipping a few settings internally (which i doubt), it'd mean changing over all the code that lives in a world where the colorspace is BGRA.

Personally, i think especially since we have some of the devs who would like to see us slowly drop support for jpegs, most of these fixes would be too involved to be worth it. We used to use our own internal libjpeg in the builds, so I see little reason why to not go back to this, not to mention, as I discovered in one of my other posts, dynamically linking libjpeg (even if this problem with special compile settings didn't exist), introduces some portability problems. Unless it for some reason tanks performance, I see little reason to not go with this fix and be done with it, although we should probably leave a note somewhere about this in the documentation.

We don't want someone 4 months down the line to upgrade libjpeg, forget to reinstate the morecfg values, and then we have this problem all over again.

EDIT: A quick scan of the code as to #3 did find this line: // GL_BGRA_EXT is *much* faster with some hardware/drivers

[jg18]

EDIT: A quick scan of the code as to #3 did find this line: // GL_BGRA_EXT is *much* faster with some hardware/drivers

Maybe that was true when taylor wrote that line (SVN revision 2422, Sun Nov 13 06:44:18 2005 UTC) but isn't necessarily still true today? Just a thought.

[chief1983]

It does appear to still be the case, but we're looking at a library change or a jpegutils tweak that would make the need to edit the library itself unnecessary.

[DarkBasilisk]

It does appear to still be the case, but we're looking at a library change or a jpegutils tweak that would make the need to edit the library itself unnecessary.

I'm pretty sure it is. OpenGL documentation even says use BGRA.

So let's see : unless the libjpegturbo guy gets back to me with good news, i haven't been able to get that new library to link against FSO***, very annoying, i'm really starting to suspect that their shared version of the library is just plain faulty, since the static one works just fine, and nearly all the other open source programs i've poked into the code for don't use it.

***If someone else with a linux system could test this, i've posted a patch. Not try to fix it mind you, just try and compile. I want to make sure this just isn't an oddity with my system getting in the way***

Good news is the tests i did with the static library worked great. It's a one line statement in jpegutils (with the new library) to switch into BGR mode, and everyone's happy and properly colored. But barring hearing back from the library's devs, I've thrown everything i could think of and consulted all the help i could think of trying to get the shared lib to work with no luck, so here's our options :

1) Use the previous patch i posted that reinstates FSO's previous solution of using a modified libjpeg
2) Setup FSO to link against a user's static libjpegturbo libraries (still not the best solution, but it avoids the potential problem of forcing SIMD into the default FSO exec, which I know we can't do.)
3) Use another library (I don't immediately know of any others that are backwards compatible with libjpeg)
4) Use libjpeg as normal, implement a cruddy hack in jpegutils which flips the B bits with the R bits manually. (This will likely be much slower, and more confusing for future devs to try and maintain)
5) Ignore the problem : list it as an acknowledged issue in documentation and tell user's how to fix it at their own will

I see no compelling reason against static libraries though, so i'm inclined to recommend #1 or #2. Also, lacking windows/mac i haven't done any testing on either of those platforms, but i can predict that solution #1 will almost certainly work on the other platforms because it's still libjpeg, and #2 will most likely work as well.

[attachment deleted by ninja]

[chief1983]

I feel like #4 is a better option because it doesn't take any libraries changes and lets us keep the fix internally.  That's what documentation is for.

[Goober5000]

Excellent diagnostic work, DarkBasilisk.  This is the kind of initiative we in the SCP really like to see.

[DarkBasilisk]

Thanks Goober   And good news, I actually heard back from the dev and I was misunderstanding something about how this stuff works. Patch involved using shared lib libjpegturbo will be up soon.

To clear up anything about this option:
1) It'll attempt to find libjpegturbo's libraries, but by nature of how the process works, if the user only has libjpeg standard availible the compile will link against that, the color bug will still exist, but it's technically compatible for anyone that doesn't feel like updating their system libraries.
2) The code is fixed on the FSO side through a change to jpegutils (which, again, simply has no effect should the turbo library not be present)
3) Library installs alongside libjpeg, so there shouldn't be any huge compability issues with having to remove one from the host system.
4) If i get a chance i'm going to try and make the configure option to link with a static jpeg library work again once more as an optional feature to give people some more options.
5) Fixes for the windows compile/build system still need to be done, since i don't have windows i can't do those... Zacam i think said he'd look at that when he got the time
6) the libjpeg-turbo folder put in freespace hasn't been quite stripped down yet for filesize considerations

EDIT: As a side note i'm also having jpegutils throw a warning for debug builds that are linked with libjpeg instead of libjpeg-turbo. This is such an uncommon error and it's easy enough to forget to install the new libraries, it'll be nice to have a debug message that points out the problem clearly when a few months from now someone is complaining still about this issue.

Patch'll be up soon!

[DarkBasilisk]

And patch is up.

Changes:
* Added some various documentation about the library and possible install problems to INSTALL
* Linux build files have been editted a bit (configure.ac, Makefile.am, etc), more on that in a sec
* In addition to what was needed to link against the new library, the build files got some features added
     * the configure flag --with-static-jpeg, actually does something now. --with-static-jpeg=lib.a, will
        tell the compile to static compile against the given static library
     * there's also a configure flag -with-jpeg-include=DIR , to specify an alternate location for the jpeg h files
     * the default is of course to link to a shared system library, but these two flags save a lot of headaches
        should a system have a problem with installing libjpegturbo
     * i might have added a comment or two to top level's configure.ac
* New library/folder added to the top level : libjpeg-turbo-1.1.1
     * I've tested this without any problems. It's supposed to be backwards compatible with up to libjpeg6b,
        and FSO code does not appear to rely on v7 and v8 features despite using v8c.
     * It's supposed to be faster than regular libjpeg, but more importantly, it has more colorspaces supported
     * BGR colorspace works for reading in images, meaning we don't have to do anything crazy to make
        the color bits read correctly.
     * It works best when compiled with SIMD options, but I've tested compiling it without SIMD and it works
        just fine. Additionally, since it's linked to system libraries, it's up to the user what to use. Essentially I'm
        noting that I kept my promise about this new library not requiring FSO's default build to need SIMD.
* Changes to jpegutils.cpp
     * the header files for jpeglib.h were switched from an absolute location to <jpeglib.h>, specified by the make
        scripts
     * The makefile will specify two locations : first where it was set to look for the jpegturbo headers (configurable by
        ./configure options), then failing that, the old libjpeg headers (more on that in a second)
* Compatibility : while of course it won't resolve the color bug, for the sake of people that don't care about jpegs i've
   made all of this happily compile without libjpegturbo being installed
     * jpegutils will throw a warning to the FSO debug log if BGR colorspace isn't supported, but will run best it can
     * libjpeg is still included in the top level folder. If the build process (unix systems) can't find libjpegturbo, it will
        default to compiling against libjpeg. I've tested this to work on linux. jg18 tested it to work on mac.
     * In the event everyone's of the mind of "Basilisk get that older stuff out of there." it's relatively trivial to remove
        this and solely use the new library.

What hasn't been done :
* Windows build system doesn't know about the changes, but will likely compile against the old libjpeg and at least not break.
     * So someone will need to setup MSVC to handle the new library. My main accessible computer isn't windows, and I know
        nothing about MSVC, so i won't be able to be much help with that.
* I removed some of the unneeded bloat from the libjpeg-turbo-1.1.1 folder, but it could probably still be stripped down more.

Oh, and of course, it resolves this bug


[attachment deleted by ninja]

[chief1983]

Just want to double check one thing.  If it's compiled against libjpeg-turbo dynamically, but the user only has libjpeg installed, what would happen?

[DarkBasilisk]

Just want to double check one thing.  If it's compiled against libjpeg-turbo dynamically, but the user only has libjpeg installed, what would happen?

From the tests : successfully compiles against libjpeg. Bug still will exist, but FSO generates a helpful debug message when it notices that it can't use the colorspace extensions.

EDIT: This is currently only because of some of the stuff i set up, it's looking for the library in two places. It wouldn't automatically do this

[chief1983]

This is why I think a fix in jpegutils would be a better solution, unless it looks like enough distros have libjpeg-turbo already available in their repos.

[DarkBasilisk]

This is why I think a fix in jpegutils would be a better solution, unless it looks like enough distros have libjpeg-turbo already available in their repos.

Not as many distros have it in repo, however the project devs release a .deb file that installs everything satisfactorily. A number of our other presently required libraries are often much harder to install than this for certain distros. And there's the optional methods i added to make static compiles should anyone not want to bother with installing it. I maintained compatiblity on compiles as that option too.

And like i said, with libjpeg there's no jpegutil's fix that wouldn't be a horrendous liability. Right now, the jpeg library manages all the reading nice and happy and is probably quite optimized. Switching the bits manually is doable, but it'll be confusing to later devs who don't know what's going on, and it'll be very slow.

Switching to a new library *is* a big deal, but most of the work is on our side making all the build processes play nice, and a lot of that's been done by this patch. Linux (and it seems OSX) already work. Also, the fact that we can't cleanly do basic functions with it does not help the case for libjpeg.

Just want to double check one thing.  If it's compiled against libjpeg-turbo dynamically, but the user only has libjpeg installed, what would happen?

Ack, i just re-read what youd said and yea... it'd break. You're thinking about nightly builds I'd expect. If someone just downloads the exe and doesn't have it, they're screwed. But... SCP seems for major releases to use statically compiled libjpeg anyways. We can always go back to doing that if you're concerned about the compatibility with officially released exes.

[chief1983]

Ack, i just re-read what youd said and yea... it'd break. You're thinking about nightly builds I'd expect. If someone just downloads the exe and doesn't have it, they're screwed. But... SCP seems for major releases to use statically compiled libjpeg anyways. We can always go back to doing that if you're concerned about the compatibility with officially released exes.

I wasn't planning on changing it, that change to dynamic linking was intended to be permanent.  So if we started using libjpeg-turbo dynamically, that'd be the same setup for the release.  Now I want to check the major distros to see if they at least offer libjpeg-turbo dev libs.  I wasn't aware of any libraries we currently depend on that are difficult to install, in fact the Wiki lists the install steps for Gentoo, Debian-based, Ubuntu-based, and Red Hat-based distros to get the required libraries installed.