GrCopyArea all 0s #61

LOGUNIVPM · 2021-10-22T10:21:57Z

Hi, I have an embedded Linux application that was previously using Microwin v0.91 (2005) and running on an old Arm CPU.
Now I'm porting the project to a new system (i.MX6 UL, armv7l) using the latest version of microwin.
My source code is well tested (>15 years of life) and was previously running without issues.

I have spent a few days adjusting the build config for this new system and the graphic library works as expected, APART from an issue that can be troublesome. We draw popups on our display and we need to backup the raster below the popup with GrCopyArea and restore it back after the popup gets deleted. Unfortunately, GrCopyArea seems not to work: it copies 0s to the raster when we try to restore the area below the popup, therefore after the popup is deleted we get a black spot below it.

I have debugged this and can't find anything wrong (from my little understanding of microwin guts). I reached one of the low level functions (rasteropGeneralLow) and can't see anything wrong (like a failed assert, a return before really copying the data, etc). The only thing I noticed is that the source and dest pitch are different: src pitch 8, dst pitch 16
What could be wrong?

The display is 1 bit only (the pixels can be on/off). These are some of the important fields in the config file:

SCREEN_PIXTYPE           = MWPF_PALETTE
SCREEN_DEPTH             = 1

SCREEN                   = FB

SCREEN_WIDTH             = 128
SCREEN_HEIGHT            = 64

MICROWIN                 = N
NANOX                    = Y
NUKLEARUI                = N
NX11                     = N
ENGINE                   = N
TINYWIDGETS              = N

LINK_APP_INTO_SERVER     = Y

HAVE_SHAREDMEM_SUPPORT   = Y

The text was updated successfully, but these errors were encountered:

LOGUNIVPM · 2021-10-22T12:38:31Z

An additional note: I have compiled the old version of Microwin and linked my application to it. The bug doesn't appear. I believe it's a regression in microwin.

ghaerr · 2021-10-22T15:29:00Z

Hello @LOGUNIVPM,

Thank you for your bug report!

I would say that if your previous application worked, and nothing was changed except the Microwindows library version, the problem is in Microwindows. One of the biggest things that changed between older and new versions is that the image copy functions (like GrCopyArea) moved from being performed on a pixel-by-pixel basis to using fast, low-level 'blit' routines instead.

The blit routines are dependent on the "image" attributes, especially pitch, for which a "conversion" blit may be performed - this is required when the source and destination image formats are not the same depth (and other reasons) and cannot be performed by the normal low level blitter.

Before we dive into that, I wonder why the pitch is different, since I'm presuming you're using GrReadArea or GrCopyArea to read screen bits into a buffer, then write them back out using GrCopyArea? It may be that the backup image (or buffer) being created is being assigned the wrong pitch, and subsequently the blit is failing. There should not be pitch differences between the buffers, IMO.

Can you post the code fragments of the copy out and copy back so we can take a closer look? Also, is there any way of actually checking the backup buffer to see whether its contents are correct, or whether they're already 0's?

Thank you!

LOGUNIVPM · 2021-10-22T16:22:18Z

Thank you for the prompt reply (and the amazing work with microwin).

These are the bites of code you need:

void LcdPopup(...) {
    	...
    	// backup of the raster
	popup_rect.width += 8; // 3 white columns border
	popup_rect.x = (LCD_WIDTH - popup_rect.width) >> 1;
	popup_rect.y = (LCD_HEIGHT - popup_rect.height) >> 1;
    	popup_bck = GrNewPixmap(popup_rect.width, popup_rect.height, NULL);
	GrCopyArea(popup_bck, gc, 0, 0, popup_rect.width, popup_rect.height,
			w, popup_rect.x, popup_rect.y, MWROP_USE_GC_MODE);
    	...
}

