diff mbox

[U-Boot,driver,cfb] Make the software cursor non-destructive

Message ID 1320742769-30423-1-git-send-email-gabeblack@chromium.org
State Superseded
Headers show

Commit Message

Gabe Black Nov. 8, 2011, 8:59 a.m. UTC 
When printing the string "\r\n" to the framebuffer console, the first
character of the current line was being replaced with a space. The "boot"
prompt would become the "oot" prompt. This change makes the cursor
non-destructive so that no matter where it goes on its way to where it's
supposed to be, the end result is that the cursor is where it's supposed to
be with the other text preserved intact.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
---
 drivers/video/cfb_console.c |   92 ++++++++++++++++++++++---------------------
 1 files changed, 47 insertions(+), 45 deletions(-)

Comments

Mike Frysinger Nov. 8, 2011, 1:43 p.m. UTC | #1 
On Tuesday 08 November 2011 03:59:29 Gabe Black wrote:
> When printing the string "\r\n" to the framebuffer console, the first
> character of the current line was being replaced with a space. The "boot"
> prompt would become the "oot" prompt. This change makes the cursor
> non-destructive so that no matter where it goes on its way to where it's
> supposed to be, the end result is that the cursor is where it's supposed to
> be with the other text preserved intact.

"preserved intact" ... word for word verbose redundant ? ;)

> --- a/drivers/video/cfb_console.c
> +++ b/drivers/video/cfb_console.c
>
>  /*
>   * Cursor definition:
>   * CONFIG_CONSOLE_CURSOR:  Uses a timer function (see
> drivers/input/i8042.c) 
> - *			   to let the cursor blink. Uses the macros
> - *			   CURSOR_OFF and CURSOR_ON.
> + *			   to let the cursor blink.

is this intentional ?  seems like the existing doc strings are fine ...

> @@ -248,9 +247,9 @@
> 
> -#define CURSOR_ON  console_cursor(1)
> +#define CURSOR_ON console_cursor(1)

unrelated whitespace change

> -#define CURSOR_ON
> -#define CURSOR_OFF video_putchar(console_col * VIDEO_FONT_WIDTH,\
> -				 console_row * VIDEO_FONT_HEIGHT, ' ')
> +void	console_cursor (int state);
> +#define CURSOR_ON console_cursor(1)
> +#define CURSOR_OFF console_cursor(0)
>  #define CURSOR_SET video_set_cursor()

so now the CONFIG_CONSOLE_CURSOR and CONFIG_VIDEO_SW_CURSOR code paths wrt 
CURSOR_* definitions are the same.  seems like they should be unified.

> +static void video_invertchar (int xx, int yy)

no space before that "("

the rest looks fine to me, but let's CC the video maintainer ...
-mike
diff mbox

Patch

diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
index 561883a..06de04e 100644
--- a/drivers/video/cfb_console.c
+++ b/drivers/video/cfb_console.c
@@ -221,8 +221,7 @@ 
 /*
  * Cursor definition:
  * CONFIG_CONSOLE_CURSOR:  Uses a timer function (see drivers/input/i8042.c)
- *			   to let the cursor blink. Uses the macros
- *			   CURSOR_OFF and CURSOR_ON.
+ *			   to let the cursor blink.
  * CONFIG_VIDEO_SW_CURSOR: Draws a cursor after the last character. No
  *			   blinking is provided. Uses the macros CURSOR_SET
  *			   and CURSOR_OFF.
@@ -248,9 +247,9 @@ 
 #endif
 void console_cursor(int state);
 
-#define CURSOR_ON  console_cursor(1)
+#define CURSOR_ON console_cursor(1)
 #define CURSOR_OFF console_cursor(0)
-#define CURSOR_SET
+#define CURSOR_SET video_set_cursor()
 #ifndef CONFIG_I8042_KBD
 #warning Cursor drawing on/off needs timer function s.a. drivers/input/i8042.c
 #endif
@@ -265,9 +264,9 @@  void console_cursor(int state);
 #error	only one of CONFIG_CONSOLE_CURSOR, CONFIG_VIDEO_SW_CURSOR, \
 	or CONFIG_VIDEO_HW_CURSOR can be defined
 #endif
-#define CURSOR_ON
-#define CURSOR_OFF video_putchar(console_col * VIDEO_FONT_WIDTH,\
-				 console_row * VIDEO_FONT_HEIGHT, ' ')
+void	console_cursor (int state);
+#define CURSOR_ON console_cursor(1)
+#define CURSOR_OFF console_cursor(0)
 #define CURSOR_SET video_set_cursor()
 #endif /* CONFIG_VIDEO_SW_CURSOR */
 
