Linux patches review

[gregory]

I redid my xdg patches. It is now better implemented I hope. I split them into 3 parts so it is easier to understand. Any opinion are welcome.

+ 03_central_userconfig_function.patch: I create a new function in PathDefs that it returns wxStandardPaths::Get().GetUserLocalDataDir(). Then I used it instead of the direct call. It kept the same behavor for all OS

+ 04_xdg_config_home.patch: the xdg implementation. It tunes the previous patch to support the XDG_CONFIG_HOME variable. Technically it move $HOME/.pcsx2 to $HOME/.config/pcsx2 (or in the special user configuration). It is better to follow the xdg specification but it is not mandatory.

+ 05_move_data_to_config.patch: Move user data files to GetUserLocalDataDir on linux.

Note1: there are probably some WX improvement (full path and co) which I am not an expert.
Note2: 05 or 04 must be applyed after 03.

[cottonvibes]

because of these annoyances, i end up just implementing classes entirely in the header files. but this model has its own problem since it increases compilation time, and the compiler has to recompile everything referencing the header file whenever the header is modified. also probably bloats the executable.

Well the bigger problem is inter-dependent classes, which is what I run into more often. A contrived example is like this:

Code:

class joe
{
    Steve m_Steve;
    void SetSteve( const Steve& steve ) {
        m_Steve = steve;
    }
};

class Steve
{
    bool HaveSomeJoe( const Joe& joe ) const {
        // do something with Joe here.
        return joe.LikesBerries();
    }
};

Now Joe gets a compile error because Steve isn't defined yet. Swap classes around and Steve gets a compile error because Joe's not defined yet. This is the biggest limitation of C++: it's still basically a single-pass compiler in most ways. I run into that sort of thing enough that I'd just as well always do code/header separation. I tried to use two different coding styles for a while -- class/code together when possible and separate them only when dependencies occur -- but the number of times I had to separate was too often, and I decided I'd just prefer general uniformity over the occasional convenience of unified class/code.

The other big annoyance for me, and this might very well be fixed in VS 2010, is that code-in-headers breaks Intellisense much more frequently. When I was working on the DatabaseLoader for example, everytime I made any changes at all, I had to wait for Intellisense to update a bunch of crap before it'd work right again. Too many changes and the DB corrupted and it'd permanently lose some symbols. I almost never have that problem when working on anything else in PCSX2 (barring GSdx, which is also way too over-headerified).

yeah that is indeed another problem with the full-classes in header model.
sometimes you can get around this by using forward class declarations if you only need a pointer/reference to the class; but yeh its an annoying problem.


another problem that sometimes happens is when you define the class methods in the header itself, the definition can conflict with some identifiers from other headers and give you ambiguous compiler errors.
i had that happen with the database loader class when it was using the "<<" and ">>" operator overloads with string streams.
msvc was getting confused with other structures that also overloaded those same operators (some wx stuff and also some random pcsx2 code), and for some reason it thought it was ambiguous :/
had to move those functions into their own .cpp file to fix the problem :/

[Air]

Ok, make sense. I better understand the expected behavior.

One others idea that had in case multiple directories were needed. It is more clean in my opinion.
Just create a PathDefs::GetUserLocalDataDir() (or whatever the name) function that return wxStandardPaths::Get().GetUserLocalDataDir(). Then use the new function in the MiscPanelStuff and the AppInit. Purpose, all paths are in the same place and it is more easier to tune or change it.
In this case all user dirs will not be created, it is correct ?

About the dir settings on linux. Just to be sure we deal with the same definition (which I do not understand first )

Code:

WX                      |    linux                   | purpose
GetUserLocalDataDir    |    $HOME/.pcsx2       | configuration data. The usermode.ini. Just for information, with xdg stuff it is move to $HOME/.config/pcsx2. Directories that begin with a dot are hidden.

GetDocumentsDir         | $HOME/pcsx2           | user data.

The idea of the previous patch was to move the user data into the configuration directory in the default setup. Exactly for the reason you mention "not to go cluttering up the user folder", I just take it in the wrong way

