The screen is redrawn more than once

[Sergey Kushnir]

Hello!
I recorded a video The screen is redrawn more than once. I did not find the reason for this phenomenon quickly. I need help.
When uGFX is initialized, screen elements are formed in the usual way, nothing extra.
At the beginning, when you press the button, the screen with the button is hidden and a function is called that makes the screen with TabSet visible.

    case GEVENT_GWIN_BUTTON:            ///(GEVENT_GWIN_CTRL_FIRST+0)
        if (((GEventGWinButton*)pe)->gwin == mainProductButton) {
            gwinHide(mainContainer);
            _ghActiveScreen = productScreenShow();
        }

GHandle productScreenShow(void)
{
    productScreenUpdate();
    gwinShow(prodTabset);
    return prodTabset;
}

Clicking on tabs only causes one screen repaint. See video.

When returning to the previous screen, it is redrawn twice.

I set a breakpoint on the gwinTabsetDraw_Std function and got this call stack:

What should be done to make the screen redrawn once?

[inmarket]

Looking at the video, whenever you swap screens (using either button) it is doing the double drawing. It is much more visible with the tab with the list because font drawing is relatively slow, but it is also happening when you switch to the button screen.

uGFX's window manager is really dumb because the drawing engine doesn't support multiple clip areas, which means it struggles with overlapping Windows. That can often lead to double draw operations. The reason the tab control doesn't have this issue is because the control code itself understands exactly how the tab pages overlap and therefore optimises its drawing for that

Check if when you are switching between the tab window and the button window that you are hiding the old window before making the new window visible so that there is only one visible full screen page at any time.

I hope that helps

[Sergey Kushnir]

I went through the debugger and figured out the logic of the window manager. The _gwinFlushRedraws function contains two branches: "Do loss of visibility first" and "Do the visible windows next". The new window is redrawn twice, here's why. The first time as a hidden window, but the visibility flag is already set. The second time as a window that is visible. It turns out that we need to somehow make it so that the new window is not redrawn as invisible. Is it possible to do this?

[inmarket]

It has been a long time since I looked at the code but...

It should be possible to retest the visibility flag in the invisible loop. If the window is now visible don't redraw it because it will get redrawn later.

[Sergey Kushnir]

I continued my research.

The _gwinFlushRedraws function handles hiding the window in the "Do loss of visibility first" branch. The GWIN_FLG_NEEDREDRAW and GWIN_FLG_SYSVISIBLE flags are checked for all windows. If the GWIN_FLG_SYSVISIBLE flag is set, the window does not require hiding and its redrawing is ignored. I marked this place with the "1==>" label.
If redrawing is required, i.e. the GWIN_FLG_NEEDREDRAW flag is set and GWIN_FLG_SYSVISIBLE is cleared, then the GWINwm->vmt->Redraw(gh) function is called. I marked this place with the "2==>" label.

