Hard Light Productions Forums
Modding, Mission Design, and Coding => FS2 Open Coding - The Source Code Project (SCP) => Topic started by: Nighteyes on February 19, 2011, 05:48:55 pm
-
Hi, I was making some CBanims for ED, and I encountered a strange bug, when trying to use JPG files for the CBanims, the in-game version rendered its colors incorrectly.
it looks like the Red and Blue color channels are flipped, here are some screens to show the problem, this was also confirmed with mjn.mixael.
this is the JPG frame from the CBanim:
(http://img573.imageshack.us/img573/4525/cbraynorflyby0008.th.jpg) (http://img573.imageshack.us/i/cbraynorflyby0008.jpg/)
this is the ingame cbanims in action with the wrong colors:
(http://img4.imageshack.us/img4/319/screen1668.th.jpg) (http://img4.imageshack.us/i/screen1668.jpg/)
*I'd love to see this fixed, as as far as I'm concerned JPG is the best format for animations like this...
-
This doesn't exactly look bad, but if it's supposed to be Sol, it'd better have correct colors.
Anyway, I agree that it should be fixed, but PNG should be a better alternative.
-
it has nothing to do with sol, and it does look bad if the colors are inverted...
now, I know that PNG is better, but its also creates a much bigger file-size, so if I can avoid that for a little thing such as CBanims, I'd like the option to do it...
*the colors look right when using the PNG format
-
*I'd love to see this fixed, as as far as I'm concerned JPG is the best format for animations like this...
You know where you should have reported this then. :p
-
Isn't there a know issue with colors getting flipped on some cards and a launcher flag for it?
-
Yeah, but it's only old (read: Obsolete) ATI cards. Sounds more like a problem in libjpg, tbh
-
Major Important Question:
Does this happen on Nightly Builds version 6979 or earlier?
Or does it only happen with Builds 6980 or later?
-
I confirmed it with 7014... I haven't tested yet with earlier builds.
-
You're such sissies.... just preswitch the colors!
(Kidding....!!! ----- running out before tomatoes hit me....)
-
Major Important Question:
Does this happen on Nightly Builds version 6979 or earlier?
Or does it only happen with Builds 6980 or later?
The question of the hour....
The button hover images also have a blue tinge.
-
It seems like ED has a custom button hover images, but no matching interface for CB anims. Happens all the time in WiP mods, interface must be simply unfinished.
-
Major Important Question:
Does this happen on Nightly Builds version 6979 or earlier?
Or does it only happen with Builds 6980 or later?
Will continue to ask until this is answered. Srsly.
-
personally I would really like to see the support for JPG fixed than this broken format added in...
-
Personally, I'd rather see the jpg support removed. But that's just me, I guess.
Also, I think you are severely misunderstanding the point of MjnMixaels inquirys. All this says to me is that if we're going to go ahead with implementing mng support, we will probably be forced top roll out our own reader library and conversion tool.
Oh, and jpg is broken? Where's the Mantis entry for that?
-
I've used JPG files for my entire first period of modding, and I've had no problems with them (I was using a crappy CRT monitor with 1024x768 screen, so I didn't noticed any quality loss, and I was using the lowest compression setting, so these files are still usable today if downsized and converted to a more effective format).
-
Yup, until us coders get more response from the original reporter this bug (if there is one) ain't getting fixed any time soon.
-
I confirmed it with 7014... I haven't tested yet with earlier builds.
I thought this was an answer... anyway I was using the latest builds at the time and asked mjn.mixael to test it as well, if you'd like I can test it again with the even newer builds, but its safe to say this occurs on builds 6980 and later...
I don't really know how to get hold of older builds...
-
There is a nightly builds forum. Use it.
-
ok, tested it with fs2_open_3_6_13r_INF-20110125_r6974 and with that version the colors displayed correctly, this is a bug from the later builds...
another thing that I found with this round of testing is FSO won't play JPEG, only if the file extension would be JPG it will play it, I thought its the same file type is it not? and a simple rename of the JPEG file to JPG will make FSO display it...
-
ok, tested it with fs2_open_3_6_13r_INF-20110125_r6974 and with that version the colors displayed correctly, this is a bug from the later builds...
another thing that I found with this round of testing is FSO won't play JPEG, only if the file extension would be JPG it will play it, I thought its the same file type is it not? and a simple rename of the JPEG file to JPG will make FSO display it...
It exactly the same file format. Both are completely valid extensions as per the JPEG standard, however FSO's file enumeration code does not look for .JPEG only .JPG, probably because FSO is limited to 32 characters for files names include the extension, dot and terminating null byte.
-
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.
-
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 :P I did a diff between the 6b libjpeg in the repositories people used to use and the actual libjpeg6b and found this :
> #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
-
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.
-
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)
-
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]
-
(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.
-
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.
-
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
-
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.
-
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.
-
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 :P
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]
-
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.
-
Excellent diagnostic work, DarkBasilisk. This is the kind of initiative we in the SCP really like to see. :)
-
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!
-
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]
-
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?
-
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 :P
-
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.
-
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.
-
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.
-
Cursory scan looks like Gentoo and Arch have it in their repositories, yum has it as well.
It's lacking from either Ubuntu or Debian's repos, but there's a provided .deb file from the devs that will work on Debian-based systems.
-
Wait, so people will need to download DLLs or something in addition to the FSO binary? I thought we were going to build the library into the program so that we wouldn't need any external dependencies?
-
Wait, so people will need to download DLLs or something in addition to the FSO binary? I thought we were going to build the library into the program so that we wouldn't need any external dependencies?
This is for the linux side, I think it's all packaged into the binary for windows. Right now for most of the external libraries, linux FSO compiles link against a system's installed libraries. I set things up to further that trend. I don't know for sure, but I'm pretty sure Zacam said, the external libraries for windows are all wrapped up into the FSO binary.
-
Yeah, Goober, whatever's been discussed so far, nothing would really change at all for any platform but Linux, aside from possibly having a different library including in SVN, but even changing Windows to libjpeg-turbo doesn't sound necessary for this to work. It would throw the warning that -turbo isn't being used though, but we could fix that simply by changing the library in SVN, or only throwing the warning on Linux.
-
As an update, it looks like we would want to switch all platforms to -turbo after all. Windows was confirmed yesterday to be exhibiting some form of bug with jpegs, and it's probably the same cause. So, the legwork has already been done for Linux in DarkBasilisk's post, now we just need to do the MSVC and Xcode projects.
-
So... then I repeat my previous question. :nervous:
-
Windows would compile -turbo just like it currently compiles libjpeg. No external DLL required for FSO at all.
Building on Linux would have the dependency of needing to have -turbo installed, or some method to defaulting to the SVN provided iteration if a system library is not found.
But most linux users are more savvy than windows users and can probably figure that out pretty easily, or they wouldn't be using linux.
No clue on Mac, but I'm sure Chief, Echelon9 and jg18 can figure it out and I can haul my mac out as well. Worst case scenario, it builds into the app what is in SVN.
-
Building on Linux would have the dependency of needing to have -turbo installed, or some method to defaulting to the SVN provided iteration if a system library is not found.
But most linux users are more savvy than windows users and can probably figure that out pretty easily, or they wouldn't be using linux.
As someone who has compiled on linux in the past, I can tell than the number of dependencies required is already large. One more or one less is absolutely no trouble.
-
Goob, the end result wouldn't be any different for the user. We'd just be switching out libjpeg for libjpeg-turbo in SVN, and compiling it in the same way we always have.
-
Building on Linux would have the dependency of needing to have -turbo installed, or some method to defaulting to the SVN provided iteration if a system library is not found.
But most linux users are more savvy than windows users and can probably figure that out pretty easily, or they wouldn't be using linux.
As someone who has compiled on linux in the past, I can tell than the number of dependencies required is already large. One more or one less is absolutely no trouble.
Agreed here.
-
Goob, the end result wouldn't be any different for the user. We'd just be switching out libjpeg for libjpeg-turbo in SVN, and compiling it in the same way we always have.
That's what I thought, but I wanted to make sure. Cool.
-
Just an update on the status of this...
There's been some problems getting the libjpeg-turbo library to work on all platforms so in the interim I've committed a patch to trunk (r7891) which does a colour byte swap in the FSO jpeg routines. I've tested this successfully with the WoD jpeg loading screens, when the nightlies are updated could some more people test this out, e.g. Nighteyes with the CBanims?
I'm going to continue to see if I can get libjpeg-turbo sorted out, but it may take some time and we don't want to delay a fix for this issue too much longer :)
-
We sure don't, this is definitely good enough to get 3.6.14 out the door, as people don't really use jpeg much these days anyway, but we don't want a regression.
-
That loop could be made more efficient. Instead of counting by ones and dividing by three each time, it should count by threes. Use k += 3 instead of k++.
-
The efficiency suggestion has been committed in trunk r7898 - thanks Goober5000 & Iss Mneur (apologies to Zacam who now has to backport to antipodes & 3.6.14 again! :))