[PATCH 2/2] [v4] user32: Implement EC_USEFONTINFO margins in the CJK case.

[Akihiro Sagawa]

Signed-off-by: Akihiro Sagawa 
---
 dlls/user32/edit.c       | 91 ++++++++++++++++++++++++++++++++++++++++++++++--
 dlls/user32/tests/edit.c |  4 +--
 2 files changed, 90 insertions(+), 5 deletions(-)

[Nikolay Sivov]

On 3/27/19 4:25 PM, Akihiro Sagawa wrote:
> -
>            /* FIXME: figure out the CJK values. */
> -
>            default_left_margin = width / 2;
> -
>            default_right_margin = width / 2;
> -
>            if (is_cjk_charset(dc)) {
> -
>                /* In the CJK case, use the minimum side bearings in hhea table */
> -
>                UINT ret;
> -
>                DWORD ppem, em_scale = 0;
> -
>                OUTLINETEXTMETRICW *otm;
> -
>                short min_lsb, min_rsb;
> -
>                ret = GetOutlineTextMetricsW(dc, 0, NULL);
> -
>                if (!ret) goto fail;
> -
>                otm = HeapAlloc(GetProcessHeap(), 0, ret);
> -
>                if (!otm) goto fail;
> -
>                otm->otmSize = sizeof(*otm);
> -
>                if (!GetOutlineTextMetricsW(dc, ret, otm)) goto fail;
> -
>                ppem = tm.tmHeight - tm.tmInternalLeading;
> -
>                em_scale = MulDiv(ppem, 1 << 16, otm->otmEMSquare);
> -
>                HeapFree(GetProcessHeap(), 0, otm);
> -
>                if (!get_side_bearings(dc, &min_lsb, &min_rsb)) goto fail;
> -
> -
>                default_left_margin = get_cjk_fontinfo_margin(width, min_lsb, em_scale);
> -
>                default_right_margin = get_cjk_fontinfo_margin(width, min_rsb, em_scale);
> -
>            } else {
> -
>            fail:
> -
>                default_left_margin = width / 2;
> -
>                default_right_margin = width / 2;
> -
>            }

You don't need to allocate 'otm', it's only necessary when you need 
string components.

Also please avoid goto jump across branches, you can simply make 
get_side_bearings() a part of condition, so default case will be hit 
naturally.

[Huw Davies]

On Wed, Mar 27, 2019 at 10:25:48PM +0900, Akihiro Sagawa wrote:
> Signed-off-by: Akihiro Sagawa
>
> dlls/user32/edit.c       | 91 ++++++++++++++++++++++++++++++++++++++++++++++--
>  dlls/user32/tests/edit.c |  4 +--
>  2 files changed, 90 insertions(+), 5 deletions(-)
> diff --git a/dlls/user32/edit.c b/dlls/user32/edit.c
> index d1659b4..c0cb056 100644
> --- a/dlls/user32/edit.c
> +++ b/dlls/user32/edit.c
> @@ -2842,6 +2842,70 @@ static void EDIT_EM_SetLimitText(EDITSTATE *es, UINT limit)
>  }
>
> +static BOOL is_cjk_charset(HDC dc)
> +{
> - switch (GdiGetCodePage(dc)) {
> - case 932: case 936: case 949: case 950: case 1361:
> -
>    return TRUE;
> - default:
> -
>    return FALSE;
> - }
>
> +}
> -
>
> +#ifdef WORDS_BIGENDIAN
> +#define GET_BE_WORD(x) (x)
> +#else
> +#define GET_BE_WORD(x) MAKEWORD(HIBYTE(x), LOBYTE(x))
> +#endif
> -
>
> +#define MS_MAKE_TAG(ch0, ch1, ch2, ch3)                               \
> -
>                ((DWORD)(BYTE)(ch0) | ((DWORD)(BYTE)(ch1) << 8) | \
> -
>                ((DWORD)(BYTE)(ch2) << 16) | ((DWORD)(BYTE)(ch3) << 24))
> -
>
> +static BOOL get_side_bearings(HDC hdc, short *min_lsb, short *min_rsb)
> +{
> - const DWORD hhea_tag = MS_MAKE_TAG('h','h','e','a');
>
> +#include "pshpack2.h"
> - struct {
> -
>    unsigned short majorVersion;
> -
>    unsigned short minorVersion;
> -
>    short ascender;
> -
>    short descender;
> -
>    short lineGap;
> -
>    unsigned short advanceWidthMax;
> -
>    short minLeftSideBearing;
> -
>    short minRightSideBearing;
> -
>    short xMaxExtent;
> -
>    short caretSlopeRise;
> -
>    short caretSlopeRun;
> -
>    short caretOffset;
> -
>    short reserved[4];
> -
>    short metricDataFormat;
> -
>    short numberOfHMetrics;
> - } hhea_table;
>
> +#include "poppack.h"
> - LONG size;
> -
> - size = GetFontData(hdc, hhea_tag, 0, &hhea_table, sizeof(hhea_table));
> - if (size != sizeof(hhea_table))
> -
>    return FALSE;
> - *min_lsb = GET_BE_WORD(hhea_table.minLeftSideBearing);
> - *min_rsb = GET_BE_WORD(hhea_table.minRightSideBearing);
> - return TRUE;
>
> +}

The undocumented gdi32 export GetCharWidthInfo() could help here.
Its prototype seems to be:

struct width_info
{
    INT lsb;
    INT rsb;
    INT unknown;
};

INT WINAPI GetCharWidthInfo(HDC hdc, struct width_info *info);

You'd first want to implement and write tests for this in gdi32.

Huw.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=50037

Your paranoid android.

=== debian9 (32 bit WoW report) ===

user32:
msg.c:8713: Test failed: WaitForSingleObject failed 102
msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead
msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead
msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000