Test: patch to add adva,ced d3d settings in winecfg

[Warren Dumortier]

Hello everybody...

My patch to add advanced d3d settings in winecfg is almost ready.
In fact, i only have to figure out how to detect video memory size to
display it when no key is set to force it or to reset. Stefan D told
me to use DirectDraw, but i wouldn't find.

So that is the last thing (except some better naming...) to do for this patch.

Everything works well and i would like some feedback.

Thanks, kind regards.

[Juan Lang]

Hi Warren,
> Everything works well and i would like some feedback.

+    if (!strcmp(buf, "enabled"))
+        CheckDlgButton(dialog, IDC_D3D_MULTISAMPLING, BST_CHECKED);
+    else
+        CheckDlgButton(dialog, IDC_D3D_MULTISAMPLING, BST_UNCHECKED);
+        HeapFree(GetProcessHeap(), 0, buf);
The HeapFree should be outdented.

+    char *memory;
+    if (updating_ui) return;
+
Minor nit:  move the blank line between the variable declaration and
the updating_ui check.

+    int selected_mode = selected_mode = SendDlgItemMessage(hDlg,
IDC_D3D_RENDERING_MODE, CB_GETCURSEL, 0, 0);
You can't declare variables other than at the beginning of a block in ANSI C.

+    unsigned int it = SendMessage(IDC_D3D_RENDERING_MODE,
CB_GETTOPINDEX, 0, 0);
Same here.

