[PATCH v2 4/8] d3dx9: Implement ID3DXFont_PreloadGlyphs.
Matteo Bruni
matteo.mystral at gmail.com
Wed Jan 22 10:11:56 CST 2020
On Mon, Jan 6, 2020 at 3:35 PM Sven Baars <sbaars at codeweavers.com> wrote:
>
> Based on a patch by Tony Wasserka.
>
> Signed-off-by: Sven Baars <sbaars at codeweavers.com>
> ---
> dlls/d3dx9_36/d3dx9_private.h | 2 +
> dlls/d3dx9_36/font.c | 239 +++++++++++++++++++++++++++++++---
> dlls/d3dx9_36/tests/core.c | 4 +-
> dlls/d3dx9_36/texture.c | 2 +-
> 4 files changed, 228 insertions(+), 19 deletions(-)
>
> diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h
> index 61ec320ba5..4f92b914ef 100644
> --- a/dlls/d3dx9_36/d3dx9_private.h
> +++ b/dlls/d3dx9_36/d3dx9_private.h
> @@ -225,6 +225,8 @@ static inline BOOL is_param_type_sampler(D3DXPARAMETER_TYPE type)
> || type == D3DXPT_SAMPLER3D || type == D3DXPT_SAMPLERCUBE;
> }
>
> +UINT make_pow2(UINT num) DECLSPEC_HIDDEN;
> +
> struct d3dx_parameter;
>
> enum pres_reg_tables
> diff --git a/dlls/d3dx9_36/font.c b/dlls/d3dx9_36/font.c
> index cb09af22f5..fe645a51c0 100644
> --- a/dlls/d3dx9_36/font.c
> +++ b/dlls/d3dx9_36/font.c
> @@ -22,6 +22,14 @@
>
> WINE_DEFAULT_DEBUG_CHANNEL(d3dx);
>
> +typedef struct _GLYPH
> +{
> + UINT id;
> + RECT blackbox;
> + POINT cellinc;
> + IDirect3DTexture9 *texture;
> +} GLYPH;
> +
Please don't add typedefs or type names in caps.
> struct d3dx_font
> {
> ID3DXFont ID3DXFont_iface;
> @@ -32,6 +40,14 @@ struct d3dx_font
>
> HDC hdc;
> HFONT hfont;
> +
> + GLYPH *glyphs;
> + UINT allocated_glyphs, glyph_count;
> +
> + IDirect3DTexture9 **textures;
> + UINT texture_count, texture_pos;
> +
> + UINT texture_size, glyph_size, glyphs_per_texture;
> };
>
> static inline struct d3dx_font *impl_from_ID3DXFont(ID3DXFont *iface)
> @@ -72,11 +88,21 @@ static ULONG WINAPI ID3DXFontImpl_Release(ID3DXFont *iface)
>
> TRACE("%p decreasing refcount to %u\n", iface, ref);
>
> - if(ref==0) {
> + if (!ref)
> + {
> + UINT i;
> + for(i = 0; i < This->texture_count; i++)
> + IDirect3DTexture9_Release(This->textures[i]);
> +
> + if (This->textures)
> + heap_free(This->textures);
> +
> + heap_free(This->glyphs);
> +
> DeleteObject(This->hfont);
> DeleteDC(This->hdc);
> IDirect3DDevice9_Release(This->device);
> - HeapFree(GetProcessHeap(), 0, This);
> + heap_free(This);
> }
> return ref;
> }
Pure nitpick, while at it you could rename This to font or something like that.
> @@ -154,10 +180,164 @@ static HRESULT WINAPI ID3DXFontImpl_PreloadCharacters(ID3DXFont *iface, UINT fir
> return S_OK;
> }
>
> +/************************************************************
> + * ID3DXFont_PreloadGlyphs
> + *
> + * Preloads the specified glyph series into the internal texture
> + *
> + * PARAMS
> + * first [I] first glyph to be preloaded
> + * last [I] last glyph to be preloaded
> + *
> + * RETURNS
> + * Success: D3D_OK
> + *
> + * NOTES
> + * The glyphs are stored in a grid.
> + * Cell sizes vary between different font sizes.
> + * The grid is filled in this order:
> + * 1 2 5 6 17 18 21 22
> + * 3 4 7 8 19 20 23 24
> + * 9 10 13 14 25 26 29 30
> + * 11 12 15 16 27 28 31 32
> + * 33 34 ...
> + * ...
> + * i.e. we try to fill one small square, then three equal-sized squares so that we get one big square, etc...
> + *
> + * The glyphs are positioned around their baseline, which is located at y position glyph_size * i + tmAscent.
> + * Concerning the x position, the glyphs are centered around glyph_size * (i + 0.5).
> + *
> + */
I hate the boilerplate documentation of Win32 methods, I think it
serves no purpose. I'd keep only the (very useful) notes part.
> static HRESULT WINAPI ID3DXFontImpl_PreloadGlyphs(ID3DXFont *iface, UINT first, UINT last)
> {
> - FIXME("iface %p, first %u, last %u stub!\n", iface, first, last);
> - return E_NOTIMPL;
> + struct d3dx_font *This = impl_from_ID3DXFont(iface);
> + UINT glyph, i, x, y;
> + TEXTMETRICW tm;
> + HRESULT hr;
> +
> + TRACE("iface %p, first %u, last %u\n", iface, first, last);
> +
> + if (last < first)
> + return D3D_OK;
> +
> + ID3DXFont_GetTextMetricsW(iface, &tm);
> +
> + for (glyph = first; glyph <= last; glyph++)
> + {
> + DWORD ret;
> + BYTE *buffer;
> + GLYPHMETRICS metrics;
> + D3DLOCKED_RECT lockrect;
> + MAT2 mat = { {0,1}, {0,0}, {0,0}, {0,1} };
> + UINT stride, offx = 0, offy = 0;
> + GLYPH *current_glyph;
> + IDirect3DTexture9 *current_texture;
> +
> + /* Check whether the glyph is already preloaded */
> + for (i = 0; i < This->glyph_count; i++)
> + if (This->glyphs[i].id == glyph)
> + break;
> + if (i < This->glyph_count)
> + continue;
> +
> + /* Calculate glyph position */
> + for (i = 0; i < 16; i++)
> + {
> + if (This->texture_pos & (1 << (2*i)))
> + offx += This->glyph_size * (1 << i);
> + if (i < 15 && This->texture_pos & (1 << (2*i + 1)))
> + offy += This->glyph_size * (1 << i);
> + }
Probably not a big deal but I think that you can compute the x,y
offsets more efficiently. See e.g. DecodeMorton2X() and
DecodeMorton2Y() at
https://fgiesen.wordpress.com/2009/12/13/decoding-morton-codes/ (the
usual version would have bitwise or instead of xor at each step but
it's probably not going to make any difference either way).
> +
> + /* Make sure we have enough memory */
> + if (This->glyph_count + 1 > This->allocated_glyphs)
> + {
> + This->allocated_glyphs <<= 1;
> + This->glyphs = heap_realloc(This->glyphs, This->allocated_glyphs * sizeof(GLYPH));
> + if (!This->glyphs)
> + return E_OUTOFMEMORY;
> + }
This drops the old memory allocation on the floor if the reallocation
fails. There are many other places with a similar pattern in Wine (or
even in d3dx9, e.g. search for realloc) which you can use as
inspiration.
Actually, when looking for those examples I spotted one that does it
wrong in d3dx_pool_sync_shared_parameter(), I'm going to write a patch
for that one...
> +
> + current_glyph = This->glyphs + This->glyph_count++;
> + current_glyph->id = glyph;
> + current_glyph->texture = NULL;
> +
> + /* Spaces are handled separately */
> + if (glyph > 0 && glyph < 4)
> + continue;
I have no clue if those two values are correct for every font but it
smells somewhat fishy...
> +
> + /* Get the glyph data */
> + ret = GetGlyphOutlineW(This->hdc, glyph, GGO_GLYPH_INDEX | GGO_GRAY8_BITMAP, &metrics, 0, NULL, &mat);
> + if (ret == GDI_ERROR)
> + continue;
We might want to print a WARN when that fails.
> +
> + buffer = heap_alloc(ret);
> + if (!buffer)
> + return E_OUTOFMEMORY;
> +
> + GetGlyphOutlineW(This->hdc, glyph, GGO_GLYPH_INDEX | GGO_GRAY8_BITMAP, &metrics, ret, buffer, &mat);
> +
> + /* Create a new texture if necessary */
> + if (This->texture_pos % This->glyphs_per_texture == 0)
It's probably better to maintain a counter for the "slots" used in the
current texture while in this function and avoid a modulo operation
for each loop iteration.
> + {
> + This->textures = heap_realloc(This->textures, (This->texture_count + 1) * sizeof(IDirect3DTexture9 *));
> + if (!This->textures)
> + {
> + heap_free(buffer);
> + return E_OUTOFMEMORY;
> + }
Same issue as above.
> +
> + if (FAILED(hr = IDirect3DDevice9_CreateTexture(This->device, This->texture_size,
> + This->texture_size, 0, 0, D3DFMT_A8R8G8B8, D3DPOOL_MANAGED,
> + &This->textures[This->texture_count], NULL)))
> + {
> + heap_free(buffer);
> + return hr;
> + }
> +
> + This->texture_count++;
> + This->texture_pos = 0;
> + offx = 0;
> + offy = 0;
> + }
> +
> + current_texture = This->textures[This->texture_count - 1];
> +
> + /* Fill in glyph data */
> + current_glyph->blackbox.left = offx - metrics.gmptGlyphOrigin.x + This->glyph_size / 2 - metrics.gmBlackBoxX / 2;
> + current_glyph->blackbox.top = offy - metrics.gmptGlyphOrigin.y + tm.tmAscent;
> + current_glyph->blackbox.right = current_glyph->blackbox.left + metrics.gmBlackBoxX;
> + current_glyph->blackbox.bottom = current_glyph->blackbox.top + metrics.gmBlackBoxY;
> + current_glyph->cellinc.x = metrics.gmptGlyphOrigin.x - 1;
> + current_glyph->cellinc.y = tm.tmAscent - metrics.gmptGlyphOrigin.y - 1;
> + current_glyph->texture = current_texture;
> +
> + /* Copy glyph data to the texture */
> + if (FAILED(hr = IDirect3DTexture9_LockRect(current_texture, 0, &lockrect, ¤t_glyph->blackbox, 0)))
> + {
> + heap_free(buffer);
> + return hr;
> + }
> +
> + stride = (metrics.gmBlackBoxX + 3) & ~3;
> +
> + for (y = 0; y < metrics.gmBlackBoxY; y++)
> + for (x = 0; x < metrics.gmBlackBoxX; x++)
> + {
> + ((BYTE*)lockrect.pBits)[4 * x + y * lockrect.Pitch ] = 255;
> + ((BYTE*)lockrect.pBits)[4 * x + y * lockrect.Pitch + 1] = 255;
> + ((BYTE*)lockrect.pBits)[4 * x + y * lockrect.Pitch + 2] = 255;
> + ((BYTE*)lockrect.pBits)[4 * x + y * lockrect.Pitch + 3] = buffer[x + y * stride] * 255 / 64;
> + }
Introducing a DWORD * variable and filling the whole pixel in one go
should make this quite a bit nicer. You can assume little-endian.
Another stylistic nit: please add brackets to the outer for when the
inner for has them.
> +
> + IDirect3DTexture9_UnlockRect(current_texture, 0);
> +
> + heap_free(buffer);
> +
> + This->texture_pos++;
> + }
> +
> + return D3D_OK;
> }
>
> static HRESULT WINAPI ID3DXFontImpl_PreloadTextA(ID3DXFont *iface, const char *string, INT count)
> @@ -294,6 +474,7 @@ HRESULT WINAPI D3DXCreateFontIndirectW(IDirect3DDevice9 *device, const D3DXFONT_
> D3DDISPLAYMODE mode;
> struct d3dx_font *object;
> IDirect3D9 *d3d;
> + TEXTMETRICW metrics;
> HRESULT hr;
>
> TRACE("(%p, %p, %p)\n", device, desc, font);
> @@ -305,39 +486,65 @@ HRESULT WINAPI D3DXCreateFontIndirectW(IDirect3DDevice9 *device, const D3DXFONT_
> IDirect3DDevice9_GetCreationParameters(device, &cpars);
> IDirect3DDevice9_GetDisplayMode(device, 0, &mode);
> hr = IDirect3D9_CheckDeviceFormat(d3d, cpars.AdapterOrdinal, cpars.DeviceType, mode.Format, 0, D3DRTYPE_TEXTURE, D3DFMT_A8R8G8B8);
> - if(FAILED(hr)) {
> + if (FAILED(hr))
> + {
> IDirect3D9_Release(d3d);
> return D3DXERR_INVALIDDATA;
> }
> IDirect3D9_Release(d3d);
>
> - object = HeapAlloc(GetProcessHeap(), 0, sizeof(struct d3dx_font));
> - if(object==NULL) {
> - *font=NULL;
> + object = heap_alloc_zero(sizeof(struct d3dx_font));
> + if (!object)
> + {
> + *font = NULL;
> return E_OUTOFMEMORY;
> }
> object->ID3DXFont_iface.lpVtbl = &D3DXFont_Vtbl;
> - object->ref=1;
> - object->device=device;
> - object->desc=*desc;
> + object->ref = 1;
> + object->device = device;
> + object->desc = *desc;
>
> object->hdc = CreateCompatibleDC(NULL);
> - if( !object->hdc ) {
> - HeapFree(GetProcessHeap(), 0, object);
> + if (!object->hdc)
> + {
> + heap_free(object);
> return D3DXERR_INVALIDDATA;
> }
>
> object->hfont = CreateFontW(desc->Height, desc->Width, 0, 0, desc->Weight, desc->Italic, FALSE, FALSE, desc->CharSet,
> desc->OutputPrecision, CLIP_DEFAULT_PRECIS, desc->Quality, desc->PitchAndFamily, desc->FaceName);
> - if( !object->hfont ) {
> + if (!object->hfont)
> + {
> DeleteDC(object->hdc);
> - HeapFree(GetProcessHeap(), 0, object);
> + heap_free(object);
> return D3DXERR_INVALIDDATA;
> }
> SelectObject(object->hdc, object->hfont);
>
> + /* allocate common memory usage */
> + object->allocated_glyphs = 32;
> + object->glyphs = heap_alloc(object->allocated_glyphs * sizeof(GLYPH));
> + if (!object->glyphs)
> + {
> + DeleteObject(object->hfont);
> + DeleteDC(object->hdc);
> + heap_free(object);
> + *font = NULL;
> + return E_OUTOFMEMORY;
> + }
> +
> + GetTextMetricsW(object->hdc, &metrics);
> +
> + object->glyph_size = make_pow2(metrics.tmHeight);
> +
> + object->texture_size = make_pow2(object->glyph_size);
> + if (object->glyph_size < 256)
> + object->texture_size = min(256, object->texture_size << 4);
> +
> + object->glyphs_per_texture = object->texture_size * object->texture_size / object->glyph_size / object->glyph_size;
> +
> IDirect3DDevice9_AddRef(device);
> - *font=&object->ID3DXFont_iface;
> + *font = &object->ID3DXFont_iface;
>
> return D3D_OK;
> }
> diff --git a/dlls/d3dx9_36/tests/core.c b/dlls/d3dx9_36/tests/core.c
> index b2a44ded70..4b731288c7 100644
> --- a/dlls/d3dx9_36/tests/core.c
> +++ b/dlls/d3dx9_36/tests/core.c
> @@ -538,7 +538,7 @@ static void test_ID3DXFont(IDirect3DDevice9 *device)
> hr = ID3DXFont_PreloadCharacters(font, 'b', 'a');
> ok(hr == D3D_OK, "ID3DXFont_PreloadCharacters returned %#x, expected %#x\n", hr, D3D_OK);
> hr = ID3DXFont_PreloadGlyphs(font, 1, 0);
> - todo_wine ok(hr == D3D_OK, "ID3DXFont_PreloadGlyphs returned %#x, expected %#x\n", hr, D3D_OK);
> + ok(hr == D3D_OK, "ID3DXFont_PreloadGlyphs returned %#x, expected %#x\n", hr, D3D_OK);
>
> hr = ID3DXFont_PreloadCharacters(font, 'a', 'a');
> ok(hr == D3D_OK, "ID3DXFont_PreloadCharacters returned %#x, expected %#x\n", hr, D3D_OK);
> @@ -590,7 +590,7 @@ static void test_ID3DXFont(IDirect3DDevice9 *device)
>
> /* Test multiple textures */
> hr = ID3DXFont_PreloadGlyphs(font, 0, 1000);
> - todo_wine ok(hr == D3D_OK, "ID3DXFont_PreloadGlyphs returned %#x, expected %#x\n", hr, D3D_OK);
> + ok(hr == D3D_OK, "ID3DXFont_PreloadGlyphs returned %#x, expected %#x\n", hr, D3D_OK);
>
> /* Test glyphs that are not rendered */
> for (glyph = 1; glyph < 4; glyph++)
> diff --git a/dlls/d3dx9_36/texture.c b/dlls/d3dx9_36/texture.c
> index d96ade9aed..47acf42aa1 100644
> --- a/dlls/d3dx9_36/texture.c
> +++ b/dlls/d3dx9_36/texture.c
> @@ -31,7 +31,7 @@ static BOOL is_pow2(UINT num)
> }
>
> /* Returns the smallest power of 2 which is greater than or equal to num */
> -static UINT make_pow2(UINT num)
> +UINT make_pow2(UINT num)
> {
> UINT result = 1;
>
This is okay but I'd prefer to move the helper to d3dx9_private.h and
make it static inline instead.
BTW, the function can be improved like in
<https://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2>.
That would be a separate patch though.
More information about the wine-devel
mailing list