Re: [PATCH v2 5/8] d3dx9: Implement ID3DXFont_GetGlyphData.

[Sven Baars]

On 22-01-2020 17:12, Matteo Bruni wrote:
> On Mon, Jan 6, 2020 at 3:35 PM Sven Baars  wrote:
> > -
> > - for (i = 0; i < This->glyph_count; i++)
> > -
> >    if (This->glyphs[i].id == glyph)
> > -
> >    {
> > -
> >        if (cellinc)
> > -
> >            *cellinc = This->glyphs[i].cellinc;
> > -
> >        if (blackbox)
> > -
> >            *blackbox = This->glyphs[i].blackbox;
> > -
> >        if (texture)
> > -
> >            *texture = This->glyphs[i].texture;
> > -
> >        if (texture && *texture)
> > -
> >            IDirect3DTexture9_AddRef(This->glyphs[i].texture);
> > -
> >        return D3D_OK;
> > -
> >    }
> > -
> > - hr = ID3DXFont_PreloadGlyphs(iface, glyph, glyph);
> > - if (FAILED(hr))
> > -
> >    return hr;
> > -
> > - /* Try again */
> > - for (i = 0; i < This->glyph_count; i++)
> > -
> >    if (This->glyphs[i].id == glyph)
> > -
> >    {
> > -
> >        if (cellinc)
> > -
> >            *cellinc = This->glyphs[i].cellinc;
> > -
> >        if (blackbox)
> > -
> >            *blackbox = This->glyphs[i].blackbox;
> > -
> >        if (texture)
> > -
> >            *texture = This->glyphs[i].texture;
> > -
> >        if (texture && *texture)
> > -
> >            IDirect3DTexture9_AddRef(This->glyphs[i].texture);
> > -
> >        return D3D_OK;
> > -
> >    }
>
> A couple of things. Is this "try once, if not found try again"
> justified by tests? Otherwise it seems easier to call PreloadGlyphs()
> unconditionally (i.e. get rid of the first loop).
> The other one is: given that looking up glyphs is going to be a
> frequent operation, linearly searching an array every time isn't going
> to be ideal. We probably want to use the RB tree from
> include/wine/rbtree.h for storing the glyphs.

Hi Matteo,

I implemented this with an RB tree, and after that tested it with and
without the first lookup. With the first lookup my test program has
about 25% more FPS. This is (partially) because it also has to do a
lookup in PreloadGlyphs to see if it's already present. So removing the
first lookup makes it always preform the lookup twice. I could implement
a helper function for the for loop in PreloadGlyphs that returns the
glyph if you like that better. Should I do that?

Cheers,
Sven