FreeLists / haiku-appserver / [haiku-appserver] Re: TextLayoutEngine

["Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>]

Stephan Aßmus <superstippi@xxxxxx> wrote:
> I started to work on extracting a class for general glyph processing.
> It
> would use the AGG glyph cache and be a global instance class.
> Whenever some
> app_server object needs to iterate over glyphs, it will go through
> the
> global TextLayoutEngine. This object has a glyph cache on its
> backend. It
> will use a template class "GlyphConsumer" such as this:
>
> template<class GlyphConsumer>
> status_t
> TextLayoutEngine::LayoutGlyphs(const char* utf8String, int32 length,
> GlyphConsumer& consumer);
>
> A glyph consumer can be anything. The TextLayoutEngine expects the
> GlyphConsumer template to export a function ConsumeGlyph(const
> agg::glyph_cache* glyph);
>
> A GlyphConsumer can be an object which sums up the width of the
> glyphs
> layouted by the TextLayoutEngine. It could be the AGGTextRenderer.
> The
> point is that we would get rid of all this code duplication to
> iterate over
> glyphs at all those places.

The basic idea sounds very good, although I am not sure why that method
should be called LayoutGlyphs() (oh, naming again).

> The global TextLayoutEngine instance would have two functions for
> granting
> exclusive access to it:
[...]
> I think this change would greatly clean up the text layouting code.

Definitely.

> Any objections or suggestions?

Yes, you can actually take it as a little bit of both.
I don't like the locking model, and I actually don't see much reason
why it should work that way.

AFAICT all font functions only require read access to the font cache -
only if the glyph they are demanding is not yet in the cache, you would
need something like a write lock.
But even that write lock should not be held while rendering/retrieving
the glyph - other threads may want to (read) access the cache while
doing this, and it this should be allowed.
IMO a finer grained lock is needed, possibly the best way is to do this
per font style (similar to what happens now, just cleaner).
So when you need glyphs you would not need Acquire/Release anymore, but
LayoutGlyph() would get a ServerFont parameter.

Inside, the lookup code could look like this:

        read lock cache
                lookup glyph for style?
                        yes, it's there, return it (and unlock the cache, of
course)
                        no, it's not there, just proceed
        unlock cache

        lock font style
                is glyph there now?
                        if yes, revert to a read lock, and bail out
                        no? then proceed
                retrieve glyph
                write lock cache
                        insert glyph into cache
                unlock cache
        unlock font style

        return glyph

Of course, as long as you work on a glyph, it needs its reference count
incremented.
The FTFace object must only be used while the font lock is held - only
the fixed values can be read at all times (like its name).

That's more or less what I would have done, at least. What do you
think?

The most important thing is that it allows read access at almost all
times - assuming that most of the characters will be cached; as there
are rarely more than 5 fonts visible on screen this should be
realistic, and therefore that case should to be optimized.

Bye,
   Axel.