void LcdPopupDestroy() {
    	...
    	// restore area
    	GrCopyArea(w, gc, popup_rect.x, popup_rect.y, popup_rect.width,
			popup_rect.height, popup_bck, 0, 0, MWROP_USE_GC_MODE);
	GrDestroyWindow(popup_bck);
}

I think they are self-explaining.

I have also tried to look at the memory. I have not been able to sort out the correct way to read the data from the memory. I have tried a clumsy attempt at writing the raster on the terminal by interpreting the memory area of the source and destination PSD in the following way, where ri is the row counter, co is the column counter and bi is the bit counter (I have a 1bit framebuffer, I am expecting microwin to use a bitfield notation, storing 8 pixels in 1 byte).

        int ri,co, bi;
        for (ri = 0; ri < height; ri++) {
                for (co = 0; co < width/8; co++) {
                        for (bi = 0; bi < 8; bi++) {
                                printf((srcpsd->addr[co+ri*width] & (1 << bi)) == 0  ? " " : "#");
                        }
                }
                printf("\n");
        }

Here is what I get (the image doesn't make sense but maybe it tells us something about the memory)

BACKUP PROCEDURE

SRC BEFORE COPY
########################################
##### ######## #        # ##  ##    ####
# ## ##   ##  ### ### ##   ### #     #  
        # ######       #        ##      
########################################
# #### ###   #####    ####     #### ##  
##   #  #   ##   ##    #     ##         
##    ##       ###    ######  ##  ####  
########################################
#### ####  ## ##    ## # ######   ######
# ##      ## ## ## ## ####  ## # ### ## 
######   ######    #######   #  ###     
######################################  
################ ############## ########
################################ ##    #
 ############## ##########    ####### ##


----------------
DST AFTER COPY
                                        
                                        
   #####                       #        
                       #

RESTORE PROCEDURE

SRC BEFORE COPY
                                        
                                        
   #####                       #        
                       #                
                                        
                                        
                                        
                                        
                                        
                                        
                                        
                                        
                                        
                                        
                                        
                                        


----------------
DST AFTER COPY
########################################
##### ######## #        # ##  ##    ####
# ## ##   ##  ### ### ##   ### #     #  
        # ######       #        ##      
########################################
# #### ###   #####    ####     #### ##  
##   #  #   ##   ##    #     ##         
##    ##       ###    ######  ##  ####  
########################################
#### ####  ## ##    ## # ######   ######
# ##      ## ## ## ## ####  ## # ### ## 
######   ######    #####                
######################################  
########                                
################################ ##    #
                        ##      ##### ##

Hope this helps! Thank you

ghaerr · 2021-10-22T16:39:27Z

I'm in the midst of another project, but will try to help you get this solved until I can dive deep into it. I don't have a 1bpp system to test on either.

The GrNewPixmap is a variant of GrNewPixmapEx which can take an image format specifier, which I think for your system would be MWIF_1BPP. This routine then calls GdCreatePixmap which doesn't actually handle MWIF_1BPP, but should default to the screen display, since 0 is passed. That routine is in drivers/genmem.c, where you'll see it picks up the data_format and pixtype. It would be interesting to know what those values are. The rest of that routine allocates memory for the pixmap, then adjusts the image settings properly, which may not be the case. We don't know yet whether the problem is in the copy out, or copying back.

With regards to printing the image, I think your code is correct in that 1BPP images are stored as 8 bits per byte, but you may have to reverse the MSB and LSB order to display properly in your debug routine. That is, read from MSB to LSB for the 'bi' variable.

I don't quite understand your SRC/DST before and after copy; they seem the same. Lets get your debug routine to print the image properly and go from there.

LOGUNIVPM · 2021-10-25T09:05:21Z

Thank you. The old version of the library works for me, but let's see if I can help.
BTW I should point out that I needed to patch the source in order to make it work on this 1bit system. The old version of our framebuffer driver would reply to a query with different values, correctly handled by microwin, with the new linux driver I had to do the following (see the ifdef PATCH_LINUX_AXEL_UL):

	/* set pixel format*/
	if(visual == FB_VISUAL_TRUECOLOR || visual == FB_VISUAL_DIRECTCOLOR) {
		switch(psd->bpp) {
		case 8:
			psd->pixtype = MWPF_TRUECOLOR332;
			break;
		case 16:
			if (fb_var.green.length == 5)
				psd->pixtype = MWPF_TRUECOLOR555;	// FIXME must also set MWPF_PIXELFORMAT in config
			else
				psd->pixtype = MWPF_TRUECOLOR565;
			break;
		case 18:
		case 24:
			psd->pixtype = MWPF_TRUECOLORRGB;
			break;
		case 32:
			psd->pixtype = MWPF_TRUECOLORARGB;
			break;
		default:
#ifdef PATCH_LINUX_AXEL_UL
			if (psd->bpp == 1) {
				EPRINTF("WARNING: 1BPP TRUECOLOR, forcing PALETTE mode\n");
				psd->pixtype = MWPF_PALETTE;
				break;
			}
#endif
			EPRINTF("Unsupported %d color (%d bpp) truecolor framebuffer\n", psd->ncolors, psd->bpp);
			goto fail;
		}

#if LINUX_SPARC
#define CG3_MMAP_OFFSET 0x4000000
#define CG6_RAM    		0x70016000
#define TCX_RAM8BIT		0x00000000
#define TCX_RAM24BIT	0x01000000
	switch (fb_fix.accel) {
	case FB_ACCEL_SUN_CGTHREE:
		psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_SHARED,fb,CG3_MMAP_OFFSET);
		break;
	case FB_ACCEL_SUN_CGSIX:
		psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_SHARED,fb,CG6_RAM);
		break;
	case FB_ACCEL_SUN_TCX:
		psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_SHARED,fb,TCX_RAM24BIT);
 		break;
	default:
		EPRINTF("Don't know how to mmap %s with accel %d\n", MW_PATH_FRAMEBUFFER, fb_fix.accel);
		goto fail;
	}
