Custom rendering conundrum

[David Kibble]

Hi Folks.

Over the past month or so I've implemented a number of custom rending routines for uGFX but now I've hit on a problem. I've created a set of very nice looking gradient fill and partial gradient fill rendering routines which I'm using without issue. However - I now want to use my partial gradient fill (fills a specific area of the screen with a gradient colour to match the gradient colour of the background at that point) to produce a label which would therefore appear 'transparent'. The label background is my custom gradient fill and the text is as per normal. It's use would be great for something like a digital clock which although functional without this enhancement looks a bit clumsy with a solid background over the gradient.

So first up this took a bit more doing than most of the other customisation I've done as the standard label fill comes from the gdispGFillStringBox function which is buried down in the gdisp code and seems to come with a lot of dependencies when you pull it out. Anyway - sorted that and have my own renamed gdispGFillStringBox and all associated functions out in my own codebase. All compiles and works fine. :-) Now the problem. (Finally I hear you cry !)

My custom rendering uses gdispDrawLine and gdispDrawLine's first instruction is MUTEX_ENTER(g), unfortunately that MUTEX_ENTER(g) is also executed by the gdispGFillStringBox and thus as it stands gdispGFillStringBox cannot therefore call my custom gradient function which uses gdispDrawLine as the MUTEX is already locked.

I hope that makes sense? So at this point I'm really open to some expert guidance on how to solve this. I can see that the gdispGFillStringBox uses some low level functions for it's fill and the gdispGDrawLine uses line_clip(g), but should I try to implement my custom gradient rendering using those low level functions? I guess its not that hard to do, but before I go off and figure it out I'd really appreciate some advice from the experts as to what the correct approach in this sort of situation would be? Have I for example gone off on completely the wrong track here?

Many thanks

Dave

[David Kibble]

Hi Guys.

Sorry for posting an update so soon but I've been having more of a poke around this. I've changed the code in my custom gdisp function to look like this;

