FillString flicker

[Timmy Brolin]

These two functions cause a lot of flickering:

gdispGFillString()
gdispGFillStringBox()

The root cause of the flicker is that they start by doing a background fill, and then render the text on top of that.

// background fill
g->p.color = bgcolor;
fillarea(g);

Is there not a better way to do it? Looking at the font rendering code, it seems to call function "skip_pixels()" for all pixels which has alpha=0.

Would it not be possible to modify "skip_pixels()", so that it draws background color to the screen for all alpha=0 pixels when gdispGFillString() / gdispGFillStringBox() are used?

That way the initial fillarea(g) could be removed, and the flickering should be gone. Without the need for any double buffering.

 

Performance bonuses:

* This change alone will improve rendering performance by significantly reducing the total number of bytes written to the display.

* Further performance improvement would be possible by avoiding the gdisp_lld_write_start() + gdisp_lld_write_color() + gdisp_lld_write_stop() sequence which the font rendering is currently doing for almost every pixel, and do pixel streaming instead.

 

There is possibly a small caveat: With kerning enabled, it might be necessary to keep track of the right-most rendered pixel on each line. To avoid having skip_pixels() overwrite parts of the previous character. But that too should be perfectly doable.

Edited September 13 by Timmy Brolin

[Timmy Brolin]

Essentially change this:

static void skip_pixels(struct renderstate_r *rstate, gU16 count)
{
    rstate->x += count;
    while (rstate->x >= rstate->x_end)
    {
        rstate->x -= rstate->x_end - rstate->x_begin;
        rstate->y++;
    }
}

Into this:

static void skip_pixels(struct renderstate_r *rstate, gU16 count)
{
   write_pixels(rstate, count, 0);
}

Plus add some background filling around the edges, and handle kerning. And remove the fillarea(g);

[Joel Bodenmann]

Hello & welcome to the µGFX community!

 

On 13/09/2024 at 12:10, Timmy Brolin said:

The root cause of the flicker is that they start by doing a background fill, and then render the text on top of that.

Is there not a better way to do it?

This is a common issue with graphics rendering not only specific to font rendering. Common solutions include:

• Render the entire element before pushing it to the framebuffer. In µGFX terms that would mean rendering into a pixmap and then blitting to the real display.
• Using global double buffering: You render to an off-screen buffer and once your rendering operation(s) are complete you swap the display's framebuffer.

 

On 13/09/2024 at 12:10, Timmy Brolin said:

Would it not be possible to modify "skip_pixels()", so that it draws background color to the screen for all alpha=0 pixels when gdispGFillString() / gdispGFillStringBox() are used?

That way the initial fillarea(g) could be removed, and the flickering should be gone. Without the need for any double buffering.

I'll need some time to get back to this as it has been a while since I was last digging in the internals of font rendering.
One thing that comes to mind is that fillarea() tends to be a fast operation as this is an operation commonly supported by hardware acceleration.

If you can propose a patch that handles the kerning issue I'd be more than happy to look into it.

 

[Timmy Brolin]

Pixmaps and global double buffering both consume a lot of RAM..

Quote

One thing that comes to mind is that fillarea() tends to be a fast operation as this is an operation commonly supported by hardware acceleration.

True, I suppose the performance will depend on the type of display controller.
At least for streaming SPI displays it should be more efficient to stream filled glyphs, than to first do a fillarea() and then plot the glyphs pixel by pixel.

I'll see if I can put together a patch proposal. It might take me a week or two.
Perhaps a patch without kerning support would suffice as an initial proof-of-concept?
 

[inmarket]

We did look briefly at doing this a few years ago. The difficulty is getting it to work reliably in all situations e.g. with kerning,  antialising etc, and with all fonts. The amount of state required was also significant as well as a number of pre-calculations.

Given the amount of work to do it properly to work in all circumstances, and at the time we were thinking if we would replace the font engine with a vector based font engine, we decided it wasn't worth the effort. There was plenty of other work that was higher priority. (At the time we were working on the window manager).

If you would like to do the work to do this we would certainly appreciate the patches. It is important however that it works with all fonts and font features (such as kerning and antialiasing), and doesn't overrun the specified area by even a single pixel.

[Timmy Brolin]

I have tried several different ways of doing this, and found that the attached patch is the best solution. It supports anti-aliasing, kerning, and bwfonts.

It implements glyph-by-glyph streaming, which both avoids the flicker, and noticeably improves performance.
The only state I have added are the streaming window coordinates and the cursor position.
I use the gdisp generic streaming functions, to take full advantage of the accelerations available also for non-streaming displays.

gdispFillString() previously took 47ms to draw "Hello World" in a 32pt aa font.
This is now down to 35ms. So, about 25% faster.

Glyph-by-glyph streaming is incompatible with kerning, so the patch implements line-by-line glyph streaming when kerning is enabled.
"Hello World" with kerning previously took 55ms, and is now down to 42ms.

Wordwrap is the only feature which is not supported (yet).
gdispGFillStringBox() will fallback to the original implementation if GDISP_NEED_TEXT_WORDWRAP is enabled.

