Re: gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font associated charset

2013-02-26 Thread Akihiro Sagawa 
On Tue, 26 Feb 2013 10:42:14 +0900, Byeongsik Jeon wrote:
  This condition should be rewritten. buf is not initialized when
  RegQueryValueExA is failed in line 3542. And strcmp(buf, cpbuf) has
  already done in line 3544.
  
 I wanted to update in registry when codepages changed, not logpixels. When
 logpixels has been changed, MS-Windows does not update the font associated
 charset registry. memset(buf, 0, sizeof(buf)) has been added.

Good work. I got it.

Including other changes, it looks good for me.
Please send your patch to wine-patches.

Akihiro Sagawa

Re: gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font associated charset

2013-02-25 Thread Akihiro Sagawa 
On Sun, 24 Feb 2013 09:55:59 +0900, Byeongsik Jeon wrote:

Could you consider adding (try N) at the end of subject line?
The rule is documented here:
http://wiki.winehq.org/SubmittingPatches#head-fea51bfdf6a29105a9a6fc6f5886ad0653ae95e5

 diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c
 index b274b83..3a99d35 100644
 --- a/dlls/gdi32/font.c
 +++ b/dlls/gdi32/font.c
 @@ -590,11 +590,75 @@ HFONT WINAPI CreateFontW( INT height, INT width, INT 
 esc,

This part looks good.

 diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
(snip)
 +static void update_font_association_info(UINT current_ansi_codepage)
 +{
 +static const WCHAR font_assoc_reg_keyW[] = {'S','y','s','t','e','m','\\',
 +
 'C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t','\\',
 +
 'C','o','n','t','r','o','l','\\','F','o','n','t','A','s','s','o','c','\0'};

It is nice introducing separate function for this work. However using
WCHAR is a bit overwork, because ANSI APIs are still used in
update_font_info() especially set_value_key(). Please follow the style
of surrounding code.

 @@ -3615,6 +3675,10 @@ static void update_font_info(void)
  }
  if (!done)
  FIXME(there is no font defaults for codepages %u,%u\n, ansi_cp, 
 oem_cp);
 +
 +/* update locale dependent font association info in registry */
 +if (strcmp(buf, cpbuf))
 +update_font_association_info(ansi_cp);
  }
  
  static BOOL init_freetype(void)

This condition should be rewritten. buf is not initialized when
RegQueryValueExA is failed in line 3542. And strcmp(buf, cpbuf) has
already done in line 3544.

 diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c
 index 943f584..9d57806 100644
 --- a/dlls/gdi32/tests/font.c
 +++ b/dlls/gdi32/tests/font.c

Why are there no tests against OEM_CHARSET and SYMBOL_CHARSET?

Regards,
Akihiro Sagawa

Re: gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font associated charset

2013-02-25 Thread Byeongsik Jeon 
Thank you.
Review again before submitting a patch to wine-patches, please.

On 02/26/13 00:08, Akihiro Sagawa wrote:
 On Sun, 24 Feb 2013 09:55:59 +0900, Byeongsik Jeon wrote:
 
 @@ -3615,6 +3675,10 @@ static void update_font_info(void)
  }
  if (!done)
  FIXME(there is no font defaults for codepages %u,%u\n, ansi_cp, 
 oem_cp);
 +
 +/* update locale dependent font association info in registry */
 +if (strcmp(buf, cpbuf))
 +update_font_association_info(ansi_cp);
  }
  
  static BOOL init_freetype(void)
 
 This condition should be rewritten. buf is not initialized when
 RegQueryValueExA is failed in line 3542. And strcmp(buf, cpbuf) has
 already done in line 3544.
 
I wanted to update in registry when codepages changed, not logpixels. When
logpixels has been changed, MS-Windows does not update the font associated
charset registry. memset(buf, 0, sizeof(buf)) has been added.

 diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c
 index 943f584..9d57806 100644
 --- a/dlls/gdi32/tests/font.c
 +++ b/dlls/gdi32/tests/font.c
 
 Why are there no tests against OEM_CHARSET and SYMBOL_CHARSET?
 
In the current Wine code, if lfCharset == OEM_CHARSET then GetTextCharset()
return a wrong value. This problem can be found in the test_oemcharset(). At
this point, I wanted to concentrate on the ANSI_CHARSET association problem.
From 9be98fc85ee685f93ba61e208ceb7d9cd6662854 Mon Sep 17 00:00:00 2001
From: Byeongsik Jeon bsj...@hanmail.net
Date: Tue, 26 Feb 2013 10:31:20 +0900
Subject: [PATCH] gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font
 associated charset

---
 dlls/gdi32/font.c   | 64 +
 dlls/gdi32/freetype.c   | 50 +
 dlls/gdi32/tests/font.c | 85 +
 3 files changed, 199 insertions(+)

diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c
index b274b83..3a99d35 100644
--- a/dlls/gdi32/font.c
+++ b/dlls/gdi32/font.c
@@ -590,11 +590,75 @@ HFONT WINAPI CreateFontW( INT height, INT width, INT esc,
 return CreateFontIndirectW( logfont );
 }
 