void _gwinFlushRedraws(GRedrawMethod how) {
	GHandle		gh;

	// Do we really need to do anything?
	if (!RedrawPending)
		return;

	// Obtain the drawing lock
	if (how == REDRAW_WAIT)
		gfxSemWait(&gwinsem, gDelayForever);
	else if (how == REDRAW_NOWAIT && !gfxSemWait(&gwinsem, gDelayNone))
		// Someone is drawing - They will do the redraw when they are finished
		return;

	// Do loss of visibility first
	while ((RedrawPending & DOREDRAW_INVISIBLES)) {
		RedrawPending &= ~DOREDRAW_INVISIBLES;				// Catch new requests

		for(gh = gwinGetNextWindow(0); gh; gh = gwinGetNextWindow(gh)) {
1==>		if ((gh->flags & (GWIN_FLG_NEEDREDRAW|GWIN_FLG_SYSVISIBLE)) != GWIN_FLG_NEEDREDRAW)
				continue;

			// Do the redraw
			#if GDISP_NEED_CLIP
				gdispGSetClip(gh->display, gh->x, gh->y, gh->width, gh->height);
2==>			_GWINwm->vmt->Redraw(gh);
				gdispGUnsetClip(gh->display);
			#else
				_GWINwm->vmt->Redraw(gh);
			#endif

			// Postpone further redraws
			#if !GWIN_REDRAW_IMMEDIATE && !GWIN_REDRAW_SINGLEOP
...

The GWINwm->vmt->Redraw(gh) function also has two branches of rendering. The first branch renders the window if the GWIN_FLG_SYSVISIBLE flag is set. The second branch either does nothing or, if the GWIN_FLG_BGREDRAW flag is set, clears the area and renders windows that have the GWIN_FLG_SYSVISIBLE flag set. This is where the next window is called when the previous one is hidden. I marked this place with the label "3==>".

static void WM_Redraw(GHandle gh) {
	gU32	flags;

	flags = gh->flags;
	gh->flags &= ~(GWIN_FLG_NEEDREDRAW|GWIN_FLG_BGREDRAW|GWIN_FLG_PARENTREVEAL);

	#if GWIN_NEED_CONTAINERS
		redo_redraw:
	#endif
	if ((flags & GWIN_FLG_SYSVISIBLE)) {
		if (gh->vmt->Redraw)
			gh->vmt->Redraw(gh);
		else if ((flags & GWIN_FLG_BGREDRAW)) {
			// We can't redraw but we want full coverage so just clear the area
			gdispGFillArea(gh->display, gh->x, gh->y, gh->width, gh->height, gh->bgcolor);

			// Only do an after clear if this is not a parent reveal
			if (!(flags & GWIN_FLG_PARENTREVEAL) && gh->vmt->AfterClear)
				gh->vmt->AfterClear(gh);
		}

		#if GWIN_NEED_CONTAINERS
			// If this is container but not a parent reveal, mark any visible children for redraw
			//	We redraw our children here as we have overwritten them in redrawing the parent
			//	as GDISP/GWIN doesn't support complex clipping regions.
			if ((flags & (GWIN_FLG_CONTAINER|GWIN_FLG_PARENTREVEAL)) == GWIN_FLG_CONTAINER) {

				// Container redraw is done

				for(gh = gwinGetFirstChild(gh); gh; gh = gwinGetSibling(gh))
					_gwinUpdate(gh);
				return;
			}
		#endif

	} else {
		if ((flags & GWIN_FLG_BGREDRAW)) {
			GHandle		gx;

			#if GWIN_NEED_CONTAINERS
				if (gh->parent) {
					// Child redraw is done

					// Get the parent to redraw the area
					gh = gh->parent;

					// The parent is already marked for redraw - don't do it now.
					if ((gh->flags & GWIN_FLG_NEEDREDRAW))
						return;

					// Use the existing clipping region and redraw now
					gh->flags |= (GWIN_FLG_BGREDRAW|GWIN_FLG_PARENTREVEAL);
					goto redo_redraw;
				}
			#endif

			// Clear the area to the background color
			gdispGFillArea(gh->display, gh->x, gh->y, gh->width, gh->height, gwinGetDefaultBgColor());

			// Now loop over all windows looking for overlaps. Redraw them if they overlap the newly exposed area.
3==>		for(gx = gwinGetNextWindow(0); gx; gx = gwinGetNextWindow(gx)) {
				if ((gx->flags & GWIN_FLG_SYSVISIBLE)
						&& gx->display == gh->display
						&& gx->x < gh->x+gh->width && gx->y < gh->y+gh->height && gx->x+gx->width >= gh->x && gx->y+gx->height >= gh->y) {
					if (gx->vmt->Redraw)
						gx->vmt->Redraw(gx);
					else
						// We can't redraw this window but we want full coverage so just clear the area
						gdispGFillArea(gx->display, gx->x, gx->y, gx->width, gx->height, gx->bgcolor);
				}
			}

		}
	}
}

I believe this is due to the specific implementation of the concept of separating the change of window rendering flags in one operating system thread and the rendering process itself in another thread.

[inmarket]

Hmm.

I can see what might be a bug that you can try...

<code>

     gh->flags |= (GWIN_FLG_BGREDRAW|GWIN_FLG_PARENTREVEAL);

     goto redo_redraw;

</code>

As the flags are saved before the redo_redraw label, I think modifying the gh->flags is probably a bug. It should perhaps just read "flags |= ...".

Give that change a try and let me know if it helps.

[Sergey Kushnir]

On 14/03/2025 at 23:31, inmarket said:

Hmm.

I can see what might be a bug that you can try...

<code>

     gh->flags |= (GWIN_FLG_BGREDRAW|GWIN_FLG_PARENTREVEAL);

     goto redo_redraw;

</code>

As the flags are saved before the redo_redraw label, I think modifying the gh->flags is probably a bug. It should perhaps just read "flags |= ...".

Give that change a try and let me know if it helps.

This code is executed only if the window has a parent. In my case, the container and tabset do not have parents. Therefore, the code is not executed and the code immediately switches to executing first the area clearing, and then searching for windows with visibility flags set and redrawing them.  I previously marked this place with the label "3==>".

[Sergey Kushnir]

I modified the WM_Redraw function code as follows and the double redrawing does not occur.

		if ((flags & GWIN_FLG_BGREDRAW)) {
			GHandle		gx;

			#if GWIN_NEED_CONTAINERS
				if (gh->parent) {
					// Child redraw is done

					// Get the parent to redraw the area
					gh = gh->parent;

					// The parent is already marked for redraw - don't do it now.
					if ((gh->flags & GWIN_FLG_NEEDREDRAW))
						return;

					// Use the existing clipping region and redraw now
					gh->flags |= (GWIN_FLG_BGREDRAW|GWIN_FLG_PARENTREVEAL);
					goto redo_redraw;
->				}else{
->                    gdispGFillArea(gh->display, gh->x, gh->y, gh->width, gh->height, gwinGetDefaultBgColor());
->                    return;
                }

			#endif

			// Clear the area to the background color
			gdispGFillArea(gh->display, gh->x, gh->y, gh->width, gh->height, gwinGetDefaultBgColor());

 

[inmarket]

Hmm. I don't think that is right, and even if it is works it will make the redraw require 2 redraw cycles to work.

I think the problem is the bottom gdispGFillArea that is using gx. Whilst WM_Redraw clears any pending drawing operations, the gdispGFillArea doesn't and it should in this case.

Try putting back the original code and then changing that bottom gdispGFillArea to...

<code>

else {

    gdispGFillArea(gx....

    gx->flags &= ~(GWIN_FLG_NEEDREDRAW|GWIN_FLG_BGREDRAW|GWIN_FLG_PARENTREVEAL);

}

</code>

The other area I pointed out is also probably still a bug but that one needs more testing.

[Sergey Kushnir]

No, two times.

 

[Sergey Kushnir]

As I noted earlier, The _gwinFlushRedraws function contains two branches: "Do loss of visibility first" and "Do the visible windows next". The first branch is for hiding the window. The second branch is for drawing windows with visibility flags set.
When a window needs to be hidden, the GWIN_FLG_BGREDRAW flag is set, which is processed in the corresponding branch of the WM_Redraw function. A check is performed, does the window have a parent? If there is a parent, then this parent needs to be redrawed. If there is no parent, then there is nothing to redraw. In my opinion, the logic is as follows. The windows that need to be shown are processed in the second branch of the _gwinFlushRedraws function.
Based on this reasoning, I modified the code as follows and marked this places with the label ">>":

static void WM_Redraw(GHandle gh) {
	gU32	flags;
>>	gU32	parents = 0;

	flags = gh->flags;
	gh->flags &= ~(GWIN_FLG_NEEDREDRAW|GWIN_FLG_BGREDRAW|GWIN_FLG_PARENTREVEAL);

	#if GWIN_NEED_CONTAINERS
		redo_redraw:
	#endif
	if ((flags & GWIN_FLG_SYSVISIBLE)) {
		if (gh->vmt->Redraw)
			gh->vmt->Redraw(gh);
		else if ((flags & GWIN_FLG_BGREDRAW)) {
			// We can't redraw but we want full coverage so just clear the area
			gdispGFillArea(gh->display, gh->x, gh->y, gh->width, gh->height, gh->bgcolor);

			// Only do an after clear if this is not a parent reveal
			if (!(flags & GWIN_FLG_PARENTREVEAL) && gh->vmt->AfterClear)
				gh->vmt->AfterClear(gh);
		}

		#if GWIN_NEED_CONTAINERS
			// If this is container but not a parent reveal, mark any visible children for redraw
			//	We redraw our children here as we have overwritten them in redrawing the parent
			//	as GDISP/GWIN doesn't support complex clipping regions.
			if ((flags & (GWIN_FLG_CONTAINER|GWIN_FLG_PARENTREVEAL)) == GWIN_FLG_CONTAINER) {

				// Container redraw is done

				for(gh = gwinGetFirstChild(gh); gh; gh = gwinGetSibling(gh))
					_gwinUpdate(gh);
				return;
			}
		#endif

	} else {
		if ((flags & GWIN_FLG_BGREDRAW)) {
			GHandle		gx;


			#if GWIN_NEED_CONTAINERS
				if (gh->parent) {
					// Child redraw is done
>>                    parents++;
					// Get the parent to redraw the area
					gh = gh->parent;

					// The parent is already marked for redraw - don't do it now.
					if ((gh->flags & GWIN_FLG_NEEDREDRAW))
						return;

					// Use the existing clipping region and redraw now
					gh->flags |= (GWIN_FLG_BGREDRAW|GWIN_FLG_PARENTREVEAL);
					goto redo_redraw;
                }

			#endif

			// Clear the area to the background color
			gdispGFillArea(gh->display, gh->x, gh->y, gh->width, gh->height, gwinGetDefaultBgColor());
          
>>			#if GWIN_NEED_CONTAINERS
>>                if (!parents) return;
>>			#endif
              
			// Now loop over all windows looking for overlaps. Redraw them if they overlap the newly exposed area.
			for(gx = gwinGetNextWindow(0); gx; gx = gwinGetNextWindow(gx)) {
				if ((gx->flags & GWIN_FLG_SYSVISIBLE)
						&& gx->display == gh->display
						&& gx->x < gh->x+gh->width && gx->y < gh->y+gh->height && gx->x+gx->width >= gh->x && gx->y+gx->height >= gh->y) {
					if (gx->vmt->Redraw)
						gx->vmt->Redraw(gx);
					else
						// We can't redraw this window but we want full coverage so just clear the area
						gdispGFillArea(gx->display, gx->x, gx->y, gx->width, gx->height, gx->bgcolor);
				}
			}

		}
	}
}

I introduced an additional variable that indicates whether the window has a parent. If there is no parent, then no redrawing occurs. If there is a parent, then it will be redrawed either in this function or later.
The gx variable is an auxiliary variable-pointer to an object in the list. Therefore, any setting of flags in this variable is meaningless.
I think this code meets your expectations.

With this code, redrawing occurs only once.

Edited March 17 by Sergey Kushnir