#elif LINUX_BLACKFIN
	psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_FILE,fb,0);
#elif UCLINUX
	psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,0,fb,0);
#ifdef PATCH_LINUX_AXEL_UL
	printf("mmaping %d bytes to %s\n", psd->size, MW_PATH_FRAMEBUFFER);
	psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_SHARED,fb,0);
#else
	psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_SHARED,fb,0);
#endif
	if(psd->addr == NULL || psd->addr == (unsigned char *)-1) {
		EPRINTF("Error mmaping %s: %m\n", MW_PATH_FRAMEBUFFER);
		goto fail;
	}

Maybe this has to do with it?

LOGUNIVPM · 2021-10-25T09:08:06Z

I have tried to fix the printing of the area, still something wrong, but at least I can read some characters on screen. As you can see the backup copy is not working and the restored copy seems like the original but with mangled bits.

BACKUP

SRC BEFORE COPY

################################################
#     ###### ##  ## ### ## #### ### ## ######   
#       ##   ##  ## ##  ##  ##  ##  ## ##  ##   
#                                               
#     ##  ##  ####  ####  ####  ######  #####   
#     ##  ## ##  ##  ##  ##     #####   ####    
#       ##    ####  ####  ####  ###### #####    
#                                               
#    ##       ##   ###### ##     ##     ##      
#    ######   ##     ##   ###### ###### ######  
################################################
################################################
#################    ##     #  #  #     #  #####
################################################
####     #  ## ##    #     #  #  #    ##  ####  
####  #### ##  ####  #  #### ### #  #  #  ####  


----------------
DST AFTER COPY
             ##       ##                    ##  
        ################                ########
             #  #####  #                #######

RESTORE


----------------
SRC BEFORE COPY
Reading 16 rows, 6 cols(applied/8)
         ##     ##                        ##    
        ################                ########
          #     #  #####                 #######
                                                
                                                
                                                
                                                
                                                
                                                
                                                
                                                
                                                
                                                
                                                
                                                
                                                