I have tested this on a ST7789 streaming display with SPI bus.

To see the difference in screen refresh flicker, just run this code:

while(1)
   gdispFillString(0, 50, "Hello World", font, GFX_WHITE, GFX_BLACK );

 

FlickerlessFillstring.patch

Edited September 24 by Timmy Brolin

[inmarket]

Thank-you very much. We will review and get it integrated asap.

[Timmy Brolin]

A semi-related observation:

StreamColor() / gdisp_lld_write_color() are called for every single pixel when streaming.

Often in a loop like this:

for(; area; area--)
	gdisp_lld_write_color(g);

When streaming fonts, these functions are called tens of thousands of times.

It would improve efficiency is these functions had a "count" argument instead. It would also be easier to map that to hardware level DMA transfers.
Also, with a count argument, there would no longer be any need for streaming display drivers to implement gdisp_lld_fill_area(), since gdisp_lld_write_color(g, count) would be equally efficient. Simplifying the drivers a bit.

Perhaps something to consider for version 3.0 of ugfx?

[inmarket]

Yes this is something that is part of uGFX v3.

It is also not quite that simple for all types of displays. You might be working with a display that uses a window based driver, but there are also framebuffer drivers, and other strange types of drivers (like window drivers with positioning, dma etc).

There is actually a lot of complexity in just adding that count relating to bounding. At the moment those questions are relatively simple as the calcs are done at the higher layer. When you add the count some of that bounding needs to be moved down a layer, adding to the complexity. This is why this task is in v3. It is at least a major version number update to add that stuff (along with other stuff we wanted in v3).

[Timmy Brolin]

On 28/09/2024 at 08:14, inmarket said:

Yes this is something that is part of uGFX v3.

It is also not quite that simple for all types of displays. You might be working with a display that uses a window based driver, but there are also framebuffer drivers, and other strange types of drivers (like window drivers with positioning, dma etc).

There is actually a lot of complexity in just adding that count relating to bounding. At the moment those questions are relatively simple as the calcs are done at the higher layer. When you add the count some of that bounding needs to be moved down a layer, adding to the complexity. This is why this task is in v3. It is at least a major version number update to add that stuff (along with other stuff we wanted in v3).

Not sure I see why the responsibility of bounding can't remain at the higher layer when a count argument is added?

[Joel Bodenmann]

We had a short internal discussion about this. We'll gladly have a look at your patch but we'll need a bit more time as we're quite busy at the moment.

In general, we're quite keen on this additional/improvement if it is not restrictive to a particular type of driver.

[Timmy Brolin]

18 hours ago, Joel Bodenmann said:

We had a short internal discussion about this. We'll gladly have a look at your patch but we'll need a bit more time as we're quite busy at the moment.

In general, we're quite keen on this additional/improvement if it is not restrictive to a particular type of driver.

There is nothing driver-specific in the patch AFAIK. Performance should be good on any driver, since it uses the generic streaming functions in gdisp which are already optimized for many types of drivers.
But of course, I don't have access to every type of display driver for testing.

[Joel Bodenmann]

4 minutes ago, Timmy Brolin said:

since it uses the generic streaming functions in gdisp which are already optimized for many types of drivers.

Does that mean your patch changes font rendering (the actual painting) to use gdispGStreamStart(), gdispGStreamColor() and gdispGStreamStop()?
And then on top of that giving gdispGStreamColor() an additional "count" parameter?

Apologies if I'm way off here - totally buried at the moment.
 

[Timmy Brolin]

Quote

Does that mean your patch changes font rendering (the actual painting) to use gdispGStreamStart(), gdispGStreamColor() and gdispGStreamStop()?

Firstly:

It was impossible for the font rendering to use the public functions gdispGStreamStart(), gdispGStreamColor() and gdispGStreamStop(), because of the mutex. So the patch make these three functions private with new names: StreamStart(), StreamColor(), StreamStop(), with public APIs gdispGStreamStart(), gdispGStreamColor(), gdispGStreamStop() handling the mutex and calling the private functions.

And now to answer your question:

The patch provides an option GDISP_FLICKERLESS_FILLSTRING. Turn it off, and everything is as before.

Turn it on, then yes font rendering with fill will use StreamStart(), StreamColor(), StreamStop().

You may very well decide to remove the option GDISP_FLICKERLESS_FILLSTRING, and just turn the change on permanently. I just felt it was practical to have it as an option for testing purposes. Makes it easy to switch back and forth.

Quote

And then on top of that giving gdispGStreamColor() an additional "count" parameter?

The patch does NOT add a count parameter. That would be a driver compatibility breaking change.

I just suggest that you add a count parameter in UGFX 3.0.

It is an easy change with obvious advantages, given that gdisp_lld_write_color() and gdispGStreamColor() are currently called in loops in many places in UGFX.

[Timmy Brolin]

Updated patch!

(I found and fixed a cornercase glitch when kerning right-justified text)

FlickerlessFillstring_2.patch