@@ -374,6 +373,10 @@  static void *video_console_address;	/* console buffer start address */
 
 static int video_logo_height = VIDEO_LOGO_HEIGHT;
 
+static int cursor_state = 0;
+static int old_col = 0;
+static int old_row = 0;
+
 static int console_col;		/* cursor col */
 static int console_row;		/* cursor row */
 
@@ -433,6 +436,22 @@  static const int video_font_draw_table32[16][4] = {
 };
 
 
+static void video_invertchar (int xx, int yy)
+{
+	int firstx = xx * VIDEO_PIXEL_SIZE;
+	int lastx = (xx + VIDEO_FONT_WIDTH) * VIDEO_PIXEL_SIZE;
+	int firsty = yy * VIDEO_LINE_LEN;
+	int lasty = (yy + VIDEO_FONT_HEIGHT) * VIDEO_LINE_LEN;
+	int x, y;
+	for (y = firsty; y < lasty; y += VIDEO_LINE_LEN) {
+		for (x = firstx; x < lastx; x++) {
+			u8 *dest = (u8 *)(video_fb_address) + x + y;
+			*dest = ~*dest;
+		}
+	}
+}
+
+
 static void video_drawchars(int xx, int yy, unsigned char *s, int count)
 {
 	u8 *cdat, *dest, *dest0;
@@ -608,27 +627,15 @@  static void video_putchar(int xx, int yy, unsigned char c)
 #if defined(CONFIG_CONSOLE_CURSOR) || defined(CONFIG_VIDEO_SW_CURSOR)
 static void video_set_cursor(void)
 {
-	/* swap drawing colors */
-	eorx = fgx;
-	fgx = bgx;
-	bgx = eorx;
-	eorx = fgx ^ bgx;
-	/* draw cursor */
-	video_putchar(console_col * VIDEO_FONT_WIDTH,
-		      console_row * VIDEO_FONT_HEIGHT, ' ');
-	/* restore drawing colors */
-	eorx = fgx;
-	fgx = bgx;
-	bgx = eorx;
-	eorx = fgx ^ bgx;
+	if (cursor_state)
+		console_cursor(0);
+	console_cursor(1);
 }
-#endif
 
-#ifdef CONFIG_CONSOLE_CURSOR
+
+
 void console_cursor(int state)
 {
-	static int last_state = 0;
-
 #ifdef CONFIG_CONSOLE_TIME
 	struct rtc_time tm;
 	char info[16];
@@ -650,17 +657,20 @@  void console_cursor(int state)
 	}
 #endif
 
-	if (state && (last_state != state)) {
-		video_set_cursor();
-	}
-
-	if (!state && (last_state != state)) {
-		/* clear cursor */
-		video_putchar(console_col * VIDEO_FONT_WIDTH,
-			      console_row * VIDEO_FONT_HEIGHT, ' ');
+	if (cursor_state != state) {
+		if (cursor_state) {
+			/* turn off the cursor */
+			video_invertchar(old_col * VIDEO_FONT_WIDTH,
+			                 old_row * VIDEO_FONT_HEIGHT);
+		} else {
+			/* turn off the cursor and record where it is */
+			video_invertchar(console_col * VIDEO_FONT_WIDTH,
+				         console_row * VIDEO_FONT_HEIGHT);
+			old_col = console_col;
+			old_row = console_row;
+		}
+		cursor_state = state;
 	}
-
-	last_state = state;
 }
 #endif
 
@@ -727,19 +737,11 @@  static void console_back(void)
 		if (console_row < 0)
 			console_row = 0;
 	}
-	video_putchar(console_col * VIDEO_FONT_WIDTH,
-		      console_row * VIDEO_FONT_HEIGHT, ' ');
+	CURSOR_SET;
 }
 
 static void console_newline(void)
 {
-	/* Check if last character in the line was just drawn. If so, cursor was
-	   overwriten and need not to be cleared. Cursor clearing without this
-	   check causes overwriting the 1st character of the line if line lenght
-	   is >= CONSOLE_COLS
-	 */
-	if (console_col < CONSOLE_COLS)
-		CURSOR_OFF;
 	console_row++;
 	console_col = 0;
 
@@ -755,7 +757,6 @@  static void console_newline(void)
 
 static void console_cr(void)
 {
-	CURSOR_OFF;
 	console_col = 0;
 }
 
@@ -763,6 +764,8 @@  void video_putc(const char c)
 {
 	static int nl = 1;
 
+	CURSOR_OFF;
+
 	switch (c) {
 	case 13:		/* back to first column */
 		console_cr();
@@ -775,7 +778,6 @@  void video_putc(const char c)
 		break;
 
 	case 9:		/* tab 8 */
-		CURSOR_OFF;
 		console_col |= 0x0008;
 		console_col &= ~0x0007;