----------------
DST AFTER COPY
################################################
##     # ## #### ### ##  #### ### ## ###   #####
       # ##   ##  ## ##   ##  ### ##  ##   ##  #
       #                                        
##     ###  ##  ####  ##  ####    ######   #####
##     # ## ##   ##  ##      ##    #####    ####
       ###    ######  ##  ####  # ######    ####
       #                                        
 ##    ###      #####       ## #     ##       ##
###    ###   ### ##     ######   ######   ######
################################################
################################################
################ ##    ##  #         #  #####  #
################################################
    ##### ##  #   #    # #  #   ##    #   ####  
##  #####  ## ##  #  ###### #####  #  #   ####

LOGUNIVPM · 2021-10-25T09:09:19Z

Regarding the function GrNewPixmapEx, I can't see where is the format applied. It is defined like this in nano-X.h:

#define GrNewPixmap(width,height,pixels) GrNewPixmapEx(width,height,0,pixels)

The format argument is always zero

ghaerr · 2021-10-25T16:46:50Z

Thanks for the information. Yes, the new source tree will likely have to be patched with the PATCH_LINUX_AXEL_UL contents. Do you happen to have a copy of that patch, or could you generate that with a diff -urN from the original patched versions?

With regards to GrNewPixmapEx, yes, the format is passed as 0, which picks up the default case in drivers/genmem.c::GdCreatePixmap(). This will default to creating an offscreen image using the root screen's attributes by default. Since your above patch changes that to "psd->pixtype = MWPF_PALETTE", that could be the source of the problem.

If I could see the old full patch, I could submit a PR which tries to make this work.

Thank you!

LOGUNIVPM · 2021-10-27T09:06:29Z

Here is the diff

--- microwin/src/drivers/scr_fb.c	2021-09-13 15:31:39.475000999 +0200
+++ microwindows/src/drivers/scr_fb.c	2021-10-27 10:57:30.165026425 +0200
@@ -161,7 +161,7 @@
 
 	psd->pitch = fb_fix.line_length;
 	psd->size = psd->yres * psd->pitch;
-	psd->flags = PSF_SCREEN;
+    psd->flags = PSF_SCREEN;
 
 	/* set pixel format*/
 	if(visual == FB_VISUAL_TRUECOLOR || visual == FB_VISUAL_DIRECTCOLOR) {
@@ -183,13 +183,6 @@
 			psd->pixtype = MWPF_TRUECOLORARGB;
 			break;
 		default:
-#ifdef PATCH_LINUX_AXEL_UL
-			if (psd->bpp == 1) {
-				EPRINTF("WARNING: 1BPP TRUECOLOR, forcing PALETTE mode\n");
-				psd->pixtype = MWPF_PALETTE;
-				break;
-			}
-#endif
 			EPRINTF("Unsupported %d color (%d bpp) truecolor framebuffer\n", psd->ncolors, psd->bpp);
 			goto fail;
 		}
@@ -256,9 +249,6 @@
 	psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_FILE,fb,0);
 #elif UCLINUX
 	psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,0,fb,0);
-#elif PATCH_LINUX_AXEL_UL
-	printf("mmaping %d bytes to %s\n", psd->size, MW_PATH_FRAMEBUFFER);
-	psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_SHARED,fb,0);
 #else
 	psd->addr = mmap(NULL, psd->size, PROT_READ|PROT_WRITE,MAP_SHARED,fb,0);
 #endif

In config I created a new arch:
ARCH = LINUX-AXEL-UL

And in Arch.rules I created a new rule for this, with all the necessary directories and flags to cross-compile. In particular the define for the patch:

ifeq ($(ARCH), LINUX-AXEL-UL)
    ...
    DEFINES += -DPATCH_LINUX_AXEL_UL
    ...
endif

