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

[Matteo Bruni]

On Mon, Jan 27, 2020 at 4:59 PM Sven Baars <sbaars at codeweavers.com> wrote:
>
> On 22-01-2020 17:12, Matteo Bruni wrote:
> > On Mon, Jan 6, 2020 at 3:35 PM Sven Baars <sbaars at codeweavers.com> 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?

Which for loop you mean, the for-each-glyph one? My guess was that
individual glyph lookups become essentially negligible once switching
to the RB tree, is that not the case?
Otherwise the only other thing that might make a difference is the
call to GetTextMetricsW. I think it makes sense to either move it in
the code path where it's actually needed (i.e. when inserting new
glyphs) or even just cache it in the font object at creation time.