Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
On Wed, May 16, 2007 at 06:41:33PM +0200, Frank Richter wrote: just add all preset memory sizes, and WM_SETTEXT the value read from the registry. Might be code-wise a bit simpler than your GETCURSEL approach, but otherswise not much different I think. -f.r. Hi Frank, I've simplified the code according Your review, see the attached patch. Thanks for help. Vit diff --git a/programs/winecfg/Bg.rc b/programs/winecfg/Bg.rc index 302bec5..a2580a3 100644 --- a/programs/winecfg/Bg.rc +++ b/programs/winecfg/Bg.rc @@ -90,6 +90,8 @@ BEGIN COMBOBOX IDC_D3D_VSHADER_MODE,115,218,125,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Đŕçđĺřč Pixel Shader (ŕęî ńĺ ďîääúđćŕ îň őŕđäóĺđŕ),IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,237,230,10 +LTEXT Video Memory Size (in megabytes):,IDC_STATIC,15,232,120,12 +COMBOBOXIDC_VIDEOMEMORY_SIZE_COMBO,130,232,40,112,CBS_DROPDOWN | WS_VSCROLL | WS_TABSTOP | CBS_LOWERCASE END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 diff --git a/programs/winecfg/Cs.rc b/programs/winecfg/Cs.rc index 5bba698..07ba100 100644 --- a/programs/winecfg/Cs.rc +++ b/programs/winecfg/Cs.rc @@ -88,6 +88,8 @@ BEGIN COMBOBOX IDC_D3D_VSHADER_MODE,105,197,140,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Povolit stínování pixelů (spoléhá se na hardware podporu),IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,216,230,10 +LTEXT Velikost videopaměti (v megabytech):,IDC_STATIC,15,232,140,12 +COMBOBOXIDC_VIDEOMEMORY_SIZE_COMBO,145,232,40,112,CBS_DROPDOWN | WS_VSCROLL | WS_TABSTOP | CBS_LOWERCASE END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 @@ -244,8 +246,8 @@ END STRINGTABLE DISCARDABLE BEGIN -IDS_SHADER_MODE_HARDWAREHardwarový -IDS_SHADER_MODE_NONEádný +IDS_SHADER_MODE_HARDWAREPodporováno zařízením +IDS_SHADER_MODE_NONEádná END STRINGTABLE DISCARDABLE diff --git a/programs/winecfg/De.rc b/programs/winecfg/De.rc index 8c77b2b..9741a48 100644 --- a/programs/winecfg/De.rc +++ b/programs/winecfg/De.rc @@ -84,6 +84,8 @@ BEGIN COMBOBOXIDC_D3D_VSHADER_MODE,140,197,105,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Pixel Shader aktivieren (wenn von Hardware unterstützt), IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,212,230,10 +LTEXT Größe des Grafikspeichers(in Megabytes),IDC_STATIC,15,232,140,12 +COMBOBOXIDC_VIDEOMEMORY_SIZE_COMBO,150,232,40,112,CBS_DROPDOWN | WS_VSCROLL | WS_TABSTOP | CBS_LOWERCASE END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 diff --git a/programs/winecfg/En.rc b/programs/winecfg/En.rc index 02d4850..218150a 100644 --- a/programs/winecfg/En.rc +++ b/programs/winecfg/En.rc @@ -79,12 +79,14 @@ BEGIN EDITTEXTIDC_DESKTOP_WIDTH,64,167,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED EDITTEXTIDC_DESKTOP_HEIGHT,117,167,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED -GROUPBOX Direct3D ,IDC_STATIC,8,189,244,50 +GROUPBOX Direct3D ,IDC_STATIC,8,189,244,60 LTEXT Vertex Shader Support: ,IDC_STATIC,15,199,80,30 COMBOBOX IDC_D3D_VSHADER_MODE,100,197,145,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Allow Pixel Shader (if supported by hardware),IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,216,230,10 +LTEXT Video Memory Size (in megabytes):,IDC_STATIC,15,232,120,12 +COMBOBOXIDC_VIDEOMEMORY_SIZE_COMBO,130,232,40,112,CBS_DROPDOWN | WS_VSCROLL | WS_TABSTOP | CBS_LOWERCASE END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 diff --git a/programs/winecfg/Es.rc b/programs/winecfg/Es.rc index 2430b02..d7dea8b 100644 --- a/programs/winecfg/Es.rc +++ b/programs/winecfg/Es.rc @@ -83,6 +83,8 @@ BEGIN COMBOBOXIDC_D3D_VSHADER_MODE,100,197,145,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Permitir Pixel Shader (si hay soporte por hardware),IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,216,230,10 +LTEXT Video Memory Size (in megabytes):,IDC_STATIC,15,232,120,12 +COMBOBOXIDC_VIDEOMEMORY_SIZE_COMBO,130,232,40,112,CBS_DROPDOWN | WS_VSCROLL | WS_TABSTOP | CBS_LOWERCASE END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 diff --git a/programs/winecfg/Fi.rc b/programs/winecfg/Fi.rc index 16da0ca..f8f8180 100644 --- a/programs/winecfg/Fi.rc +++ b/programs/winecfg/Fi.rc @@ -84,6 +84,8 @@ BEGIN COMBOBOX IDC_D3D_VSHADER_MODE,100,218,150,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Salli Pixel Shader:n käyttö laitteiston tukiessa,IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,237,230,10 +LTEXT Video Memory Size (in megabytes):,IDC_STATIC,15,232,120,12 +COMBOBOXIDC_VIDEOMEMORY_SIZE_COMBO,130,232,40,112,CBS_DROPDOWN | WS_VSCROLL
Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
case CBN_SELCHANGE: {
SendMessage(GetParent(hDlg), PSM_CHANGED, 0, 0);
switch (LOWORD(wParam)) {
- case IDC_D3D_VSHADER_MODE:
on_d3d_vshader_mode_changed(hDlg); break;
- }
+case IDC_D3D_VSHADER_MODE: on_d3d_vshader_mode_changed(hDlg);
break;
+case IDC_VIDEOMEMORY_SIZE_COMBO:
set_from_videomemory_size_changed(hDlg); break;
+}
I believe you should align your indentation method on the one the file already
uses (ie tabs, not spaces).
+// vim:sw=4:expandtab
This has nothing to do with wine
Laurent
On Wed, 16 May 2007 15:03:52 +0200, Vit Hrachovy [EMAIL PROTECTED] wrote:
Hi,
the attached patch adds new editable combobox input 'Video Memory size'
for Graphics/Direct3D tab of winecfg.
I've tried to implement Frank's comments to English language only.
Please consider this patch as a proposal, so if it gets positive review,
I'll propagate the changes to all remaining language resources.
May I kindly ask someone for the patch review?
Regards
Vit
Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
On Wed, May 16, 2007 at 03:18:22PM +0200, Laurent Vromman wrote:
case CBN_SELCHANGE: {
SendMessage(GetParent(hDlg), PSM_CHANGED, 0, 0);
switch (LOWORD(wParam)) {
- case IDC_D3D_VSHADER_MODE:
on_d3d_vshader_mode_changed(hDlg); break;
- }
+case IDC_D3D_VSHADER_MODE:
on_d3d_vshader_mode_changed(hDlg); break;
+case IDC_VIDEOMEMORY_SIZE_COMBO:
set_from_videomemory_size_changed(hDlg); break;
+}
I believe you should align your indentation method on the one the file
already uses (ie tabs, not spaces).
+// vim:sw=4:expandtab
This has nothing to do with wine
Laurent
Hi Laurent
citing from: http://www.winehq.org/site/docs/winedev-guide/style-notes
Tabs are not forbidden but discouraged. A tab is defined as 8
characters and the usual amount of indentation is 4 characters.
and from: http://www.winehq.org/site/sending_patches
Don't mix tabs and spaces because it makes the diff output
unreadable, use consistent indentation.
The actual file contents breaks both citations above, my code doesn't
conflict with any one of them.
For vim footer, I agree it shall not be present. If the only comment
regards formatting, I'm happy and I can fix the formatting as You wish.
Cheers
Vit
Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
It is not really as I wish. I have just noticed that there is two different
kind of indentation in the mentioned switch loop, which looked strange on the
moment.
I am really not a wine master, others will maybe fixed us.
Laurent
On Wed, 16 May 2007 15:59:06 +0200, Vit Hrachovy [EMAIL PROTECTED] wrote:
On Wed, May 16, 2007 at 03:18:22PM +0200, Laurent Vromman wrote:
case CBN_SELCHANGE: {
SendMessage(GetParent(hDlg), PSM_CHANGED, 0, 0);
switch (LOWORD(wParam)) {
-case IDC_D3D_VSHADER_MODE:
on_d3d_vshader_mode_changed(hDlg);
break;
-}
+case IDC_D3D_VSHADER_MODE:
on_d3d_vshader_mode_changed(hDlg); break;
+case IDC_VIDEOMEMORY_SIZE_COMBO:
set_from_videomemory_size_changed(hDlg); break;
+}
I believe you should align your indentation method on the one the file
already uses (ie tabs, not spaces).
+// vim:sw=4:expandtab
This has nothing to do with wine
Laurent
Hi Laurent
citing from: http://www.winehq.org/site/docs/winedev-guide/style-notes
Tabs are not forbidden but discouraged. A tab is defined as 8
characters and the usual amount of indentation is 4 characters.
and from: http://www.winehq.org/site/sending_patches
Don't mix tabs and spaces because it makes the diff output
unreadable, use consistent indentation.
The actual file contents breaks both citations above, my code doesn't
conflict with any one of them.
For vim footer, I agree it shall not be present. If the only comment
regards formatting, I'm happy and I can fix the formatting as You wish.
Cheers
Vit
Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
I would make the combobox editable (plain CBS_DROPDOWN), just add all preset memory sizes, and WM_SETTEXT the value read from the registry. -f.r.
Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
On 16.05.2007 18:18, Frank Richter wrote: I would make the combobox editable (plain CBS_DROPDOWN), Er, you do that already. Pays off to read... just add all preset memory sizes, and WM_SETTEXT the value read from the registry. Might be code-wise a bit simpler than your GETCURSEL approach, but otherswise not much different I think. -f.r.
Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
Am Freitag 27 April 2007 09:23 schrieb Vit Hrachovy: Hi, the attached patch adds new textbox input 'Video Memory size' for Graphics/Direct3D tab of winecfg. Updated every live locale resource to include this. Adding this option was discussed on thread 'More Direct3D settings in winecfg (was: Enabling GLSL in winecfg)' at wine-devel. changelog entry: Add Video Memory textbox to Graphics/direct3D part of winecfg May I kindly ask someone for the patch review? On a quick look I see a number of whitespace issues in the patch. in the .rc files you are using tabs while the rest of the code uses spaces at some places, in other lines you're using spaces yourself. In the code you add some trailing whitespace(Git warns about that in rebase and git-am As for the German translation you could use Größe des Grafikspeichers(in Megabytes) pgpzJzM2W8lCi.pgp Description: PGP signature
Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
On Fri, Apr 27, 2007 at 12:14:47PM +0200, Stefan Dösinger wrote: On a quick look I see a number of whitespace issues in the patch. in the .rc files you are using tabs while the rest of the code uses spaces at some places, in other lines you're using spaces yourself. In the code you add some trailing whitespace(Git warns about that in rebase and git-am As for the German translation you could use Größe des Grafikspeichers(in Megabytes) Hi Stefan, thanks for comments, I'm including the fixed patch with added German translation. Thanks Vit diff --git a/programs/winecfg/Bg.rc b/programs/winecfg/Bg.rc index 302bec5..a3d9860 100644 --- a/programs/winecfg/Bg.rc +++ b/programs/winecfg/Bg.rc @@ -84,12 +84,14 @@ BEGIN EDITTEXTIDC_DESKTOP_WIDTH,64,188,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED EDITTEXTIDC_DESKTOP_HEIGHT,117,188,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED -GROUPBOX Direct3D ,IDC_STATIC,8,210,244,50 +GROUPBOX Direct3D ,IDC_STATIC,8,210,244,60 LTEXT Ďîääđúćęŕ íŕ Vertex Shader: ,IDC_STATIC,15,220,105,30 COMBOBOX IDC_D3D_VSHADER_MODE,115,218,125,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Đŕçđĺřč Pixel Shader (ŕęî ńĺ ďîääúđćŕ îň őŕđäóĺđŕ),IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,237,230,10 +LTEXT Video Memory Size (in megabytes):,IDC_STATIC,15,232,120,12 +EDITTEXTIDC_VIDEOMEMORY_SIZE,130,232,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 diff --git a/programs/winecfg/Cs.rc b/programs/winecfg/Cs.rc index 5bba698..6a91d54 100644 --- a/programs/winecfg/Cs.rc +++ b/programs/winecfg/Cs.rc @@ -88,6 +88,8 @@ BEGIN COMBOBOX IDC_D3D_VSHADER_MODE,105,197,140,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Povolit stínování pixelů (spoléhá se na hardware podporu),IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,216,230,10 +LTEXT Velikost videopaměti (v megabytech):,IDC_STATIC,15,232,120,12 +EDITTEXTIDC_VIDEOMEMORY_SIZE,135,232,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 diff --git a/programs/winecfg/De.rc b/programs/winecfg/De.rc index 8c77b2b..cc6d43b 100644 --- a/programs/winecfg/De.rc +++ b/programs/winecfg/De.rc @@ -78,12 +78,14 @@ BEGIN LTEXT X,IDC_DESKTOP_BY,114,167,8,8,WS_DISABLED EDITTEXTIDC_DESKTOP_HEIGHT,123,167,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED -GROUPBOX Direct3D ,IDC_STATIC,8,189,244,38 +GROUPBOX Direct3D ,IDC_STATIC,8,189,244,60 LTEXT Unterstützung für Vertex Shader: ,IDC_STATIC,15,199,120,8 COMBOBOXIDC_D3D_VSHADER_MODE,140,197,105,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Pixel Shader aktivieren (wenn von Hardware unterstützt), IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,212,230,10 +LTEXT Größe des Grafikspeichers (in Megabytes):,IDC_STATIC,15,232,135,12 +EDITTEXTIDC_VIDEOMEMORY_SIZE,150,232,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 diff --git a/programs/winecfg/En.rc b/programs/winecfg/En.rc index 02d4850..84e77d9 100644 --- a/programs/winecfg/En.rc +++ b/programs/winecfg/En.rc @@ -79,12 +79,14 @@ BEGIN EDITTEXTIDC_DESKTOP_WIDTH,64,167,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED EDITTEXTIDC_DESKTOP_HEIGHT,117,167,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED -GROUPBOX Direct3D ,IDC_STATIC,8,189,244,50 +GROUPBOX Direct3D ,IDC_STATIC,8,189,244,60 LTEXT Vertex Shader Support: ,IDC_STATIC,15,199,80,30 COMBOBOX IDC_D3D_VSHADER_MODE,100,197,145,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Allow Pixel Shader (if supported by hardware),IDC_D3D_PSHADER_MODE,Button,BS_AUTOCHECKBOX | WS_TABSTOP,15,216,230,10 +LTEXT Video Memory Size (in megabytes):,IDC_STATIC,15,232,120,12 +EDITTEXTIDC_VIDEOMEMORY_SIZE,130,232,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED END IDD_DLLCFG DIALOG DISCARDABLE 0, 0, 260, 250 diff --git a/programs/winecfg/Es.rc b/programs/winecfg/Es.rc index 34993c2..b4a 100644 --- a/programs/winecfg/Es.rc +++ b/programs/winecfg/Es.rc @@ -78,12 +78,14 @@ BEGIN EDITTEXTIDC_DESKTOP_WIDTH,64,188,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED EDITTEXTIDC_DESKTOP_HEIGHT,117,188,40,12,ES_AUTOHSCROLL | ES_NUMBER | WS_DISABLED -GROUPBOX Direct3D ,IDC_STATIC,8,210,244,50 +GROUPBOX Direct3D ,IDC_STATIC,8,210,244,60 LTEXT Soporte Vertex Shader: ,IDC_STATIC,15,220,80,30 COMBOBOXIDC_D3D_VSHADER_MODE,100,218,150,70,CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL Permitir
Re: review: add Video Memory text input to winecfg Graphics/Direct3D tab
On 27.04.2007 09:23, Vit Hrachovy wrote: Hi, the attached patch adds new textbox input 'Video Memory size' for Graphics/Direct3D tab of winecfg. Updated every live locale resource to include this. Some thoughts on the UI: - Maybe make it an editable combobox with common sizes in the dropdown (64, 128, 256 ...) for convenience. - It looks like the field is left empty when no video memory setting is found in the registry. Maybe it's better to show the default value from wined3d then? -f.r.