ghaerr · 2021-10-27T20:18:33Z

Hello @LOGUNIVPM,

Thanks for the diff. It seems the only real change made is to force palette mode on an otherwise TRUECOLOR-reported frame buffer. The funny thing is that without that patch in the new version, one would think that the startup would fail with "Unsupported color ..." message. Since yours doesn't, I don't quite understand.

Nonetheless, patching the following into the open code from above would force the palette mode:

 		default:
+#if 1
+			if (psd->bpp == 1) {
+				EPRINTF("WARNING: 1BPP TRUECOLOR, forcing PALETTE mode\n");
+				psd->pixtype = MWPF_PALETTE;
+				break;
+			}
+#endif
 			EPRINTF("Unsupported %d color (%d bpp) truecolor framebuffer\n", psd->ncolors, psd->bpp);

It would be interesting to know the rootpsd->pixtype in the drivers/genmem.c routine where GdCreatePixmap is called from GrNewPixmapEx with a "0" format parameter, which would force the image to use the "root" display, which the above patch sets to palette mode correctly.

I'm not sure the above patch will change anything, but if you'd like to try it, I'd be happy to help debug why the new version isn't working. I'd also want to see the GrCopyArea code that reads the screen and replaces it, so I can better see what the application is doing.

Thank you!

LOGUNIVPM · 2021-10-29T17:27:52Z

Well, actually I made that patch because the program would end on "Unsupported color". With the patch, when psd->bpp == 1 (my case) I avoid failing and set MWPF_PALETTE instead, hopefully that is correct.

In reply to your question: these are the arguments to GdCreatePixmap:
GdCreatePixmap (rootpsd=0xafc90 <scrdev>, width=width@entry=10, height=height@entry=10, format=0, pixels=0x0, palsize=palsize@entry=0)

And this is the pixtype:

(gdb) p rootpsd->pixtype
$1 = 2

ghaerr · 2021-10-30T04:37:03Z

Hmmm, rootpsd->pixtype is 2, which means MWPF_PALETTE. So it sounds like that's not the reason I was guessing was a problem in GdCreatePixMap.

Perhaps we should look at the C routines that do the SRC and DST BEFORE/AFTER COPY. It seems the code is almost correct for displaying the source PIXMAP, but not quite. I can kind of see letters, but there still seems to be an issue, I can't quite tell what the correct characters are supposed to be in the image, that would help.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GrCopyArea all 0s #61

GrCopyArea all 0s #61

LOGUNIVPM commented Oct 22, 2021

LOGUNIVPM commented Oct 22, 2021

ghaerr commented Oct 22, 2021

LOGUNIVPM commented Oct 22, 2021

ghaerr commented Oct 22, 2021

LOGUNIVPM commented Oct 25, 2021

LOGUNIVPM commented Oct 25, 2021

LOGUNIVPM commented Oct 25, 2021

ghaerr commented Oct 25, 2021

LOGUNIVPM commented Oct 27, 2021

ghaerr commented Oct 27, 2021

LOGUNIVPM commented Oct 29, 2021

ghaerr commented Oct 30, 2021

GrCopyArea all 0s #61

GrCopyArea all 0s #61

Comments

LOGUNIVPM commented Oct 22, 2021

LOGUNIVPM commented Oct 22, 2021

ghaerr commented Oct 22, 2021

LOGUNIVPM commented Oct 22, 2021

BACKUP PROCEDURE

RESTORE PROCEDURE

ghaerr commented Oct 22, 2021

LOGUNIVPM commented Oct 25, 2021

LOGUNIVPM commented Oct 25, 2021

BACKUP

RESTORE

LOGUNIVPM commented Oct 25, 2021

ghaerr commented Oct 25, 2021

LOGUNIVPM commented Oct 27, 2021

ghaerr commented Oct 27, 2021

LOGUNIVPM commented Oct 29, 2021

ghaerr commented Oct 30, 2021