RobertBeckebans / RBDOOM-3-BFG

Doom 3 BFG Edition source port with updated DX12 / Vulkan renderer and modern game engine features
https://www.moddb.com/mods/rbdoom-3-bfg
GNU General Public License v3.0
1.38k stars 247 forks source link

Jpeg 9b #345

Closed wtfbbqhax closed 2 months ago

wtfbbqhax commented 7 years ago

Latest version of IJG Jpeg, removes decent chunk of responsibility from the top CMakeLists.txt.

Small part of some prep work for decoupling the circular dependencies and a cmake refactor.

coldtobi commented 7 years ago

I'm wondering if we should utilize libjpeg-turbo? Libjpeg-turbo is more widely supported by distributions and e.g the default libjpeg implementation. As far as I see, we do not need the features of jpeg-9... Additionally ijpeg-9b breaks API, while libjpeg-turbo maintains the libjpeg-8 API.... Just my 2c...

wtfbbqhax commented 7 years ago

@coldtobi 1 function call had to change for compatibility with the engine, not exactly a significant difference. Understandably, I've not made use of any new functionality.

Particularly, I'm updating all the libs/ since several of the packaged have very very bad security vulnerabilities (PNG being the worst).

libjpeg-turbo did cross my mind, specifically because I would rather use git-submodules for easier management, keeping them up-to-date and ijp doesn't seem to have a public repo or presence to facilitate such.

coldtobi commented 7 years ago

Am Samstag, den 24.09.2016, 11:59 -0700 schrieb Victor Roemer:

(disclaimer: written with my Debian Developer hat on)

@coldtobi 1 function call had to change for compatibility with the engine, not exactly a significant difference. Understandably, I've not made use of any new functionality. 

Particularly, I'm updating all the libs/ since several of the packaged have very very bad security vulnerabilities (PNG being the worst). 

Debian has migrated to libpng1.6 earlier this year, so I'm running rbdoom3 already on libpng1.6 for Debian with it.

libjpeg-turbo did cross my mind, specifically because I would rather use git-submodules for easier management, keeping them up-to-date and ijp doesn't seem to have a public repo or presence to facilitate such. 

Debian is using libjpeg-turbo as default implementation, so I'm using it there as well for rbdoom3 without known problems so far.

tobi

wtfbbqhax commented 7 years ago

Am Samstag, den 24.09.2016, 11:59 -0700 schrieb Victor Roemer:

(disclaimer: written with my Debian Developer hat on)

Uh oh, now we getting some legit feedback 😜

Debian has migrated to libpng1.6 earlier this year, so I'm running rbdoom3 already on libpng1.6 for Debian with it.

Are you sure? LibPNG 1.5, 1.6 made various datatypes opaque and incompatible from the doom3 implementation. Maybe Debian patched libPNG to retain this compatibility?

Otherwise, it could be that CMakeLists.txt is not actually using the system libraries as expected. This setup should really provide you as a maintainer the option to "require" the use of system dependencies vs builtins to make sure this doesn't happen.

Debian is using libjpeg-turbo as default implementation, so I'm using it there as well for rbdoom3 without known problems so far.

Oh! That's good enough for me, I will look at using it instead 👍

coldtobi commented 7 years ago

Am Samstag, den 24.09.2016, 14:31 -0700 schrieb Victor Roemer:

Am Samstag, den 24.09.2016, 11:59 -0700 schrieb Victor Roemer:

(disclaimer: written with my Debian Developer hat on)

Uh oh, now we getting some legit feedback 😜

just disclaiming because I'm selfish: If you use the libs we have in Debian this will help me to reduce the effort to maintain the rbdoom in Debian :)

Debian has migrated to libpng1.6 earlier this year, so I'm running rbdoom3 already on libpng1.6 for Debian with it.

Are you sure? LibPNG 1.5, 1.6 made various datatypes opaque and incompatible from the doom3 implementation. Maybe Debian patched libPNG to retain this compatibility? 

Yepp, petty sure as I drove the migration in Debian ;-) Also, the PR  https://github.com/RobertBeckebans/RBDOOM-3-BFG/pull/293 is the patch I carry in Debian https://anonscm.debian.org/cgit/pkg-games/rbdoom3bfg.git/tree/debian/pa tches/libpng16.patch

Otherwise, it could be that CMakeLists.txt is not actually using the system libraries as expected. This setup should really provide you as a maintainer the option to "require" the use of system dependencies vs builtins to make sure this doesn't happen.

I ensure that in Debian .. all external libraries' source is removed when I import a new snapshot 

Debian is using libjpeg-turbo as default implementation, so I'm using it there as well for rbdoom3 without known problems so far.

Oh! That's good enough for me, I will look at using it instead 👍 

coldtobi commented 7 years ago

Debian has migrated to libpng1.6 earlier this year, so I'm running rbdoom3 already on libpng1.6 for Debian with it.

Are you sure? LibPNG 1.5, 1.6 made various datatypes opaque and incompatible from the doom3 implementation. Maybe Debian patched libPNG to retain this compatibility?

Maybe I should be more verbose on this.

Libpng1.6's code is not patched at all in Debian, it's only patch we carry there is about to make its config tool compatible with multiarch setups. [r]

The change you're describing that several structs are now private and only [sg]ettable through its API are a feature since libpng1.5 [1] after long being depreciated already. The API-way is already around since before 1.2 (that's why my PR #293 will be transparent when used with libpng1.2) and libpng 1.4 started to warn loudly if direct-access to those structs were detected.

So the change is just not to access the struct anymore but to utilize the API (which rbdoom3bfg did, except of the few locations where #293 fixes them.)

[r] https://salsa.debian.org/debian/libpng1.6/tree/master/debian/patches [1] http://www.libpng.org/pub/png/src/libpng-1.4.x-to-1.5.x-summary.txt

(Updated Link [r] as Debian moved its git repositories to a new place (salsa.debian.org)

Akira13641 commented 3 years ago

FYI, currently, you can succesfully build RBDOOM 3 BFG after directly replacing the files in libs/jpeg-6 with those from jpeg9d, as long as you pass the USE_NEWER_JPEG preprocessor flag. All I did (on Windows) was first call ren *.c *.cpp in the jpeg9d source folder, and then use Winmerge to replace everything in libs/jpeg-6 that registered as being "different" with the newer equivalent, also delete files that registered as no longer existing in jpeg9d. Lastly, it was also necessary to rename the jconfig.vc from the jpeg9d source folder to jconfig.h, and also replace the libs/jpeg-6 one with that.

Additionally, as far as libpng, you can at the very least replace the libs/png files with those of version 1.4.22 without changing anything else, which is what I typically do locally. Even doing that for the master branch would be a good step in the right direction versus the current 1.2.49 version being used, IMO.

DanielGibson commented 3 years ago

TBH, just throw libjpeg and libpng away and use stb_image.h, that causes a lot less trouble

coldtobi commented 3 years ago

Sorry for repeating myself: Debian compiles rbdoom3bfg against libjpeg-turbo, which is for many Linux distris the default libjpeg implementation. It does not need patching rbdoom3bfg at all.

TBH, just throw libjpeg and libpng away and use stb_image.h, that causes a lot less trouble

Well, not that sure about the quality of that library: https://security-tracker.debian.org/tracker/source-package/libstb A few of those still open CVEs are from 2019, partly with fixes available as PRs…

Akira13641 commented 3 years ago

Moving to libstb would also require actual changes to the code of RBDoom3 BFG itself, whereas like I said above the codebase is in fact currently 100% compatible as-is with the very latest version of libjpeg as long as you use the USE_NEWER_JPEG flag.