+                if (updating_ui) break;
+                    SendMessage(GetParent(hDlg), PSM_CHANGED, 0, 0);
+        	    switch (LOWORD(wParam)) {
+                        case IDOK:
+                            d3d_advanced_settings_save(hDlg);
+                            EndDialog(hDlg, wParam);
+			    return TRUE;
You've got some screwy indentation going on here.  Please be
consistent.  Avoid tabs too, please.

+		        case IDC_D3D_GLSL: on_d3d_glsl_clicked(hDlg); break;
+                        case IDC_D3D_ADVANCED: on_d3d_advanced(hDlg); break;
Same here.

-		    case IDC_D3D_VSHADER_MODE: on_d3d_vshader_mode_changed(hDlg); break;
+			case IDC_D3D_VSHADER_MODE: on_d3d_vshader_mode_changed(hDlg); break;
Please avoid whitespace-only changes in your patch.

Regards,
--Juan

[Warren Dumortier]

2009/4/9 Juan Lang :
> Hi Warren,
> > Everything works well and i would like some feedback.
> - if (!strcmp(buf, "enabled"))
> - CheckDlgButton(dialog, IDC_D3D_MULTISAMPLING, BST_CHECKED);
> - else
> - CheckDlgButton(dialog, IDC_D3D_MULTISAMPLING, BST_UNCHECKED);
> - HeapFree(GetProcessHeap(), 0, buf);
>
> The HeapFree should be outdented.
> - char *memory;
> - if (updating_ui) return;
> -
>
> Minor nit:  move the blank line between the variable declaration and
> the updating_ui check.
> - int selected_mode = selected_mode = SendDlgItemMessage(hDlg,
>
> IDC_D3D_RENDERING_MODE, CB_GETCURSEL, 0, 0);
> You can't declare variables other than at the beginning of a block in ANSI C.
> - unsigned int it = SendMessage(IDC_D3D_RENDERING_MODE,
>
> CB_GETTOPINDEX, 0, 0);
> Same here.
> - if (updating_ui) break;
> - SendMessage(GetParent(hDlg), PSM_CHANGED, 0, 0);
> - switch (LOWORD(wParam)) {
> - case IDOK:
> - d3d_advanced_settings_save(hDlg);
> - EndDialog(hDlg, wParam);
> - return TRUE;
>
> You've got some screwy indentation going on here.  Please be
> consistent.  Avoid tabs too, please.
> - case IDC_D3D_GLSL: on_d3d_glsl_clicked(hDlg); break;
> - case IDC_D3D_ADVANCED: on_d3d_advanced(hDlg); break;
>
> Same here.
> - case IDC_D3D_VSHADER_MODE: on_d3d_vshader_mode_changed(hDlg); break;
> - case IDC_D3D_VSHADER_MODE: on_d3d_vshader_mode_changed(hDlg); break;
>
> Please avoid whitespace-only changes in your patch.
>
> Regards,
> --Juan

Sorry, forgot the -B option to generate the diff, put atm it's only
for testing! :)

[Austin English]

On Thu, Apr 9, 2009 at 4:28 AM, Warren Dumortier  wrote:
> Hello everybody...
>
> My patch to add advanced d3d settings in winecfg is almost ready.
> In fact, i only have to figure out how to detect video memory size to
> display it when no key is set to force it or to reset. Stefan D told
> me to use DirectDraw, but i wouldn't find.
>
> So that is the last thing (except some better naming...) to do for this patch.
>
> Everything works well and i would like some feedback.
>
> Thanks, kind regards.

Gives some compiler warnings here:
-limits -Wpointer-arith  -g -O2  -o x11drvdlg.o x11drvdlg.c
x11drvdlg.c: In function ‘d3d_advanced_settings_save’:
x11drvdlg.c:451: warning: ISO C90 forbids mixed declarations and code
x11drvdlg.c: In function ‘d3d_advanced_settings_reset’:
x11drvdlg.c:463: warning: passing argument 1 of ‘SetDlgItemTextA’
makes pointer from integer without a cast
x11drvdlg.c:465: warning: passing argument 1 of ‘SendMessageA’ makes
pointer from integer without a cast
x11drvdlg.c:465: warning: ISO C90 forbids mixed declarations and code
x11drvdlg.c:466: warning: passing argument 1 of ‘SendDlgItemMessageA’
makes pointer from integer without a cast
x11drvdlg.c:468: warning: passing argument 1 of ‘SendMessageA’ makes
pointer from integer without a cast
x11drvdlg.c:469: warning: passing argument 1 of ‘SendDlgItemMessageA’
makes pointer from integer without a cast
x11drvdlg.c:471: warning: passing argument 1 of ‘SendMessageA’ makes
pointer from integer without a cast
x11drvdlg.c:472: warning: passing argument 1 of ‘SendDlgItemMessageA’
makes pointer from integer without a cast

Other than that, and the comments Juan and others pointed out, looks good.

[Austin English]

On Thu, Apr 9, 2009 at 5:32 PM, Austin English  wrote:
> On Thu, Apr 9, 2009 at 4:28 AM, Warren Dumortier  wrote:
> > Hello everybody...
> >
> > My patch to add advanced d3d settings in winecfg is almost ready.
> > In fact, i only have to figure out how to detect video memory size to
> > display it when no key is set to force it or to reset. Stefan D told
> > me to use DirectDraw, but i wouldn't find.
> >
> > So that is the last thing (except some better naming...) to do for this patch.
> >
> > Everything works well and i would like some feedback.
> >
> > Thanks, kind regards.
>
> Gives some compiler warnings here:
> -limits -Wpointer-arith  -g -O2  -o x11drvdlg.o x11drvdlg.c
> x11drvdlg.c: In function ‘d3d_advanced_settings_save’:
> x11drvdlg.c:451: warning: ISO C90 forbids mixed declarations and code
> x11drvdlg.c: In function ‘d3d_advanced_settings_reset’:
> x11drvdlg.c:463: warning: passing argument 1 of ‘SetDlgItemTextA’
> makes pointer from integer without a cast
> x11drvdlg.c:465: warning: passing argument 1 of ‘SendMessageA’ makes
> pointer from integer without a cast
> x11drvdlg.c:465: warning: ISO C90 forbids mixed declarations and code
> x11drvdlg.c:466: warning: passing argument 1 of ‘SendDlgItemMessageA’
> makes pointer from integer without a cast
> x11drvdlg.c:468: warning: passing argument 1 of ‘SendMessageA’ makes
> pointer from integer without a cast
> x11drvdlg.c:469: warning: passing argument 1 of ‘SendDlgItemMessageA’
> makes pointer from integer without a cast
> x11drvdlg.c:471: warning: passing argument 1 of ‘SendMessageA’ makes
> pointer from integer without a cast
> x11drvdlg.c:472: warning: passing argument 1 of ‘SendDlgItemMessageA’
> makes pointer from integer without a cast
>
> Other than that, and the comments Juan and others pointed out, looks good.
>
> --
> -Austin

Compiling under 64-bit is bit more verbose:
/usr/lib/gcc-snapshot/bin/gcc -c -I. -I. -I../../include
-I../../include   -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing
-Wdeclaration-after-statement -Wwrite-strings -Wtype-limits
-Wpointer-arith  -g -O2  -o x11drvdlg.o x11drvdlg.c
x11drvdlg.c: In function 'd3d_advanced_settings_save':
x11drvdlg.c:451: warning: ISO C90 forbids mixed declarations and code
x11drvdlg.c: In function 'd3d_advanced_settings_reset':
x11drvdlg.c:463: warning: passing argument 1 of 'SetDlgItemTextA'
makes pointer from integer without a cast
../../include/winuser.h:4943: note: expected 'HWND' but argument is of
type 'int'
x11drvdlg.c:465: warning: passing argument 1 of 'SendMessageA' makes
pointer from integer without a cast
../../include/winuser.h:4908: note: expected 'HWND' but argument is of
type 'int'
x11drvdlg.c:465: warning: ISO C90 forbids mixed declarations and code
x11drvdlg.c:466: warning: passing argument 1 of 'SendDlgItemMessageA'
makes pointer from integer without a cast
../../include/winuser.h:4904: note: expected 'HWND' but argument is of
type 'int'
x11drvdlg.c:468: warning: passing argument 1 of 'SendMessageA' makes
pointer from integer without a cast
../../include/winuser.h:4908: note: expected 'HWND' but argument is of
type 'int'
x11drvdlg.c:469: warning: passing argument 1 of 'SendDlgItemMessageA'
makes pointer from integer without a cast
../../include/winuser.h:4904: note: expected 'HWND' but argument is of
type 'int'
x11drvdlg.c:471: warning: passing argument 1 of 'SendMessageA' makes
pointer from integer without a cast
../../include/winuser.h:4908: note: expected 'HWND' but argument is of
type 'int'
x11drvdlg.c:472: warning: passing argument 1 of 'SendDlgItemMessageA'
makes pointer from integer without a cast
../../include/winuser.h:4904: note: expected 'HWND' but argument is of
type 'int'
LD_LIBRARY_PATH="../../libs/wine:$LD_LIBRARY_PATH" ../../tools/wrc/wrc
--nostdinc -I. -I. -I../../include -I../../include    -fowinecfg.res
winecfg.rc
../../tools/winegcc/winegcc -m64 -B../../tools/winebuild -mwindows
appdefaults.o audio.o drive.o drivedetect.o driveui.o libraries.o
main.o theme.o winecfg.o x11drvdlg.o     winecfg.res -o winecfg.exe.so
 -luuid -lcomdlg32 -lcomctl32 -lshell32 -lole32 -lwinmm -lshlwapi
-luxtheme -luser32 -lgdi32 -ladvapi32 -lkernel32
../../libs/port/libwine_port.a