Re: [PATCH 5/5] d3dx9: Add a very basic ID3DXFont_DrawText implementation.

[Matteo Bruni]

On Wed, Feb 26, 2020 at 12:10 PM Sven Baars  wrote:
> Based on a patch by Tony Wasserka.
> Signed-off-by: Sven Baars
>
> dlls/d3dx9_36/font.c       | 125 +++++++++++++++++++++++++++++++++++--
>  dlls/d3dx9_36/tests/core.c |  59 ++++++++---------
>  2 files changed, 146 insertions(+), 38 deletions(-)

I have a number of style nitpicks in addition to those mentioned in
reply to the previous patch, inline.
> diff --git a/dlls/d3dx9_36/font.c b/dlls/d3dx9_36/font.c
> index 44bd08a9fb..861d083cf7 100644
> --- a/dlls/d3dx9_36/font.c
> +++ b/dlls/d3dx9_36/font.c
> @@ -462,17 +462,130 @@ static HRESULT WINAPI ID3DXFontImpl_PreloadTextW(ID3DXFont *iface, const WCHAR *
>  static INT WINAPI ID3DXFontImpl_DrawTextA(ID3DXFont *iface, ID3DXSprite *sprite,
>          const char *string, INT count, RECT *rect, DWORD format, D3DCOLOR color)
>  {
> - FIXME("iface %p, sprite %p, string %s, count %d, rect %s, format %#x, color 0x%08x stub!\n",
> -
>        iface,  sprite, debugstr_a(string), count, wine_dbgstr_rect(rect), format, color);
> - return 1;
> - INT ret, countW;
> - WCHAR *wstr;
> -
> - TRACE("iface %p, sprite %p, string %s, count %d, rect %s, format %#x, color 0x%08x\n",
> -
>      iface,  sprite, debugstr_a(string), count, wine_dbgstr_rect(rect), format, color);
> -
> - if (!string || count == 0)
> -
>    return 0;

!count
> -
> - countW = MultiByteToWideChar(CP_ACP, 0, string, count < 0 ? -1 : count, NULL, 0);
> -
> - if (countW == 0)
> -
>    return 0;
> -
> - wstr = heap_alloc_zero(countW * sizeof(WCHAR));
> - if (!wstr)
> -
>    return 0;
> -
> - MultiByteToWideChar(CP_ACP, 0, string, count < 0 ? -1 : count, wstr, countW);
> -
> - ret = ID3DXFont_DrawTextW(iface, sprite, wstr, count, rect, format, color);
> -
> - heap_free(wstr);
> -
> - return ret;
>
> }
>
> static INT WINAPI ID3DXFontImpl_DrawTextW(ID3DXFont *iface, ID3DXSprite *sprite,
>          const WCHAR *string, INT count, RECT *rect, DWORD format, D3DCOLOR color)
>  {
> - FIXME("iface %p, sprite %p, string %s, count %d, rect %s, format %#x, color 0x%08x stub!\n",
> -
>        iface,  sprite, debugstr_w(string), count, wine_dbgstr_rect(rect), format, color);
> - return 1;
> - struct d3dx_font *font = impl_from_ID3DXFont(iface);
> - ID3DXSprite *target = sprite;
> - RECT textrect = {0};
> - int lh, x, y;
> - int ret = 0;
> -
> - TRACE("iface %p, sprite %p, string %s, count %d, rect %s, format %#x, color 0x%08x\n",
> -
>      iface,  sprite, debugstr_w(string), count, wine_dbgstr_rect(rect), format, color);
> -
> - if (!string)
> -
>    return 0;
> -
> - if (count < 0)
> -
>    count = lstrlenW(string);
> -
> - if (count == 0)
> -
>    return 0;
> -
> - if (!rect)
> - {
> -
>    y = ID3DXFont_DrawTextW(iface, NULL, string, count, &textrect, format | DT_CALCRECT, 0);
> -
> -
>    if (format & DT_CALCRECT)
> -
>        return y;
> - }
> - else
> -
>    textrect = *rect;

If one of the branches of the if requires braces, the other one should
have them too.
> -
> - x = textrect.left;
> - y = textrect.top;
> -
> - lh = font->metrics.tmHeight;
> -
> - if (!(format & DT_CALCRECT) && !sprite)
> - {
> -
>    D3DXCreateSprite(font->device, &target);
> -
>    ID3DXSprite_Begin(target, 0);
> - }
> -
> - if (!(format & DT_CALCRECT))
> - {
> -
>    GCP_RESULTSW results;
> -
>    D3DXVECTOR3 pos;
> -
>    UINT i;
> -
> -
>    ZeroMemory(&results, sizeof(GCP_RESULTSW));

Just memset() instead. Also you want to use sizeof(results), like in
the memory allocation functions.
> -
>    results.nGlyphs = count;
> -
> -
>    results.lpCaretPos = heap_alloc(count * sizeof(INT));
> -
>    if (!results.lpCaretPos)
> -
>        goto cleanup;
> -
> -
>    results.lpGlyphs = heap_alloc(count * sizeof(WCHAR));
> -
>    if (!results.lpGlyphs)
> -
>    {
> -
>        heap_free(results.lpCaretPos);
> -
>        goto cleanup;
> -
>    }
> -
> -
>    GetCharacterPlacementW(font->hdc, string, count, 0, &results, 0);
> -
> -
>    for (i = 0; i < results.nGlyphs; ++i)
> -
>    {
> -
>        LPDIRECT3DTEXTURE9 texture;

IDirect3DTexture9 *texture;
> -
>        POINT cell_inc;
> -
>        RECT black_box;
> -
> -
>        ID3DXFont_GetGlyphData(iface, results.lpGlyphs[i], &texture, &black_box, &cell_inc);
> -
> -
>        if (!texture)
> -
>            continue;
> -
> -
>        pos.x = cell_inc.x + x + results.lpCaretPos[i];
> -
>        pos.y = cell_inc.y + y;
> -
> -
>        ID3DXSprite_Draw(target, texture, &black_box, NULL, &pos, color);
> -
>        IDirect3DTexture9_Release(texture);
> -
>    }
> -
> -
>    heap_free(results.lpCaretPos);
> -
>    heap_free(results.lpGlyphs);
> - }
> - y += lh;
> -
> - ret = y - textrect.top;
> -
>
> +cleanup:
> - if (target != sprite)
> - {
> -
>    ID3DXSprite_End(target);
> -
>    ID3DXSprite_Release(target);
> - }
> -
> - return ret;
>
> }