I found some time to toy with cmake again. I made a few changes, mostly cleanups.
In fact: always use project libraries and link them static to the plugins or the emu itself. Except portaudio, this will take a while, it's huge.
Do not use system ones.
Added liba52 to the build system and link against it.
While testing I encountered a few runtime errors, like CDVDiso is not working properly, undefined reference to _fdopen, I believe something goes wrong with bzip2, because _fdopen is undefined in the static lib.
Another problem occured in spu-x, here a undefined reference to HostSys::MemProt.... This is strange, becaus if I execute nm on the Utilities lib, the reference is available. Well, I'll play with that, propably some stupid mistake by me.
I attached the patches, all of them are made against trunk. So if you patch more than one, you propably will get problems. Me to blame, something went wrong with the patch order.
As always, feel free to comment, delete or patch the changes.
I'm stil trying to understand why inline break the code... In this process, I found a strange switch that seems missing some break statement. Not sure what the code must do but seem wrong. Could you give a look ?
File: plugins/spu2-x/src/Mixer.cpp
693- // Boost reverb volume
694- int temp = 1;
695: switch (ReverbBoost)
696- {
697- case 0: break;
698- case 1: temp = 2;
699- case 2: temp = 4;
700- case 3: temp = 8;
701- }
05-15-2010, 02:47 PM (This post was last modified: 05-15-2010, 02:49 PM by arcum42.)
Yeah, that looks like a mistake. The case statements would fall through, making the reverb boost either off or on max. I went ahead and added break statements, since if the intent really was for it either to be off or at top volume, there are better ways to put it in there...
@Athos: I'll go ahead and test those patches tomorrow, and see what I can come up with...
@athos, the linking, due to interdependancy between Utilies and x86emitter, is wrong in spu2x use the following line
target_link_libraries(${spu2xName} Utilities x86emitter Utilities)
About the patches. I thinks is a bad idea to always link agains project library. Have a choice is probably the best solution moreover it won't hurt. Although the code need some improvement.
The problem is distribution policy (at least in debian and surely ubuntu, and probably others), they did not allow to use 3rd party source. You must link your executable against distribution library.
The rule is 1bug fix = a bug fix for everyone. A regression = a regression for everyone = and so the library will be fixed.
(05-15-2010, 04:33 PM)gregory Wrote: @athos, the linking, due to interdependancy between Utilies and x86emitter, is wrong in spu2x use the following line
target_link_libraries(${spu2xName} Utilities x86emitter Utilities)
okay, that's strange, but if it's working, I'm fine with that.
Quote:About the patches. I thinks is a bad idea to always link agains project library. Have a choice is probably the best solution moreover it won't hurt. Although the code need some improvement.
The problem is distribution policy (at least in debian and surely ubuntu, and probably others), they did not allow to use 3rd party source. You must link your executable against distribution library.
The rule is 1bug fix = a bug fix for everyone. A regression = a regression for everyone = and so the library will be fixed.
Well, I agree, it's always better to link against system libraries, but the idea behind this was, to use the project ones, so the library base is the same, on Linux and Windows.
Of course its possible to add a switch to enable/disable system libraries, but this will make the whole build system generation complicated, with a lots of if etc..
I don't mind this, I can do this, this is not a problem, but like I said, this will end up in many if's.
Okay, you people, all that are using cmake or want to use it and all pcsx2 devs, just tell me, what you want, and I'll try to 'implement' it. I'm pretty good at cmake now, at least I do beleave that.
Of course there are also problems with cmake, like 32/64 bit compilation, there is no easy solution for that, one would be a 32 bit chroot, the other one, would be a 32 bit cross compiler.
I also wrote a script to use the mingw32 cross compiler, but first I need to port pcsx2 to mingw.
I also changed the behaviour of cmake. Resources are generated, but also deleted on clean, added a install script, changed the app folder structure to be more *nix-like.
So, I just need to know, what you people want, thats all, .
Well I do not thinks the build will become too complicated with a bunch of 'if'.
I agree with you, first question is what people whant. I see 4 modes. (external = distribution, internal = 3rdparty dir)
1/ full external
2/ full internal
3/ mix external and internal. External highest priority.
4/ mix external and internal. Internal highest priority.
Note1: Well full internal needs proper definition. There is plenty of libraries (gtk,wx,nvidia-cg, etc...) that are in fact external.
Note2: At the moment there is something very bad in my opinion. You could link with external library but actually uses the internal include...
Note3: some simplification could be done instead of doing thing like that in plugins. Plugins does not need to be aware of the origin of the library.
-------------------------------------------------------------
# link target with SoundTouch
if(projectSoundTouch)
target_link_libraries(${zerospu2Name} SoundTouch)
else(projectSoundTouch)
target_link_libraries(${zerospu2Name} ${SOUNDTOUCH_LIBRARIES})
endif(projectSoundTouch)
-------------------------------------------------------------
Just do in searchforstuff.cmake when selecting the internal soundtouch
set(SOUNDTOUCH_LIBRARIES SoundTouch)
and in the plugin: target_link_libraries(${zerospu2Name} ${SOUNDTOUCH_LIBRARIES})
-------------------------------------------------------------
The cleaning of ressource is a good idea. Maybe you could also have a look into gs plugins with ps2hw.dat file that is not delete.
Ok I found why inline break my code. There is an error in the asm that I send you ... Sometimes gcc could be tricky. The imul function updates both eax and edx registers, so both register must either be an output or in the clobber list. The fact is you cannot put a register both in the clobber list and in the input contraint. So eax register must be defined as an input and output... I thought naively that an input register was also consider clobber but seems that I was wrongs... So please found a patch that fix this silly behavior.
**********************************************************************************
In the process I done a lots of reseach of inline stuff. And I found some interessing stuff. May I share them with you.
Some __forceinline cause some problem of gcc. To skip the error I replace them with inline (my understanding was "inline if possible else do nothing"). But believe me or not, gcc do the inlining in this situation. I do not know why, neither if it is allowed !
The bad consequence is missing symbol. example clamp_mix function
* Mixer.cpp. The function is defined here and used.
* Mixer.h contains the header with extern keyword
* Reverb.cpp use also the clamp_mix functions
The issue is after the compilation of mixer.cpp into object. The symbol clamp_mix does not exists (because the function is inline the function does not exist). So there is a missing symbol in Reverb.cpp.
If I understand all my internet reading. The function must be defined in a header and then the header file includes in Mixer.cpp and Reverb.cpp. An others solution to not polluate the header is to create a specific file (extension .inl for example) and to include it.
Note: there are some .inl file in the project.
Note2: there is also maybe some trick with extern/inline, definition/declaration, special compiler option but I did not understanding the all parts and it does not seem to work.
05-18-2010, 10:30 PM (This post was last modified: 05-18-2010, 10:38 PM by gregory.)
Here some patches that you could find usefull.
+ 33_clean_fpic.patch: clean cmake now that the asm is fix
+ 34_add_ndebug.patch: add ndebug everywhere. Expect potentially some inline failure...
+ 36_add_PADsetSettingsDir_part1.patch & 37_add_PADsetSettingsDir_part2.patch:
I update the zeropad & onepad plugins to use ini setting like others plugings. Current implementation does not write in the good place when you install the plugins in a non standard directory.
Note I add the callback PADsetSettingsDir, I do not know if is ok (a comment in PS2Edefs.h said that a API is obsolete). Do not know impact on windows build.
Ah, one question, spu2x writes lots of temp config file in my $HOME (like .pcsx2AbIDIg). Is it an intended behavior ? Or my debian installation is troublesome ?
Edit:
These patches and the previous are based againts rev 3029
Thanks for the patches. I'll make sure I get the assembly squared away. Hopefully ndebug won't be too bad, because I took care of a lot of that on the code::blocks side recently.
As far as the impact on the Windows build of ZeroPad & onepad, don't worry about it. Zeropad's Windows version is majorly broken, and would need a rewrite to even be functional. Because of that, it was yanked from the main project files on the windows side. And as far as Onepad goes, it was based on Zeropad, and I believe I removed most of the Windows code out until it could be rewritten, though I left things set up where it could be done.
I don't think spu2-x should be writing temp config files in your home directory, but then, it hasn't gotten a lot of testing on Linux. I'll have to look at that...
Here some minor patches that could be useful or not.
+ 31_add_cdvd_cmake_and_fix.patch : I had it so... It adds the cmake for CDVDlinuz plugins. It will not work directly because of version.h wrongly used. In local I rename the version to do a quick test, so it is the only issue.
+ 38_remove_unused_define.patch: Just remove some define that are not used anywhere.
+ 39_clean_cmake.patch: Multiple things, need to sort good and bad.
* Add a detection of liba52: FindA52.cmake
* Add some options to control the 3rdparty build, you can select 1 library or all. See main CMakeLists.txt for the new options list. What is your opinion ?
Example to build with internal soundtouch
cmake -DFORCE_INTERNAL_SOUNDTOUCH=TRUE CMakeLists.txt
Note1: must be properly calculated before including SearchForStuff. And so I move some include.
Note2: Feel free to update the default value. But in my opinion it is better to control it from cmake invocation.
* In case of internal library also set variable _LIBRARIES. The purpose is to separate plugins and third party. Plugins do not need to know origin of library but the library name.
Note: just an idea. Maybe move all build configuration variables into a specific file.
.
... Sometimes gcc could be tricky. The imul function updates both eax and edx registers, so both register must either be an output or in the clobber list. The fact is you cannot put a register both in the clobber list and in the input contraint. So eax register must be defined as an input and output... I thought naively that an input register was also consider clobber but seems that I was wrongs... So please found a patch that fix this silly behavior.