[fbcon] Centre background picture on console

Centre the background picture on the console, to give a more
consistent result when the aspect ratio does not match the requested
width and height.

Once drawn for the first time, nothing should ever overwrite the
margins of the display.  We can therefore eliminate the logic used to
redraw only the margin areas, and use much simpler code to draw the
complete initial background image.

Simplify the redrawing logic further by making the background picture
buffer equal in size to the frame buffer.  In the common case of a
background picture which is designed to fill the screen, this wastes
no extra memory, and the combined code simplifications reduce the size
of fbcon.o by approximately 15%.

Redefine the concept of "margin" to match the intuitive definition
(i.e. the size of the gap, rather than the position of the boundary
line).

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/core/fbcon.c

@@ -98,6 +98,32 @@ static void fbcon_set_default_background ( struct fbcon *fbcon ) {
 	fbcon->background = FBCON_TRANSPARENT;
 }
 
+/**
+ * Clear rows of characters
+ *
+ * @v fbcon		Frame buffer console
+ * @v ypos		Starting Y position
+ */
+static void fbcon_clear ( struct fbcon *fbcon, unsigned int ypos ) {
+	struct fbcon_text_cell cell = {
+		.foreground = fbcon->foreground,
+		.background = fbcon->background,
+		.character = ' ',
+	};
+	size_t offset;
+	unsigned int xpos;
+
+	/* Clear stored character array */
+	for ( ; ypos < fbcon->character.height ; ypos++ ) {
+		offset = ( ypos * fbcon->character.width * sizeof ( cell ) );
+		for ( xpos = 0 ; xpos < fbcon->character.width ; xpos++ ) {
+			copy_to_user ( fbcon->text.start, offset, &cell,
+				       sizeof ( cell ) );
+			offset += sizeof ( cell );
+		}
+	}
+}
+
 /**
  * Store character at specified position
  *
@@ -106,9 +132,8 @@ static void fbcon_set_default_background ( struct fbcon *fbcon ) {
  * @v xpos		X position
  * @v ypos		Y position
  */
