Thank you for your reply Marc-André.
I agree that it’s hard to understand how each variable is used and
that can of course be solved with more comments and documentation, but
some of that can also be solved with separation of the variables. It’s
not a big change that I suggest. Maybe you misunderstand what I was
trying to explain. My suggestion is to move all variables that are
used only in the clients and not in the freeRDP libraries into a sub
structure - like this:
struct client_settings
{
ALIGN64 BOOL AllowFontSmoothing; /* moved from struct rdp_settings 961 */
ALIGN64 BOOL DisableWallpaper; /* moved from struct rdp_settings 962 */
ALIGN64 BOOL DisableFullWindowDrag; /* moved from struct rdp_settings 963 */
ALIGN64 BOOL DisableMenuAnims; /* moved from struct rdp_settings 964 */
ALIGN64 BOOL DisableThemes; /* moved from struct rdp_settings 965 */
ALIGN64 BOOL DisableCursorShadow; /* moved from struct rdp_settings 966 */
ALIGN64 BOOL DisableCursorBlinking; /* moved from struct rdp_settings 967 */
ALIGN64 BOOL AllowDesktopComposition; /* moved from struct
rdp_settings 968 */
... and more...????
};
typedef struct client_settings clientOnlySettings;
struct rdp_settings
{
....
clientOnlySettings *clientSettings;
...
}
And then use that like this:
if (settings->clientSettings->DisableWallpaper)
settings->PerformanceFlags |= PERF_DISABLE_WALLPAPER;
if (settings->clientSettings->DisableFullWindowDrag)
settings->PerformanceFlags |= PERF_DISABLE_FULLWINDOWDRAG;
if (settings->clientSettings->DisableMenuAnims)
settings->PerformanceFlags |= PERF_DISABLE_MENUANIMATIONS;
Best regards
- Arvid
2013/1/12 Marc-André Moreau <marcandre.mor...@gmail.com>:
> Hi,
>
> I have to disagree: this would create a lot of duplication and therefore
> causing even more confusion and making the code harder to maintain.
>
> Performance flags are a special case. To process them correctly in the
> client, you need to separately consider them and then construct the final
> PerformanceFlags value after all parameters have been parsed. One may set a
> connection type which involves setting specific performance flag defaults,
> and then individually override some of the performance flags. The connection
> type is also very important for RemoteFX which requires the type to be set
> to LAN.
>
> Here are the settings you refer to:
>
> /* Client Info (Performance Flags) */
> ALIGN64 UINT32 PerformanceFlags; /* 960 */
> ALIGN64 BOOL AllowFontSmoothing; /* 961 */
> ALIGN64 BOOL DisableWallpaper; /* 962 */
> ALIGN64 BOOL DisableFullWindowDrag; /* 963 */
> ALIGN64 BOOL DisableMenuAnims; /* 964 */
> ALIGN64 BOOL DisableThemes; /* 965 */
> ALIGN64 BOOL DisableCursorShadow; /* 966 */
> ALIGN64 BOOL DisableCursorBlinking; /* 967 */
> ALIGN64 BOOL AllowDesktopComposition; /* 968 */
>
> The core FreeRDP libraries will make use of PerformanceFlags, while the
> common client library will make use of all others for the construction of a
> proper PerformanceFlags value (client/common/cmdline.c). PerformanceFlags
> corresponds directly to the protocol variable which is sent by the client
> and received by the server, and the other variables are simply helpers which
> help with proper construction and analysis of PerformanceFlags.
>
> If we don't have these helpers in rdpSettings, then each client will need
> its own equivalent to properly parse the parameters in any order and
> properly construct the final value like this:
>
> settings->PerformanceFlags = PERF_FLAG_NONE;
>
> if (settings->AllowFontSmoothing)
> settings->PerformanceFlags |= PERF_ENABLE_FONT_SMOOTHING;
>
> if (settings->AllowDesktopComposition)
> settings->PerformanceFlags |= PERF_ENABLE_DESKTOP_COMPOSITION;
>
> if (settings->DisableWallpaper)
> settings->PerformanceFlags |= PERF_DISABLE_WALLPAPER;
>
> if (settings->DisableFullWindowDrag)
> settings->PerformanceFlags |= PERF_DISABLE_FULLWINDOWDRAG;
>
> if (settings->DisableMenuAnims)
> settings->PerformanceFlags |= PERF_DISABLE_MENUANIMATIONS;
>
> if (settings->DisableThemes)
> settings->PerformanceFlags |= PERF_DISABLE_THEMING;
>
> And that's putting aside the defaults for connection types:
>
> int freerdp_set_connection_type(rdpSettings* settings, int type)
> {
> if (type == CONNECTION_TYPE_MODEM)
> {
> settings->DisableWallpaper = TRUE;
> settings->AllowFontSmoothing = FALSE;
> settings->AllowDesktopComposition = FALSE;
> settings->DisableFullWindowDrag = TRUE;
> settings->DisableMenuAnims = TRUE;
> settings->DisableThemes = TRUE;
> }
> else if (type == CONNECTION_TYPE_BROADBAND_LOW)
> {
> settings->DisableWallpaper = TRUE;
> settings->AllowFontSmoothing = FALSE;
> settings->AllowDesktopComposition = FALSE;
> settings->DisableFullWindowDrag = TRUE;
> settings->DisableMenuAnims = TRUE;
> settings->DisableThemes = FALSE;
> }
> else if (type == CONNECTION_TYPE_SATELLITE)
> {
> settings->DisableWallpaper = TRUE;
> settings->AllowFontSmoothing = FALSE;
> settings->AllowDesktopComposition = TRUE;
> settings->DisableFullWindowDrag = TRUE;
> settings->DisableMenuAnims = TRUE;
> settings->DisableThemes = FALSE;
> }
> else if (type == CONNECTION_TYPE_BROADBAND_HIGH)
> {
> settings->DisableWallpaper = TRUE;
> settings->AllowFontSmoothing = FALSE;
> settings->AllowDesktopComposition = TRUE;
> settings->DisableFullWindowDrag = TRUE;
> settings->DisableMenuAnims = TRUE;
> settings->DisableThemes = FALSE;
> }
> else if (type == CONNECTION_TYPE_WAN)
> {
> settings->DisableWallpaper = FALSE;
> settings->AllowFontSmoothing = TRUE;
> settings->AllowDesktopComposition = TRUE;
> settings->DisableFullWindowDrag = FALSE;
> settings->DisableMenuAnims = FALSE;
> settings->DisableThemes = FALSE;
> }
> else if (type == CONNECTION_TYPE_LAN)
> {
> settings->DisableWallpaper = FALSE;
> settings->AllowFontSmoothing = TRUE;
> settings->AllowDesktopComposition = TRUE;
> settings->DisableFullWindowDrag = FALSE;
> settings->DisableMenuAnims = FALSE;
> settings->DisableThemes = FALSE;
> }
> else if (type == CONNECTION_TYPE_AUTODETECT)
> {
> settings->DisableWallpaper = FALSE;
> settings->AllowFontSmoothing = TRUE;
> settings->AllowDesktopComposition = TRUE;
> settings->DisableFullWindowDrag = FALSE;
> settings->DisableMenuAnims = FALSE;
> settings->DisableThemes = FALSE;
>
> settings->NetworkAutoDetect = TRUE;
> }
>
> return 0;
> }
>
> If xfreerdp, dfreerdp, wfreerdp and all the rest all had their own copies
> and rdpSettings was only for protocol variables we'd have a much bigger
> problem. Perhaps the real issue here is documenting how each variable in
> rdpSettings is used?
>
> Best regards,
> - Marc-Andre
>
> On Fri, Jan 11, 2013 at 4:04 PM, arvid norr <norrar...@gmail.com> wrote:
>>
>> In the rdpSettings structure many settings can be configured. Some of
>> the parameters are used by the freeRDP libaries but some are only used
>> internally by the client software.
>> This is very hard to understand and also complicates the API to freeRDP.
>> I suggest that all settings that are only used by the client and are
>> not used by the libraries are moved into a sub data structure.
>>
>> Examples are the PerformanceFlags variable that is used by freeRDP
>> libraries. DisableThemes and DisableWallpaper are only used by the
>> client. When using the API you don’t understand if you must both set
>> the PerformanceFlags and also set for example DisableThemes.
>>
>> Regards
>> Arvid
>>
>>
>> ------------------------------------------------------------------------------
>> Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and
>> much more. Get web development skills now with LearnDevNow -
>> 350+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
>> SALE $99.99 this month only -- learn more at:
>> http://p.sf.net/sfu/learnmore_122812
>> _______________________________________________
>> Freerdp-devel mailing list
>> Freerdp-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/freerdp-devel
>
>
------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_123012
_______________________________________________
Freerdp-devel mailing list
Freerdp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/freerdp-devel