void gdispGGradFillStringBox(GWidgetObject * gw, coord_t x, coord_t y, coord_t cx, coord_t cy, const char* str, font_t font, color_t color, color_t bgcolor, justify_t justify)
{
    GDisplay * g = (GDisplay *)gw->g.display;

    #if GDISP_NEED_TEXT_WORDWRAP
        wrapParameters_t wrapParameters;
        uint16_t nbrLines;
    #endif

    g->p.cx = cx;
    g->p.cy = cy;
    g->t.font = font;
    g->t.clipx0 = g->p.x = x;
    g->t.clipy0 = g->p.y = y;
    g->t.clipx1 = x+cx;
    g->t.clipy1 = y+cy;
    g->t.color = color;
    g->t.bgcolor = g->p.color = bgcolor;

    TEST_CLIP_AREA(g) {
        gradientPartialRendering(gw,&mainFormGradConfig);

        // Moved to avoid the MUTEX which locks on (g)
        MUTEX_ENTER(g);
        // background fill
        //fillarea(g);

I added the gw widget param to make it easier to drive my gradientPartialRendering function but the main thing I wanted to check is what is the risk around moving the MUTEX_ENTER(g) in the way I have? Without pouring over the low level code it's hard to know. Just FYI - my application does do a small amount of GUI updates via a background thread with the bulk of the work being on a single GUI thread. I'm assuming that's why you have the MUTEX in there?

Anyway - the result is perfect and is attached. Note the clock in the lower right corner.

Dave

[inmarket]

The mutex is there to protect the GDriver structure from multi-threaded access.

In GDISP there are two types of functions; base calls and high level calls.

Base functions eg gdispDrawLine() take the mutex and use internal private low level calls to complete the operation. It was not intended that a user ever write such a function for their own application as any code inside the mutex is potentionally very complex and there are lots of ways that the code may not work properly on different platforms. These functions hide all the complexity of different display types, multiple display handling, syncing, multi-thread protection and many other hardware related issues from the end user. In many ways they are the core of uGFX.

High level GDISP calls however are much simpler. They use the base functions and therefore are implicitly multithread safe. An example is gdispDrawBox().

So in answer to your question you should wherever possible avoid playing with the mutex or doing anything below that level. If it can be implemented using the GDISP base functions then it should. As you are doing edge to edge and top to bottom gradients this may not be possible except by using the GDISP streaming functions. The GDISP streaming functions are particularly designed for this or video style displaying.

If you believe that your functions really are more appropriate to be in the GDISP base functions then we would be happy to consider their inclusion in the base API. For inclusion we would need source for your functions and we would be checking carefully (and if necessary fix) the issues talked about above to ensure good results for all situations.

By the way, nice functionality! 

[David Kibble]

Many thanks - I'll take a look at moving my gradient draw routines to the low level functions. Once I'm happy with all of this I do still plan to share my gradient code, I guess I'm a bit of a perfectionist so didn't want to pass it on until I'd shaken out all these sorts of issues.

Dave.

[Joel Bodenmann]

Hello David,

We'd definitely love to see your gradient code!

[David Kibble]

Hi Guys.

I'm having to partially give up on this approach. I looked at the low level innards of the line drawing routines and to utilize them I'd either have to pull a huge chunk of the internal gdisp code into my codebase or directly modify your gdisp code. Both of those would be a PITA when the next version of uGFX comes along. So for now what I've done is quite simple and I cannot believe I missed it before;

void gdispGGradFillStringBox(GWidgetObject * gw, coord_t x, coord_t y, coord_t cx, coord_t cy, const char* str, font_t font, color_t color, color_t bgcolor, justify_t justify)
{
    GDisplay * g = (GDisplay *)gw->g.display;
    MUTEX_ENTER(g)

    #if GDISP_NEED_TEXT_WORDWRAP
        wrapParameters_t wrapParameters;
        uint16_t nbrLines;
    #endif

    g->p.cx = cx;
    g->p.cy = cy;
    g->t.font = font;
    g->t.clipx0 = g->p.x = x;
    g->t.clipy0 = g->p.y = y;
    g->t.clipx1 = x+cx;
    g->t.clipy1 = y+cy;
    g->t.color = color;
    g->t.bgcolor = g->p.color = bgcolor;

    TEST_CLIP_AREA(g) {
        MUTEX_EXIT(g)
        gradientPartialRendering(gw,&mainFormGradConfig);
        MUTEX_ENTER(g);
        // background fill
        //fillarea(g);

So it's a bit of a hack - but gives very little window of opportunity for other threads to corrupt 'g'

If you can either expose the internal functions or make the MUTEX's controllable (or have any other ideas how I can link my code to your code without mods) then I can come back to this. For now though, the above works fine for my multi-threaded app.

Just to recap in case anyone else is reading..... The problem here was how to override the rendering of the label component. Easy to create a custom rendering function for the label, but that custom rendering cannot easily use high level gdisp functions like DrawLine because the Label rendering routine itself uses the same MUTEX as the other gdisp functions. Hence if you lift and shift the existing label rendering to modify it, you get stuck in a mutual dependency of MUTEX's.

Thanks for the idea and info though, all much appreciated.

Dave

Edited November 7, 2016 by Joel Bodenmann
Removing unnecessary quote and using code box

[inmarket]

I think there is a misunderstanding here. There is more than one mutex involved.

1. The low level gdisp calls maintain a mutex to protect the GDisplay structure. You should never play with this mutex unless you are writing low level gdisp code.

2. There is a seperate mutex that protects GWIN structures and drawing. This mutex is held by the  gwin code during any gwin drawing routines.

So, within a GWIN custom draw routine you are not allowed to use GWIN drawing routines such as gwinDrawLine as that would attempt to take the gwin mutex again resulting in deadlock. You ARE allowed to use GDISP calls such as gdispDrawLine because that does not touch the gwin mutex.

For new GDISP calls they can either directly use other gdisp calls or they can take the GDisplay mutex and use low level functions available only in gdisp.c such as line_clip. They should NEVER reference anything to do with the GWIN structures as they are below that level.

A GWIN custom draw however should NEVER be touching the GDisplay mutex as that mutex is only for use by the gdisp calls. It should also never touch the GWIN mutex as that is managed by the gwin code itself. A gwin custom draw should only make use of gdisp drawing calls.

As you are looking to change color on every pixel you can either use the gdispDrawPixel call  (or whatever it is called), or you can make use of the more efficient gdisp streaming calls (see the gdisp streaming demo).

The final option is to write a new gradient gdisp area fill call and then use that within your custom draw however that is probably the most complex way of doing things. If you want to try that approach, first try writing it using the gdisp streaming calls and then translate that code block into a proper gdisp call. Using the gdisp streaming calls you don't need to worry about mutexes or driver models, all the nasty hardware and mutex stuff is already taken care of for you allowing you to concentrate on your real code.

[David Kibble]

2 hours ago, inmarket said:

I think there is a misunderstanding here. There is more than one mutex involved.

1. The low level gdisp calls maintain a mutex to protect the GDisplay structure. You should never play with this mutex unless you are writing low level gdisp code.

2. There is a seperate mutex that protects GWIN structures and drawing. This mutex is held by the  gwin code during any gwin drawing routines.

So, within a GWIN custom draw routine you are not allowed to use GWIN drawing routines such as gwinDrawLine as that would attempt to take the gwin mutex again resulting in deadlock. You ARE allowed to use GDISP calls such as gdispDrawLine because that does not touch the gwin mutex.

For new GDISP calls they can either directly use other gdisp calls or they can take the GDisplay mutex and use low level functions available only in gdisp.c such as line_clip. They should NEVER reference anything to do with the GWIN structures as they are below that level.

A GWIN custom draw however should NEVER be touching the GDisplay mutex as that mutex is only for use by the gdisp calls. It should also never touch the GWIN mutex as that is managed by the gwin code itself. A gwin custom draw should only make use of gdisp drawing calls.

As you are looking to change color on every pixel you can either use the gdispDrawPixel call  (or whatever it is called), or you can make use of the more efficient gdisp streaming calls (see the gdisp streaming demo).

The final option is to write a new gradient gdisp area fill call and then use that within your custom draw however that is probably the most complex way of doing things. If you want to try that approach, first try writing it using the gdisp streaming calls and then translate that code block into a proper gdisp call. Using the gdisp streaming calls you don't need to worry about mutexes or driver models, all the nasty hardware and mutex stuff is already taken care of for you allowing you to concentrate on your real code.

Hi - I'm not sure I'm getting you on this;

Here's the standard line draw code;
void gdispGDrawLine(GDisplay *g, coord_t x0, coord_t y0, coord_t x1, coord_t y1, color_t color) {
    MUTEX_ENTER(g);
    g->p.x = x0;
    g->p.y = y0;
    g->p.x1 = x1;
    g->p.y1 = y1;
    g->p.color = color;
    line_clip(g);
    autoflush(g);
    MUTEX_EXIT(g);
}

MUTEX is on GDisplay

Here's the standard gdispFillStringBox which is called by the standard label rendering routine:
    void gdispGFillStringBox(GDisplay *g, coord_t x, coord_t y, coord_t cx, coord_t cy, const char* str, font_t font, color_t color, color_t bgcolor, justify_t justify) {
        #if GDISP_NEED_TEXT_WORDWRAP
            wrapParameters_t wrapParameters;
            uint16_t nbrLines;
        #endif

        MUTEX_ENTER(g);

        g->p.cx = cx;
        g->p.cy = cy;
        g->t.font = font;
        g->t.clipx0 = g->p.x = x;
        g->t.clipy0 = g->p.y = y;
        g->t.clipx1 = x+cx;
        g->t.clipy1 = y+cy;
        g->t.color = color;
        g->t.bgcolor = g->p.color = bgcolor;

        TEST_CLIP_AREA(g) {

            // background fill
            fillarea(g);

Again MUTEX on GDisplay - or am I wrong?

So - my problem is that as I want to use gdispDrawLine from my gradient code which is called from my cusomised FillStringBox the MUTEX on GDisplay is already locked. Hence my workaround to unlock it prior to my gradient draw and then re-lock it all within my cusomised FillStringBox. This is absolutely the problem as a simple GDB session shows and is all at the gdisp layer not the gwin layer. Yes - the cusom label rendering as the gwin layer, but that MUST call the FillStringBox routine in gdisp to repaint the background.

Now it's quite possible I'm missing your point, so apologies in that case, but as far as I can tell there is no separate gwin MUTEX at play in the problem here.

On the streaming front - I'll take a look at that when I have a bit of spare time. Performance on my Cortex A8 board is absolutely fine, can't even see the slightest hint of the redraw occurring.

Many thanks for the help with this.

Dave

[inmarket]

These are low level GDISP functions. Any low level GDISP function should not have any reference to a GWIN object.

I think you are possibly trying to do too much in your gradient call, in particular the drawing of the text.

Your label custom draw should look something like...

customDraw(...) {
	yourGradientFill(gw->g.display, gw->g.x, gw->g.y, gw->g.width, gw->g.height, gradient color params);
	gdispGDrawStringBox(....);
}

Note the use of the DrawStringBox rather than the FillStringBox as you have already filled the background with your gradient.

Your gradient fill routine then can either be a high level call (using other GDISP functions such as the streaming or line functions), OR you can write your own low level gdisp call which takes the mutex and does all the hard work itself (and does not call other GDISP public functions). I would suggest the former.

As an example - have a look at the button standard draw function. You will notice that the buttons have a top to bottom gradient with centered text on top so it will be very similar in code to the custom draw you are trying to write for the label (albeit with a different gradient algorithm).

 

[David Kibble]

Hi.

OK - that is a good call. I'd missed the non-filled gdispGDrawStringBox(...) function. Using that makes the code a lot simpler as now I don't need all the lower level supporting gdisp code for my custom gdispGFillStringBox(...), also thanks for the design guidance re "no gwin calls from gdisp". I'm going to have a look at the streaming API when I get a chance, but the rendering is only taking around 400us max for a whole screen and about 80us for an individual label so it's not a pressing performance issue.

Many thanks

Dave

Edited November 8, 2016 by Joel Bodenmann
Removing unnecessary quote box

[Joel Bodenmann]

It would be nice if you could share your finished rendering function. I'm sure that that could be helpful for many people. Maybe it can even be included in the official µGFX library.

 

2 hours ago, David Kibble said:

the rendering is only taking around 400us max for a whole screen and about 80us for an individual label so it's not a pressing performance issue.

That might be the case on your computer using the SDL back-end. Running this on real hardware (eg. a Cortex-M4) based system will yield way worse results. The type of interface used between the CPU and the actual display will also have a huge impact.

[David Kibble]

Hi Joel.

Many thanks for the reply. Its an ARM Cortex A8 driving a 480x272 RGB mode TFT. The SDL emulator is about 250us. Not tried anything else yet. I've got a BBB knocking around, might benchmark that for fun.

Dave