[PATCH 2/2] usp10: Fixed wgBlank, wgDefault, wgInvalid and wgKashida.
Qian Hong
fracting at gmail.com
Tue Nov 13 12:50:00 CST 2012
Hello,
Thanks for comment too!
On Wed, Nov 14, 2012 at 3:25 AM, Nikolay Sivov <bunglehead at gmail.com> wrote:
> I think it's better to avoid macros if possible.
>>
Got it.
>> + if (!sc->sfnt)
>> + {
>> + if (GetTextMetricsW(hdc, &tmW))
>> + sc->sfp.wgBlank = tmW.tmBreakChar;
>> + else
>> + {
>> + ERR("Get wgBlank failed, fallback to Numeric_space.\n");
>> + sc->sfp.wgBlank = Numeric_space;
>> + }
>
> If it's a normal fallback case it's not an error I guess.
>
It's not a normal fallback, GetTextMetricsW() should not fail, and I
didn't see fail here for all fonts I tested.
Should I remove the else{} block?
>
>> +{
>> +#define Zero_width_space 0x200b /* Zero Width Space */
>> +#define Invalid_char3 0xf71b /* Unknow, found by black box testing */
>> +#define NON_EXIST_INDEX 0xffff /* Default non exist char index */
>> + WCHAR invalid_chars[3] = {Numeric_space, Zero_width_space,
>> Invalid_char3};
>
> I don't see why this can't be hardcoded without macros and one line comment
> about meaning of this.
Thanks, will change that.
>>
>> +#define Kashida_char 0x0640 /* Kashida */
>> + WCHAR kashida_chars[] = {Kashida_char};
>
> Same here, also naming is a bit strange. I think it's enough to name it
> 'kashida' and make it 'static const WCHAR'.
Good point, thanks Nikolay.
More information about the wine-devel
mailing list