-static void fbcon_store_character ( struct fbcon *fbcon,
-				    struct fbcon_text_cell *cell,
-				    unsigned int xpos, unsigned int ypos ) {
+static void fbcon_store ( struct fbcon *fbcon, struct fbcon_text_cell *cell,
+			  unsigned int xpos, unsigned int ypos ) {
 	size_t offset;
 
 	/* Store cell */
@@ -125,14 +150,9 @@ static void fbcon_store_character ( struct fbcon *fbcon,
  * @v xpos		X position
  * @v ypos		Y position
  */
-static void fbcon_draw_character ( struct fbcon *fbcon,
-				   struct fbcon_text_cell *cell,
-				   unsigned int xpos, unsigned int ypos ) {
-	static uint32_t black[FBCON_CHAR_WIDTH];
+static void fbcon_draw ( struct fbcon *fbcon, struct fbcon_text_cell *cell,
+			 unsigned int xpos, unsigned int ypos ) {
 	struct fbcon_font_glyph glyph;
-	userptr_t picture_start;
-	size_t picture_offset;
-	size_t picture_stride;
 	size_t offset;
 	size_t pixel_len;
 	size_t skip_len;
@@ -154,30 +174,23 @@ static void fbcon_draw_character ( struct fbcon *fbcon,
 	pixel_len = fbcon->pixel->len;
 	skip_len = ( fbcon->pixel->stride - fbcon->character.len );
 
-	/* Calculate background picture geometry */
-	if ( ( xpos < fbcon->picture.character.width ) &&
-	     ( ypos < fbcon->picture.character.height ) ) {
-		picture_start = fbcon->picture.start;
-		picture_offset = ( fbcon->picture.indent +
-				   ( ypos * fbcon->picture.character.stride ) +
-				   ( xpos * fbcon->picture.character.len ) );
-		picture_stride = fbcon->picture.pixel.stride;
-	} else {
-		picture_start = virt_to_user ( black );
-		picture_offset = 0;
-		picture_stride = 0;
-	}
-	assert ( fbcon->character.len <= sizeof ( black ) );
-
 	/* Check for transparent background colour */
 	transparent = ( cell->background == FBCON_TRANSPARENT );
 
 	/* Draw character rows */
 	for ( row = 0 ; row < FBCON_CHAR_HEIGHT ; row++ ) {
 
-		/* Draw background picture */
-		memcpy_user ( fbcon->start, offset, picture_start,
-			      picture_offset, fbcon->character.len );
+		/* Draw background picture, if applicable */
+		if ( transparent ) {
+			if ( fbcon->picture.start ) {
+				memcpy_user ( fbcon->start, offset,
+					      fbcon->picture.start, offset,
+					      fbcon->character.len );
+			} else {
+				memset_user ( fbcon->start, offset, 0,
+					      fbcon->character.len );
+			}
+		}
 
 		/* Draw character row */
 		for ( column = FBCON_CHAR_WIDTH, bitmask = glyph.bitmask[row] ;
@@ -194,63 +207,15 @@ static void fbcon_draw_character ( struct fbcon *fbcon,
 
 		/* Move to next row */
 		offset += skip_len;
-		picture_offset += picture_stride;
 	}
 }
 
 /**
- * Redraw margins
+ * Redraw all characters
  *
  * @v fbcon		Frame buffer console
  */
-static void fbcon_redraw_margins ( struct fbcon *fbcon ) {
-	struct fbcon_picture *picture = &fbcon->picture;
-	size_t pixel_len = fbcon->pixel->len;
-	size_t offset = 0;
-	size_t picture_offset = 0;
-	size_t row_len;
-	size_t left_len;
-	size_t right_len;
-	size_t right_offset;
-	unsigned int y;
-
-	/* Calculate margin parameters */
-	row_len = ( picture->pixel.width * pixel_len );
-	left_len = ( picture->margin.left * pixel_len );
-	right_offset = ( picture->margin.right * pixel_len );
-	right_len = ( ( picture->pixel.width - picture->margin.right ) *
-		      pixel_len );
-
-	/* Redraw margins */
-	for ( y = 0 ; y < picture->pixel.height ; y++ ) {
-		if ( ( y < picture->margin.top ) ||
-		     ( y >= picture->margin.bottom ) ) {
-
-			/* Within top or bottom margin: draw whole row */
-			memcpy_user ( fbcon->start, offset, picture->start,
-				      picture_offset, row_len );
-
-		} else {
-
-			/* Otherwise, draw left and right margins */
-			memcpy_user ( fbcon->start, offset, picture->start,
-				      picture_offset, left_len );
-			memcpy_user ( fbcon->start, ( offset + right_offset ),
-				      picture->start,
-				      ( picture_offset + right_offset ),
-				      right_len );
-		}
-		offset += fbcon->pixel->stride;
-		picture_offset += picture->pixel.stride;
-	}
-}
-
-/**
- * Redraw characters
- *
- * @v fbcon		Frame buffer console
- */
-static void fbcon_redraw_characters ( struct fbcon *fbcon ) {
+static void fbcon_redraw ( struct fbcon *fbcon ) {
 	struct fbcon_text_cell cell;
 	size_t offset = 0;
 	unsigned int xpos;
@@ -261,55 +226,12 @@ static void fbcon_redraw_characters ( struct fbcon *fbcon ) {
 		for ( xpos = 0 ; xpos < fbcon->character.width ; xpos++ ) {
 			copy_from_user ( &cell, fbcon->text.start, offset,
 					 sizeof ( cell ) );
-			fbcon_draw_character ( fbcon, &cell, xpos, ypos );
+			fbcon_draw ( fbcon, &cell, xpos, ypos );
 			offset += sizeof ( cell );
 		}
 	}
 }
 
-/**
- * Redraw screen
- *
- * @v fbcon		Frame buffer console
- */
-static void fbcon_redraw ( struct fbcon *fbcon ) {
-
-	/* Redraw margins */
-	fbcon_redraw_margins ( fbcon );
-
-	/* Redraw characters */
-	fbcon_redraw_characters ( fbcon );
-}
-
-/**
- * Clear portion of screen
- *
- * @v fbcon		Frame buffer console
- * @v ypos		Starting Y position
- */
-static void fbcon_clear ( struct fbcon *fbcon, unsigned int ypos ) {
-	struct fbcon_text_cell cell = {
-		.foreground = fbcon->foreground,
-		.background = fbcon->background,
-		.character = ' ',
-	};
-	size_t offset;
-	unsigned int xpos;
-
-	/* Clear stored character array */
-	for ( ; ypos < fbcon->character.height ; ypos++ ) {
-		offset = ( ypos * fbcon->character.width * sizeof ( cell ) );
-		for ( xpos = 0 ; xpos < fbcon->character.width ; xpos++ ) {
-			copy_to_user ( fbcon->text.start, offset, &cell,
-				       sizeof ( cell ) );
-			offset += sizeof ( cell );
-		}
-	}
-
-	/* Redraw screen */
-	fbcon_redraw ( fbcon );
-}
-
 /**
  * Scroll screen
  *
@@ -329,6 +251,9 @@ static void fbcon_scroll ( struct fbcon *fbcon ) {
 
 	/* Update cursor position */
 	fbcon->ypos--;
+
+	/* Redraw all characters */
+	fbcon_redraw ( fbcon );
 }
 
 /**
@@ -349,7 +274,7 @@ static void fbcon_draw_cursor ( struct fbcon *fbcon, int show_cursor ) {
 		cell.foreground = ( ( fbcon->background == FBCON_TRANSPARENT ) ?
 				    0 : fbcon->background );
 	}
-	fbcon_draw_character ( fbcon, &cell, fbcon->xpos, fbcon->ypos );
+	fbcon_draw ( fbcon, &cell, fbcon->xpos, fbcon->ypos );
 }
 
 /**
@@ -391,9 +316,12 @@ static void fbcon_handle_ed ( struct ansiesc_context *ctx,
 	/* We assume that we always clear the whole screen */
 	assert ( params[0] == ANSIESC_ED_ALL );
 
-	/* Clear screen */
+	/* Clear character array */
 	fbcon_clear ( fbcon, 0 );
 
+	/* Redraw all characters */
+	fbcon_redraw ( fbcon );
+
 	/* Reset cursor position */
 	fbcon->xpos = 0;
 	fbcon->ypos = 0;
@@ -541,8 +469,8 @@ void fbcon_putchar ( struct fbcon *fbcon, int character ) {
 		cell.foreground = ( fbcon->foreground | fbcon->bold );
 		cell.background = fbcon->background;
 		cell.character = character;
-		fbcon_store_character ( fbcon, &cell, fbcon->xpos, fbcon->ypos);
-		fbcon_draw_character ( fbcon, &cell, fbcon->xpos, fbcon->ypos );
+		fbcon_store ( fbcon, &cell, fbcon->xpos, fbcon->ypos );
+		fbcon_draw ( fbcon, &cell, fbcon->xpos, fbcon->ypos );
 
 		/* Advance cursor */
 		fbcon->xpos++;
@@ -561,72 +489,6 @@ void fbcon_putchar ( struct fbcon *fbcon, int character ) {
 	fbcon_draw_cursor ( fbcon, fbcon->show_cursor );
 }
 
-/**
- * Calculate character geometry from pixel geometry
- *
- * @v pixel		Pixel geometry
- * @v character		Character geometry to fill in
- */
-static void fbcon_char_geometry ( const struct fbcon_geometry *pixel,
-				  struct fbcon_geometry *character ) {
-
-	character->width = ( pixel->width / FBCON_CHAR_WIDTH );
-	character->height = ( pixel->height / FBCON_CHAR_HEIGHT );
-	character->len = ( pixel->len * FBCON_CHAR_WIDTH );
-	character->stride = ( pixel->stride * FBCON_CHAR_HEIGHT );
-}
-
-/**
- * Calculate margins from pixel geometry
- *
- * @v pixel		Pixel geometry
- * @v margin		Margins to fill in
- */
-static void fbcon_margin ( const struct fbcon_geometry *pixel,
-			   struct fbcon_margin *margin ) {
-	unsigned int xgap;
-	unsigned int ygap;
-
-	xgap = ( pixel->width % FBCON_CHAR_WIDTH );
-	ygap = ( pixel->height % FBCON_CHAR_HEIGHT );
-	margin->left = ( xgap / 2 );
-	margin->top = ( ygap / 2 );
-	margin->right = ( pixel->width - ( xgap - margin->left ) );
-	margin->bottom = ( pixel->height - ( ygap - margin->top ) );
-}
-
-/**
- * Align to first indented boundary
- *
- * @v value		Original value
- * @v blocksize		Block size
- * @v indent		Indent
- * @v max		Maximum allowed value
- * @ret value		Aligned value
- */
-static unsigned int fbcon_align ( unsigned int value, unsigned int blocksize,
-				  unsigned int indent, unsigned int max ) {
-	unsigned int overhang;
-
-	/* Special case: 0 is always a boundary regardless of the indent */
-	if ( value == 0 )
-		return value;
-
-	/* Special case: first boundary is the indent */
-	if ( value < indent )
-		return indent;
-
-	/* Round up to next indented boundary */
-	overhang = ( ( value - indent ) % blocksize );
-	value = ( value + ( ( blocksize - overhang ) % blocksize ) );
-
-	/* Limit to maximum value */
-	if ( value > max )
-		value = max;
-
-	return value;
-}
-
 /**
  * Initialise background picture
  *
@@ -636,52 +498,26 @@ static unsigned int fbcon_align ( unsigned int value, unsigned int blocksize,
  */
 static int fbcon_picture_init ( struct fbcon *fbcon,
 				struct pixel_buffer *pixbuf ) {
+	struct fbcon_geometry *pixel = fbcon->pixel;
 	struct fbcon_picture *picture = &fbcon->picture;
-	size_t pixel_len = fbcon->pixel->len;
 	size_t len;
+	size_t pixbuf_stride;
+	size_t indent;
+	size_t pixbuf_indent;
 	size_t offset;
 	size_t pixbuf_offset;
 	uint32_t rgb;
 	uint32_t raw;
 	unsigned int x;
 	unsigned int y;
+	unsigned int width;
+	unsigned int height;
+	int xgap;
+	int ygap;
 	int rc;
 
-	/* Calculate pixel geometry */
-	picture->pixel.width = fbcon_align ( pixbuf->width, FBCON_CHAR_WIDTH,
-					     fbcon->margin.left,
-					     fbcon->pixel->width );
-	picture->pixel.height = fbcon_align ( pixbuf->height, FBCON_CHAR_HEIGHT,
-					      fbcon->margin.top,
-					      fbcon->pixel->height );
-	picture->pixel.len = pixel_len;
-	picture->pixel.stride = ( picture->pixel.width * picture->pixel.len );
-
-	/* Calculate character geometry */
-	fbcon_char_geometry ( &picture->pixel, &picture->character );
-
-	/* Calculate margins */
-	memcpy ( &picture->margin, &fbcon->margin, sizeof ( picture->margin ) );
-	if ( picture->margin.left > picture->pixel.width )
-		picture->margin.left = picture->pixel.width;
-	if ( picture->margin.top > picture->pixel.height )
-		picture->margin.top = picture->pixel.height;
-	if ( picture->margin.right > picture->pixel.width )
-		picture->margin.right = picture->pixel.width;
-	if ( picture->margin.bottom > picture->pixel.height )
-		picture->margin.bottom = picture->pixel.height;
-	picture->indent = ( ( picture->margin.top * picture->pixel.stride ) +
-			    ( picture->margin.left * picture->pixel.len ) );
-	DBGC ( fbcon, "FBCON %p picture is pixel %dx%d, char %dx%d at "
-	       "[%d-%d),[%d-%d)\n", fbcon, picture->pixel.width,
-	       picture->pixel.height, picture->character.width,
-	       picture->character.height, picture->margin.left,
-	       picture->margin.right, picture->margin.top,
-	       picture->margin.bottom );
-
 	/* Allocate buffer */
-	len = ( picture->pixel.width * picture->pixel.height *
-		picture->pixel.len );
+	len = ( pixel->height * pixel->stride );
 	picture->start = umalloc ( len );
 	if ( ! picture->start ) {
 		DBGC ( fbcon, "FBCON %p could not allocate %zd bytes for "
@@ -690,20 +526,36 @@ static int fbcon_picture_init ( struct fbcon *fbcon,
 		goto err_umalloc;
 	}
 
+	/* Centre picture on console */
+	pixbuf_stride = ( pixbuf->width * sizeof ( rgb ) );
+	xgap = ( ( ( int ) ( pixel->width - pixbuf->width ) ) / 2 );
+	ygap = ( ( ( int ) ( pixel->height - pixbuf->height ) ) / 2 );
+	indent = ( ( ( ( ygap >= 0 ) ? ygap : 0 ) * pixel->stride ) +
+		   ( ( ( xgap >= 0 ) ? xgap : 0 ) * pixel->len ) );
+	pixbuf_indent =	( ( ( ( ygap < 0 ) ? -ygap : 0 ) * pixbuf_stride ) +
+			  ( ( ( xgap < 0 ) ? -xgap : 0 ) * sizeof ( rgb ) ) );
+	width = pixbuf->width;
+	if ( width > pixel->width )
+		width = pixel->width;
+	height = pixbuf->height;
+	if ( height > pixel->height )
+		height = pixel->height;
+	DBGC ( fbcon, "FBCON %p picture is pixel %dx%d at [%d,%d),[%d,%d)\n",
+	       fbcon, width, height, xgap, ( xgap + pixbuf->width ), ygap,
+	       ( ygap + pixbuf->height ) );
+
 	/* Convert to frame buffer raw format */
 	memset_user ( picture->start, 0, 0, len );
-	pixbuf_offset = 0;
-	for ( y = 0 ; ( y < pixbuf->height ) &&
-		      ( y < picture->pixel.height ) ; y++ ) {
-		offset = ( y * picture->pixel.stride );
-		pixbuf_offset = ( y * pixbuf->width * sizeof ( rgb ) );
-		for ( x = 0 ; ( x < pixbuf->width ) &&
-			      ( x < picture->pixel.width ) ; x++ ) {
+	for ( y = 0 ; y < height ; y++ ) {
+		offset = ( indent + ( y * pixel->stride ) );
+		pixbuf_offset = ( pixbuf_indent + ( y * pixbuf_stride ) );
+		for ( x = 0 ; x < width ; x++ ) {
 			copy_from_user ( &rgb, pixbuf->data, pixbuf_offset,
 					 sizeof ( rgb ) );
 			raw = fbcon_colour ( fbcon, rgb );
-			copy_to_user ( picture->start, offset, &raw, pixel_len);
-			offset += pixel_len;
+			copy_to_user ( picture->start, offset, &raw,
+				       pixel->len );
+			offset += pixel->len;
 			pixbuf_offset += sizeof ( rgb );
 		}
 	}
@@ -731,6 +583,8 @@ int fbcon_init ( struct fbcon *fbcon, userptr_t start,
 		 struct fbcon_colour_map *map,
 		 struct fbcon_font *font,
 		 struct pixel_buffer *pixbuf ) {
+	unsigned int xgap;
+	unsigned int ygap;
 	int rc;
 
 	/* Initialise data structure */
@@ -750,21 +604,33 @@ int fbcon_init ( struct fbcon *fbcon, userptr_t start,
 	       user_to_phys ( fbcon->start, fbcon->len ) );
 
 	/* Derive character geometry from pixel geometry */
-	fbcon_char_geometry ( pixel, &fbcon->character );
-	fbcon_margin ( pixel, &fbcon->margin );
+	fbcon->character.width = ( pixel->width / FBCON_CHAR_WIDTH );
+	fbcon->character.height = ( pixel->height / FBCON_CHAR_HEIGHT );
+	fbcon->character.len = ( pixel->len * FBCON_CHAR_WIDTH );
+	fbcon->character.stride = ( pixel->stride * FBCON_CHAR_HEIGHT );
+
+	/* Calculate margin */
+	xgap = ( pixel->width % FBCON_CHAR_WIDTH );
+	ygap = ( pixel->height % FBCON_CHAR_HEIGHT );
+	fbcon->margin.left = ( xgap / 2 );
+	fbcon->margin.top = ( ygap / 2 );
+	fbcon->margin.right = ( xgap - fbcon->margin.left );
+	fbcon->margin.bottom = ( ygap - fbcon->margin.top );
 	fbcon->indent = ( ( fbcon->margin.top * pixel->stride ) +
 			  ( fbcon->margin.left * pixel->len ) );
 	DBGC ( fbcon, "FBCON %p is pixel %dx%d, char %dx%d at "
 	       "[%d-%d),[%d-%d)\n", fbcon, fbcon->pixel->width,
 	       fbcon->pixel->height, fbcon->character.width,
-	       fbcon->character.height, fbcon->margin.left, fbcon->margin.right,
-	       fbcon->margin.top, fbcon->margin.bottom );
+	       fbcon->character.height, fbcon->margin.left,
+	       ( fbcon->pixel->width - fbcon->margin.right ),
+	       fbcon->margin.top,
+	       ( fbcon->pixel->height - fbcon->margin.bottom ) );
 
 	/* Set default colours */
 	fbcon_set_default_foreground ( fbcon );
 	fbcon_set_default_background ( fbcon );
 
-	/* Allocate stored character array */
+	/* Allocate and initialise stored character array */
 	fbcon->text.start = umalloc ( fbcon->character.width *
 				      fbcon->character.height *
 				      sizeof ( struct fbcon_text_cell ) );
@@ -772,13 +638,20 @@ int fbcon_init ( struct fbcon *fbcon, userptr_t start,
 		rc = -ENOMEM;
 		goto err_text;
 	}
+	fbcon_clear ( fbcon, 0 );
+
+	/* Set framebuffer to all black (including margins) */
+	memset_user ( fbcon->start, 0, 0, fbcon->len );
 
 	/* Generate pixel buffer from background image, if applicable */
 	if ( pixbuf && ( ( rc = fbcon_picture_init ( fbcon, pixbuf ) ) != 0 ) )
 		goto err_picture;
 
-	/* Clear screen */
-	fbcon_clear ( fbcon, 0 );
+	/* Draw background picture (including margins), if applicable */
+	if ( fbcon->picture.start ) {
+		memcpy_user ( fbcon->start, 0, fbcon->picture.start, 0,
+			      fbcon->len );
+	}
 
 	/* Update console width and height */
 	console_set_size ( fbcon->character.width, fbcon->character.height );

src/include/ipxe/fbcon.h

@@ -103,14 +103,6 @@ struct fbcon_text {
 struct fbcon_picture {
 	/** Start address */
 	userptr_t start;
-	/** Pixel geometry */
-	struct fbcon_geometry pixel;
-	/** Character geometry */
-	struct fbcon_geometry character;
-	/** Margin */
-	struct fbcon_margin margin;
-	/** Indent to first character (in bytes) */
-	size_t indent;
 };
 
 /** A frame buffer console */