+#define ASSOC_CHARSET_OEM(1)
+#define ASSOC_CHARSET_ANSI   (2)
+#define ASSOC_CHARSET_SYMBOL (4)
+static DWORD get_associated_charset_info(void)
+{
+static DWORD associated_charset = -1;
+
+if (associated_charset == -1)
+{
+static const WCHAR assoc_charset_reg_keyW[] = 
{'S','y','s','t','e','m','\\',
+
'C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t','\\',
+
'C','o','n','t','r','o','l','\\','F','o','n','t','A','s','s','o','c','\\',
+'A','s','s','o','c','i','a','t','e','d',' 
','C','h','a','r','s','e','t','\0'};
+static const WCHAR ansiW[] = {'A','N','S','I','(','0','0',')','\0'};
+static const WCHAR oemW[] = {'O','E','M','(','F','F',')','\0'};
+static const WCHAR symbolW[] = 
{'S','Y','M','B','O','L','(','0','2',')','\0'};
+static const WCHAR yesW[] = {'Y','E','S','\0'};
+HKEY hkey;
+WCHAR *dataW;
+DWORD type, data_len, max_data_len;
+
+associated_charset = 0;
+
+if (RegOpenKeyW(HKEY_LOCAL_MACHINE,
+assoc_charset_reg_keyW, hkey) != ERROR_SUCCESS)
+return 0;
+
+if (RegQueryInfoKeyW(hkey, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
NULL,
+ max_data_len, NULL, NULL) != ERROR_SUCCESS)
+return 0;
+
+dataW = HeapAlloc(GetProcessHeap(), 0, max_data_len);
+
+memset(dataW, 0, max_data_len);
+data_len = max_data_len;
+RegQueryValueExW(hkey, ansiW, NULL, type, (LPBYTE)dataW, data_len);
+if (type == REG_SZ  !strcmpiW(dataW, yesW))
+associated_charset |= ASSOC_CHARSET_ANSI;
+
+memset(dataW, 0, max_data_len);
+data_len = max_data_len;
+RegQueryValueExW(hkey, oemW, NULL, type, (LPBYTE)dataW, data_len);
+if (type == REG_SZ  !strcmpiW(dataW, yesW))
+associated_charset |= ASSOC_CHARSET_OEM;
+
+memset(dataW, 0, max_data_len);
+data_len = max_data_len;
+RegQueryValueExW(hkey, symbolW, NULL, type, (LPBYTE)dataW, data_len);
+if (type == REG_SZ  !strcmpiW(dataW, yesW))
+associated_charset |= ASSOC_CHARSET_SYMBOL;
+
+HeapFree(GetProcessHeap(), 0, dataW);
+RegCloseKey(hkey);
+
+TRACE(associated_charset = %d\n, associated_charset);
+}
+
+return associated_charset;
+}
+
 static void update_font_code_page( DC *dc )
 {
 CHARSETINFO csi;
 int charset = GetTextCharsetInfo( dc-hSelf, NULL, 0 );
 
+if (charset == ANSI_CHARSET 
+get_associated_charset_info()  ASSOC_CHARSET_ANSI)
+charset = DEFAULT_CHARSET;
+
 /* Hmm, nicely designed api this one! */
 if (TranslateCharsetInfo( ULongToPtr(charset), csi, TCI_SRCCHARSET) )
 dc-font_code_page = csi.ciACP;
diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c

Re: gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font associated charset.

2013-02-22 Thread Akihiro Sagawa 
On Fri, 22 Feb 2013 11:21:20 +0900, Byeongsik Jeon wrote:
 This patch fix a original topic of the wine-bugs #16325.

Thanks for your work on this.

 +static DWORD get_associated_charset_info(void)
 +{
 +static DWORD associated_charset = -1;
 +
 +if (associated_charset == -1)
 +{
 +const char *assoc_charset_reg_key =
 +System\\CurrentControlSet\\Control\\FontAssoc\\Associated 
 Charset;

Please use WCHARs and W version of APIs in gdi32/font.c, like:
const WCHAR yesW[] = { 'Y', 'E', 'S', 0 }; and so on.

 @@ -3615,6 +3615,33 @@ static void update_font_info(void)
  }
  if (!done)
  FIXME(there is no font defaults for codepages %u,%u\n, ansi_cp, 
 oem_cp);
 +
 +if (RegCreateKeyA( HKEY_LOCAL_MACHINE,
 +   
 System\\CurrentControlSet\\Control\\FontAssoc\\Associated Charset,

As far as I know, in SBCS locale, this key doesn't exist. Therefore it
should delete in SBCS locale initialization and create the key in DBCS
locale initialization.

Regards,
Akihiro Sagawa

Re: gdi32: Fix the GdiGetCodePage() support ANSI_CHARSET font associated charset.

2013-02-21 Thread Qian Hong 
On Fri, Feb 22, 2013 at 10:21 AM, Byeongsik Jeon bsj...@hanmail.net wrote:
 This patch fix a original topic of the wine-bugs #16325.

Thanks for the great work! I've been waiting for this fix for a really
long time :)
Out basic Wine test VM does not include Chinese/Japanese Windows, so I
submit Byeongsik's patch to the testbot again with Chinese/Japanese
Windows included: https://testbot.winehq.org/JobDetails.pl?Key=24496
All tests related to this patch passed (Those failed tests are not
related to this patch)

I also manually test the patch on Windows with Korean locale and the
test case passed as well. Hopes that helps :)


--
Regards,
Qian Hong

-
http://www.winehq.org