Sorry for the delay on this: yes, this seems good to me. If WX's behavior for GetUserLocalDataDir on Linux isn't ideal, then we should make our own and use that instead (it can default to WX's implementation for MSW, and use your preferred method for Linux). Example:

Code:

wxString GetUserLocalDataDir()
{
#ifdef __LINUX__
    return L"$HOME/.pcsx2";  // or whatever you rather it to be.. ?
#else
    return wxStandardPaths::Get().GetUserLocalDataDir();
#endif
}

... and then replace all the instances of wxStandardPaths::Get().GetUserLocalDataDir() with our own local version.

I'm pretty sure that's what we're aiming for. I'll let you do the legwork and commit though. I think you have patches that are mostly done already anyway.

[gregory]

OK, I commit my patches. Except one. I want to be sure that everyone agree with it.

The last part is to move (in linux default configuration) user data from $HOME/pcsx2 to $HOME/.config/pcsx2. See the small patch attached for details.

I feel better this way but it is not an objective decision

[Air]

OK, I commit my patches. Except one. I want to be sure that everyone agree with it.

The last part is to move (in linux default configuration) user data from $HOME/pcsx2 to $HOME/.config/pcsx2. See the small patch attached for details.

I feel better this way but it is not an objective decision

Going to bump this. Do any other Linux users have an opinion on this matter? I'll clarify in case the topic is confusing:

Should PCSX2, on first run, default to:

1. $HOME/pcsx2
2. $HOME/.config/pcsx2

The first is what 0.9.7 r3119 defaults to. The second is proposed as a new default since it would hide PCSX2 files by default and reduce user dir clutter. Users would have the option to manually specify $HOME/pcsx2 instead during the first time wizard, if they want.

My take: (but I'm not a Linux user, so beware!) -- I think $HOME/.config/pcsx2 should be the "optional" one (or any subfolder of the users' choosing). Most of the files that PCSX2 places in its /pcsx2 folder are things most users will want to get direct access to sooner or later. Examples:

BIOS files (needed for running!)
Screenshots
Memorycards (swapping, making backups, etc)
Logs (which are needed more in emulators than many other types of apps)

PCSX2 on Windows has some ability to abstract away from needing these folders readily visible to the user; since it has the Open in Explorer button. Unfortunately that does not work reliably in Linux (which would be nice if it were fixed, admittedly; though I think its window-manager dependent). And even with the Open buttons working, data files that a user can and will want to move/copy/rename/edit directly probably shouldn't default to being hidden from view.

[wingnux]

My vote is for the $HOME/.config/pcsx2 option. I've always thought it should be that way, for us linux users it's pretty messy to have program config folders on our $HOME.

I also agree that the folder location always should be optional with the 2nd option as the default one.

PS.: Currently the wizard creates a "PCSX2" (upper case) folder on the $HOME dir as default option.

[loicm]

$HOME/.config/pcsx2 would be the best solution. Linux users are used to go into hidden folders for all manual configuration.

But please use $XDG_CONFIG_HOME instead of a hard coded path, that way you allow for distributions and users preference. See the XDG Base Directory Specification. Most of the software actively developed already use it.

[gregory]

Actually it is the XDG_CONFIG_HOME which is implemented. The $HOME/.config is only the default value.

[gregory]

Air,

I've got a new patch/idea for improving the memcpy function. It does 2 things:
* do not use the shr (so *2 for everythings). Purpose avoid to save qwc
* Use "sub qwc 1" at the beginning. It is a little tricky but it avoids to do a "cmp qwc 1" in 32bytes loops. And in the 16bytes copy, qwc = 0 <=> 16 bytes block ; qwc = -1 <=> nothing to copy.
- I do not know if the type (and how to do it properly) of qwc must be updated (signed int instead of unsigned int)

In the end, we gain a register space, a mov instruction and replace a shr/jump by a sub/jump.

Note, it is the linux version but it can be easily done on windows.


As a side question, is there any possibility to dump the frame rate, to detect code improvement ?

[jmeloranta]

...talking about wx.... has anyone noticed that the latest svn does not build with non-unicode version of wx? (data types char and wxChar are identical and some operator overloadings will fail because of this) It compiles fine with unicode wx.