FreeRDP / FreeRDP-old

DEPRECATED VERSION - Check https://github.com/FreeRDP/FreeRDP : FreeRDP is a free remote desktop protocol client
http://www.freerdp.com
Apache License 2.0
80 stars 883 forks source link

GDI Refactoring + Consistency Improvements #29

Closed awakecoding closed 13 years ago

awakecoding commented 13 years ago

These changes consist of refactoring of GDI components, along with the addition of an optional command-line option in xfreerdp to select between hardware (X11-based) or software (libgdi-based) GDI rendering. The DirectFB UI has been updated along with the same command-line option, except that the hardware-accelerated GDI implementation is just a stub at this point. libgdi now only uses the gdi_ prefix for files that implement parts of the Microsoft GDI API. Unit tests have been cleaned up as well, where unused tests have been removed. The test executable can now be passed a list of test suites to execute.

otavio commented 13 years ago

@awakecoding I do like the general idea of it but I do object about the renaming of the libraries.

It is quite difficult to discover what libutil came from when looking at a bloated /usr/lib as does for libgdi and librfx. I prefer to have them all prefixed with libfreerdp. I'd suggest to use libfreerdp- as prefix.

I will review the rest of changes tomorrow.

awakecoding commented 13 years ago

@otavio: actually, I made those commits after creating this pull request, but apparently they get automatically added

let's not merge until we figure out the best solution for library names. libfreerdp_whatever is quite annoying though...

otavio commented 13 years ago

@awakecoding sure.

I wasn't suggesting libfreerdp_whatever but libfreerdp-whatever. I don't think it is bad since it give us a lot of credit if something use for example our libfreerdp-rfx or libfreerdp-gdi. I really think it is worth it.

awakecoding commented 13 years ago

Actually, besides the fact that prefixing everything with libfreerdp- makes long names, what annoys me the most is how unreadable it is when you look at different library names. Maybe a compromise would be to use libfreerdp_* instead of libfreerdp, since it would clearly separate the actual library name out of it. I looked on my system, many other library names use this type of convention when more than one library is provided by the same project. We'd get:

libfreerdp_gdi libfreerdp_rfx libfreerdp_kbd libfreerdp_utils libfreerdp_chanman

awakecoding commented 13 years ago

@otavio: just saw your comment after replying

we still have another unresolved issue with the naming convention, how is libfreerdp different from all the libfreerdp- prefixed stuff? libfreerdp is for the network stuff, yet it has a name implying it a superset of all libfreerdp- libraries

maybe we should get rid of the libfreerdp name altogether:

libfreerdp -> libfrdp-core libfreerdputils -> libfrdp-utils libfreerdpchanman -> libfrdp-chanman

etc...

awakecoding commented 13 years ago

Ok, maybe libfrdp is overkill, we're just saving 3 letters :( libfreerdp does look quite long but I guess we can stick with it. argh.

I suggest the same for renaming libfreerdp though, let's name it libfreerdp-core

otavio commented 13 years ago

@awakecoding I'd use full name with dash, not underscore. As I said it give us credit and I think it is better for us.

frdp doesn't means nothing to outsiders. I agree about adding -core for main library thought.

otavio commented 13 years ago

Agreed.

awakecoding commented 13 years ago

Did a simple test on my machine to figure out which one between the underscore and the dash was more popular:

awake@workstation:/usr/lib64> ls -l __ | wc -l 260 awake@workstation:/usr/lib64> ls -l -_ | wc -l 1533

your suggestion of the dash therefore wins the popularity contest ;)

ok, let's move on with libfreerdp-*, along with libfreerdp becoming libfreerdp-core

awakecoding commented 13 years ago

Ok, I just finished enforcing the new naming convention on the entire code base

librairies now use the libfreerdp- prefix, and libfreerdp has been renamed to libfreerdp-core

Unless there is something else, I guess this could be merged soon

otavio commented 13 years ago

I think it all seems good but I also believe someone needs to build and test it into Windows before we merge this stuff in.

awakecoding commented 13 years ago

@otavio: I started updating the Windows port yesterday, I'll finish that + work on the other things you've just mentioned

awakecoding commented 13 years ago

The Windows port should now be properly updated with the latest changes, did you see anything else?

otavio commented 13 years ago

Even thought it is already merged I'd like to say it seemed OK for me.

Please wait a bit more of a reply next time. A day or two wouldn't hurt.