public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
       [not found] <cover.1627356917.git.cadenkline9@gmail.com>
@ 2021-07-30  2:45 ` Caden Kline
  2021-08-03  0:04   ` Wu, Hao A
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Caden Kline @ 2021-07-30  2:45 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Hao A Wu, Zhichao Gao, Ray Ni

Improved encoding of box drawing characters for different terminal types.
This includes Dec special graphics mode and more utf8.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Caden Kline <cadenkline9@gmail.com>
---
 .../Universal/Console/TerminalDxe/Terminal.h  |  24 +-
 .../Universal/Console/TerminalDxe/Ansi.c      |   2 +-
 .../Console/TerminalDxe/TerminalConOut.c      | 322 ++++++++++++++----
 3 files changed, 269 insertions(+), 79 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 360e58e84743..83c3ea94a042 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -122,7 +122,10 @@ typedef struct {
   EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;
   LIST_ENTRY                          NotifyList;
   EFI_EVENT                           KeyNotifyProcessEvent;
+  BOOLEAN                             DecSpecialGraphicsMode;
 } TERMINAL_DEV;
+// This the lenth the escape squences for entering and exiting Dec Special Graphics Mode
+#define LENGTH_DEC_ESCAPE                 0x03
 
 #define INPUT_STATE_DEFAULT               0x00
 #define INPUT_STATE_ESC                   0x01
@@ -169,6 +172,7 @@ typedef struct {
   UINT16  Unicode;
   CHAR8   PcAnsi;
   CHAR8   Ascii;
+  CHAR8   DecSpecialGraphics;
 } UNICODE_TO_CHAR;
 
 //
@@ -1367,20 +1371,22 @@ Utf8ToUnicode (
 /**
   Detects if a Unicode char is for Box Drawing text graphics.
 
-  @param  Graphic      Unicode char to test.
-  @param  PcAnsi       Optional pointer to return PCANSI equivalent of
-                       Graphic.
-  @param  Ascii        Optional pointer to return ASCII equivalent of
-                       Graphic.
-
-  @retval TRUE         If Graphic is a supported Unicode Box Drawing character.
+  @param  Graphic             Unicode char to test.
+  @param  PcAnsi              Optional pointer to return PCANSI equivalent of
+                              Graphic.
+  @param  Ascii               Optional pointer to return ASCII equivalent of
+                              Graphic.
+  @param  DecSpecialGraphics  Optional pointer to return Dec Special Graphics equivalent of
+                              Graphic.
+  @retval TRUE                If Graphic is a supported Unicode Box Drawing character.
 
 **/
 BOOLEAN
 TerminalIsValidTextGraphics (
   IN  CHAR16  Graphic,
-  OUT CHAR8   *PcAnsi, OPTIONAL
-  OUT CHAR8   *Ascii OPTIONAL
+  OUT CHAR8   *PcAnsi,            OPTIONAL
+  OUT CHAR8   *Ascii,             OPTIONAL
+  OUT CHAR8   *DecSpecialGraphics OPTIONAL
   );
 
 /**
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
index f117d90b9de3..5ae5a4f0212e 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
@@ -63,7 +63,7 @@ AnsiTestString (
 
     if ( !(TerminalIsValidAscii (*WString) ||
         TerminalIsValidEfiCntlChar (*WString) ||
-        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL) )) {
+        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL, NULL) )) {
 
       return EFI_UNSUPPORTED;
     }
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index aae470e9562c..1c22ed426715 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -16,61 +16,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 //
 UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {
-  { BOXDRAW_HORIZONTAL,                 0xc4, L'-' },
-  { BOXDRAW_VERTICAL,                   0xb3, L'|' },
-  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/' },
-  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\' },
-  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\' },
-  { BOXDRAW_UP_LEFT,                    0xd9, L'/' },
-  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|' },
-  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|' },
-  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+' },
-  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+' },
-  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+' },
-  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-' },
-  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|' },
-  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/' },
-  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/' },
-  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/' },
-  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\' },
-  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\' },
-  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\' },
-  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\' },
-  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\' },
-  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\' },
-  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/' },
-  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/' },
-  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/' },
-  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|' },
-  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|' },
-  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|' },
-  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|' },
-  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|' },
-  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|' },
-  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+' },
-  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+' },
-  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+' },
-  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+' },
-  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+' },
-  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+' },
-  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+' },
-  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+' },
-  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+' },
+  { BOXDRAW_HORIZONTAL,                 0xc4, L'-',  0x71 },
+  { BOXDRAW_VERTICAL,                   0xb3, L'|',  0x78 },
+  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/',  0x6c },
+  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\', 0x6b },
+  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\', 0x6d },
+  { BOXDRAW_UP_LEFT,                    0xd9, L'/',  0x6a },
+  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|',  0x74 },
+  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|',  0x75 },
+  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+',  0x77 },
+  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+',  0x76 },
+  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+',  0x6e },
+  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-',  0x71 },
+  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|',  0x78 },
+  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/',  0x6c },
+  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/',  0x6c },
+  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/',  0x6c },
+  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\', 0x6b },
+  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\', 0x6b },
+  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\', 0x6b },
+  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\', 0x6d },
+  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\', 0x6d },
+  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\', 0x6d },
+  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/',  0x6a },
+  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/',  0x6a },
+  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/',  0x6a },
+  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|',  0x74 },
+  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|',  0x74 },
+  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|',  0x74 },
+  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|',  0x75 },
+  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|',  0x75 },
+  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|',  0x75 },
+  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+',  0x77 },
+  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+',  0x77 },
+  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+',  0x77 },
+  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+',  0x76 },
+  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+',  0x76 },
+  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+',  0x76 },
+  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+',  0x6e },
+  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+',  0x6e },
+  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+',  0x6e },
 
-  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*' },
-  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+' },
+  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*',  0x61 },
+  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+',  0x61 },
 
-  { GEOMETRICSHAPE_UP_TRIANGLE,         '^', L'^' },
-  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>', L'>' },
-  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v', L'v' },
-  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<', L'<' },
+  { GEOMETRICSHAPE_UP_TRIANGLE,         '^',  L'^',  L'^' },
+  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>',  L'>',  L'>' },
+  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v',  L'v',  L'v' },
+  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<',  L'<',  L'<' },
 
-  { ARROW_LEFT,                         '<', L'<' },
-  { ARROW_UP,                           '^', L'^' },
-  { ARROW_RIGHT,                        '>', L'>' },
-  { ARROW_DOWN,                         'v', L'v' },
-
-  { 0x0000,                             0x00, L'\0' }
+  { ARROW_LEFT,                         '<',  L'<',  L'<' },
+  { ARROW_UP,                           '^',  L'^',  L'^' },
+  { ARROW_RIGHT,                        '>',  L'>',  L'>' },
+  { ARROW_DOWN,                         'v',  L'v',  L'v' },
 };
 
 CHAR16 mSetModeString[]            = { ESC, '[', '=', '3', 'h', 0 };
@@ -80,6 +78,8 @@ CHAR16 mSetCursorPositionString[]  = { ESC, '[', '0', '0', ';', '0', '0', 'H', 0
 CHAR16 mCursorForwardString[]      = { ESC, '[', '0', '0', 'C', 0 };
 CHAR16 mCursorBackwardString[]     = { ESC, '[', '0', '0', 'D', 0 };
 
+CHAR8  SetDecModeString[]          = {ESC, 0x28, 0x30};
+CHAR8  ExitDecModeString[]         = {ESC, 0x28, 0x42};
 //
 // Body of the ConOut functions
 //
@@ -183,16 +183,19 @@ TerminalConOutOutputString (
   EFI_STATUS                  Status;
   UINT8                       ValidBytes;
   CHAR8                       CrLfStr[2];
+  CHAR8                       DecChar;
+  UINTN                       ModeSwitchLength;
   //
   //  flag used to indicate whether condition happens which will cause
   //  return EFI_WARN_UNKNOWN_GLYPH
   //
   BOOLEAN                     Warning;
 
-  ValidBytes  = 0;
-  Warning     = FALSE;
-  AsciiChar   = 0;
-
+  ValidBytes       = 0;
+  Warning          = FALSE;
+  AsciiChar        = 0;
+  DecChar          = 0;
+  ModeSwitchLength = LENGTH_DEC_ESCAPE;
   //
   //  get Terminal device data structure pointer.
   //
@@ -217,17 +220,136 @@ TerminalConOutOutputString (
   for (; *WString != CHAR_NULL; WString++) {
 
     switch (TerminalDevice->TerminalType) {
-
     case TerminalTypePcAnsi:
-    case TerminalTypeVt100:
-    case TerminalTypeVt100Plus:
-    case TerminalTypeTtyTerm:
-    case TerminalTypeLinux:
+      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar, NULL)) {
+        //
+        // If it's not a graphic character convert Unicode to ASCII.
+        //
+          GraphicChar = (CHAR8)*WString;
+
+        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar (GraphicChar))) {
+          //
+          // when this driver use the OutputString to output control string,
+          // TerminalDevice->OutputEscChar is set to let the Esc char
+          // to be output to the terminal emulation software.
+          //
+          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
+            GraphicChar = 27;
+          } else {
+            GraphicChar = '?';
+            Warning     = TRUE;
+          }
+        }
+
+        AsciiChar = GraphicChar;
+      }
+      Length = 1;
+      Status = TerminalDevice->SerialIo->Write (
+                                          TerminalDevice->SerialIo,
+                                          &Length,
+                                          &GraphicChar
+                                          );
+
+      if (EFI_ERROR (Status)) {
+        goto OutputError;
+      }
+
+      break;
     case TerminalTypeXtermR6:
-    case TerminalTypeVt400:
     case TerminalTypeSCO:
+        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar) && !TerminalIsValidAscii (*WString)) {
+          //Box graphics are split into 2 types simple and advanced
+          //simple are drawn with dec special graphics
+          //advanced are drawn with utf8
+          //This checks for simple because they have a lower value than the advanced
+          if(*WString < BOXDRAW_DOUBLE_HORIZONTAL) {
+            if (!TerminalDevice->DecSpecialGraphicsMode) {
+              ValidBytes = 0;
+              ModeSwitchLength = LENGTH_DEC_ESCAPE;
+              Status = TerminalDevice->SerialIo->Write (
+                                                   TerminalDevice->SerialIo,
+                                                   &ModeSwitchLength,
+                                                   (UINT8 *)SetDecModeString
+                                          );
+          TerminalDevice->DecSpecialGraphicsMode = TRUE;
+        }
 
-      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
+        GraphicChar = DecChar;
+        Length = 1;
+          } else {
+            if (TerminalDevice->DecSpecialGraphicsMode) {
+              ModeSwitchLength = LENGTH_DEC_ESCAPE;
+              Status = TerminalDevice->SerialIo->Write (
+                                                   TerminalDevice->SerialIo,
+                                                   &ModeSwitchLength,
+                                                   (UINT8 *)ExitDecModeString
+                                                );
+                if (EFI_ERROR (Status)) {
+                  goto OutputError;
+                }
+
+            TerminalDevice->DecSpecialGraphicsMode = FALSE;
+          }
+
+          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
+          Length = ValidBytes;
+        }
+        } else {
+          if (TerminalDevice->DecSpecialGraphicsMode) {
+          Status = TerminalDevice->SerialIo->Write (
+                                          TerminalDevice->SerialIo,
+                                          &ModeSwitchLength,
+                                          (UINT8 *)ExitDecModeString
+                                          );
+          if (EFI_ERROR (Status)) {
+            goto OutputError;
+          }
+
+          TerminalDevice->DecSpecialGraphicsMode = FALSE;
+        }
+
+          GraphicChar = (CHAR8)*WString;
+
+        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar (GraphicChar))) {
+          //
+          // when this driver use the OutputString to output control string,
+          // TerminalDevice->OutputEscChar is set to let the Esc char
+          // to be output to the terminal emulation software.
+          //
+            if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
+              GraphicChar = 27;
+              Length = 1;
+            } else {
+          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
+          Length = ValidBytes;
+            }
+          } else {
+        Length = 1;
+      }
+      }
+
+        if (ValidBytes) {
+        Status = TerminalDevice->SerialIo->Write (
+                                        TerminalDevice->SerialIo,
+                                        &Length,
+                                               (UINT8 *)&Utf8Char
+                                        );
+          ValidBytes = 0;
+        } else {
+      Status = TerminalDevice->SerialIo->Write (
+                                      TerminalDevice->SerialIo,
+                                      &Length,
+                                      &GraphicChar
+                                      );
+        }
+      if (EFI_ERROR (Status)) {
+        goto OutputError;
+      }
+
+      break;
+    case TerminalTypeVt100:
+    case TerminalTypeTtyTerm:
+      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar, NULL)) {
         //
         // If it's not a graphic character convert Unicode to ASCII.
         //
@@ -248,12 +370,9 @@ TerminalConOutOutputString (
         }
 
         AsciiChar = GraphicChar;
-
       }
 
-      if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {
-        GraphicChar = AsciiChar;
-      }
+      GraphicChar = AsciiChar;
 
       Length = 1;
 
@@ -267,8 +386,67 @@ TerminalConOutOutputString (
         goto OutputError;
       }
 
-      break;
+        break;
+    case TerminalTypeVt100Plus:
+    case TerminalTypeVt400:
+        Length = 1;
+        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)) {
+          if (!TerminalDevice->DecSpecialGraphicsMode) {
+            ModeSwitchLength = LENGTH_DEC_ESCAPE;
+            Status = TerminalDevice->SerialIo->Write (
+                                                 TerminalDevice->SerialIo,
+                                                 &ModeSwitchLength,
+                                                 (UINT8 *)SetDecModeString
+                                          );
+          TerminalDevice->DecSpecialGraphicsMode = TRUE;
+        }
 
+        GraphicChar = DecChar;
+        } else {
+          if (TerminalDevice->DecSpecialGraphicsMode) {
+            ModeSwitchLength = LENGTH_DEC_ESCAPE;
+            Status = TerminalDevice->SerialIo->Write (
+                                                 TerminalDevice->SerialIo,
+                                                 &ModeSwitchLength,
+                                                 (UINT8 *)ExitDecModeString
+                                          );
+
+          if (EFI_ERROR (Status)) {
+            goto OutputError;
+          }
+
+          TerminalDevice->DecSpecialGraphicsMode = FALSE;
+        }
+
+          GraphicChar = (CHAR8)*WString;
+
+        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar (GraphicChar))) {
+          //
+          // when this driver use the OutputString to output control string,
+          // TerminalDevice->OutputEscChar is set to let the Esc char
+          // to be output to the terminal emulation software.
+          //
+          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
+            GraphicChar = 27;
+          } else {
+            GraphicChar = '?';
+            Warning     = TRUE;
+          }
+        }
+      }
+
+    Status = TerminalDevice->SerialIo->Write (
+                                      TerminalDevice->SerialIo,
+                                      &Length,
+                                      &GraphicChar
+                                      );
+
+      if (EFI_ERROR (Status)) {
+        goto OutputError;
+      }
+
+        break;
+    case TerminalTypeLinux:
     case TerminalTypeVtUtf8:
       UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
       Length = ValidBytes;
@@ -280,8 +458,10 @@ TerminalConOutOutputString (
       if (EFI_ERROR (Status)) {
         goto OutputError;
       }
+
       break;
     }
+
     //
     //  Update cursor position.
     //
@@ -875,7 +1055,8 @@ BOOLEAN
 TerminalIsValidTextGraphics (
   IN  CHAR16  Graphic,
   OUT CHAR8   *PcAnsi, OPTIONAL
-  OUT CHAR8   *Ascii OPTIONAL
+  OUT CHAR8   *Ascii, OPTIONAL
+  OUT CHAR8   *DecSpecialGraphics OPTIONAL
   )
 {
   UNICODE_TO_CHAR *Table;
@@ -897,6 +1078,9 @@ TerminalIsValidTextGraphics (
       if (Ascii != NULL) {
         *Ascii = Table->Ascii;
       }
+      if (DecSpecialGraphics != NULL){
+        *DecSpecialGraphics = Table->DecSpecialGraphics;
+      }
 
       return TRUE;
     }
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
  2021-07-30  2:45 ` [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters Caden Kline
@ 2021-08-03  0:04   ` Wu, Hao A
  2021-08-05 23:18   ` [edk2-devel] " Nate DeSimone
  2021-08-09  5:56   ` Wu, Hao A
  2 siblings, 0 replies; 6+ messages in thread
From: Wu, Hao A @ 2021-08-03  0:04 UTC (permalink / raw)
  To: Caden Kline, devel@edk2.groups.io, Gao, Zhichao, Ni, Ray; +Cc: Wang, Jian J

Sorry Zhichao and Ray,

Could you help to review this patch? Thanks in advance.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Caden Kline <cadenkline9@gmail.com>
> Sent: Friday, July 30, 2021 10:45 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box
> drawing characters
> 
> Improved encoding of box drawing characters for different terminal types.
> This includes Dec special graphics mode and more utf8.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Caden Kline <cadenkline9@gmail.com>
> ---
>  .../Universal/Console/TerminalDxe/Terminal.h  |  24 +-
>  .../Universal/Console/TerminalDxe/Ansi.c      |   2 +-
>  .../Console/TerminalDxe/TerminalConOut.c      | 322 ++++++++++++++----
>  3 files changed, 269 insertions(+), 79 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 360e58e84743..83c3ea94a042 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -122,7 +122,10 @@ typedef struct {
>    EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;
> 
>    LIST_ENTRY                          NotifyList;
> 
>    EFI_EVENT                           KeyNotifyProcessEvent;
> 
> +  BOOLEAN                             DecSpecialGraphicsMode;
> 
>  } TERMINAL_DEV;
> 
> +// This the lenth the escape squences for entering and exiting Dec Special
> Graphics Mode
> 
> +#define LENGTH_DEC_ESCAPE                 0x03
> 
> 
> 
>  #define INPUT_STATE_DEFAULT               0x00
> 
>  #define INPUT_STATE_ESC                   0x01
> 
> @@ -169,6 +172,7 @@ typedef struct {
>    UINT16  Unicode;
> 
>    CHAR8   PcAnsi;
> 
>    CHAR8   Ascii;
> 
> +  CHAR8   DecSpecialGraphics;
> 
>  } UNICODE_TO_CHAR;
> 
> 
> 
>  //
> 
> @@ -1367,20 +1371,22 @@ Utf8ToUnicode (
>  /**
> 
>    Detects if a Unicode char is for Box Drawing text graphics.
> 
> 
> 
> -  @param  Graphic      Unicode char to test.
> 
> -  @param  PcAnsi       Optional pointer to return PCANSI equivalent of
> 
> -                       Graphic.
> 
> -  @param  Ascii        Optional pointer to return ASCII equivalent of
> 
> -                       Graphic.
> 
> -
> 
> -  @retval TRUE         If Graphic is a supported Unicode Box Drawing character.
> 
> +  @param  Graphic             Unicode char to test.
> 
> +  @param  PcAnsi              Optional pointer to return PCANSI equivalent of
> 
> +                              Graphic.
> 
> +  @param  Ascii               Optional pointer to return ASCII equivalent of
> 
> +                              Graphic.
> 
> +  @param  DecSpecialGraphics  Optional pointer to return Dec Special Graphics
> equivalent of
> 
> +                              Graphic.
> 
> +  @retval TRUE                If Graphic is a supported Unicode Box Drawing
> character.
> 
> 
> 
>  **/
> 
>  BOOLEAN
> 
>  TerminalIsValidTextGraphics (
> 
>    IN  CHAR16  Graphic,
> 
> -  OUT CHAR8   *PcAnsi, OPTIONAL
> 
> -  OUT CHAR8   *Ascii OPTIONAL
> 
> +  OUT CHAR8   *PcAnsi,            OPTIONAL
> 
> +  OUT CHAR8   *Ascii,             OPTIONAL
> 
> +  OUT CHAR8   *DecSpecialGraphics OPTIONAL
> 
>    );
> 
> 
> 
>  /**
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> index f117d90b9de3..5ae5a4f0212e 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> @@ -63,7 +63,7 @@ AnsiTestString (
> 
> 
>      if ( !(TerminalIsValidAscii (*WString) ||
> 
>          TerminalIsValidEfiCntlChar (*WString) ||
> 
> -        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL) )) {
> 
> +        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL, NULL) )) {
> 
> 
> 
>        return EFI_UNSUPPORTED;
> 
>      }
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index aae470e9562c..1c22ed426715 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -16,61 +16,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
> 
>  //
> 
>  UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {
> 
> -  { BOXDRAW_HORIZONTAL,                 0xc4, L'-' },
> 
> -  { BOXDRAW_VERTICAL,                   0xb3, L'|' },
> 
> -  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/' },
> 
> -  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\' },
> 
> -  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\' },
> 
> -  { BOXDRAW_UP_LEFT,                    0xd9, L'/' },
> 
> -  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|' },
> 
> -  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|' },
> 
> -  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+' },
> 
> -  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+' },
> 
> -  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+' },
> 
> -  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-' },
> 
> -  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|' },
> 
> -  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/' },
> 
> -  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/' },
> 
> -  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/' },
> 
> -  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\' },
> 
> -  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\' },
> 
> -  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\' },
> 
> -  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\' },
> 
> -  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\' },
> 
> -  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\' },
> 
> -  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/' },
> 
> -  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/' },
> 
> -  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/' },
> 
> -  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|' },
> 
> -  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|' },
> 
> -  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|' },
> 
> -  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|' },
> 
> -  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|' },
> 
> -  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|' },
> 
> -  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+' },
> 
> -  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+' },
> 
> -  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+' },
> 
> -  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+' },
> 
> -  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+' },
> 
> -  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+' },
> 
> -  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+' },
> 
> -  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+' },
> 
> -  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+' },
> 
> +  { BOXDRAW_HORIZONTAL,                 0xc4, L'-',  0x71 },
> 
> +  { BOXDRAW_VERTICAL,                   0xb3, L'|',  0x78 },
> 
> +  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/',  0x6c },
> 
> +  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\', 0x6b },
> 
> +  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\', 0x6d },
> 
> +  { BOXDRAW_UP_LEFT,                    0xd9, L'/',  0x6a },
> 
> +  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|',  0x74 },
> 
> +  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|',  0x75 },
> 
> +  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+',  0x77 },
> 
> +  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+',  0x76 },
> 
> +  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+',  0x6e },
> 
> +  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-',  0x71 },
> 
> +  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|',  0x78 },
> 
> +  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/',  0x6c },
> 
> +  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/',  0x6c },
> 
> +  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/',  0x6c },
> 
> +  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\', 0x6b },
> 
> +  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\', 0x6b },
> 
> +  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\', 0x6b },
> 
> +  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\', 0x6d },
> 
> +  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\', 0x6d },
> 
> +  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\', 0x6d },
> 
> +  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/',  0x6a },
> 
> +  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/',  0x6a },
> 
> +  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/',  0x6a },
> 
> +  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|',  0x74 },
> 
> +  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|',  0x74 },
> 
> +  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|',  0x74 },
> 
> +  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|',  0x75 },
> 
> +  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|',  0x75 },
> 
> +  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|',  0x75 },
> 
> +  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+',  0x77 },
> 
> +  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+',  0x77 },
> 
> +  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+',  0x77 },
> 
> +  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+',  0x76 },
> 
> +  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+',  0x76 },
> 
> +  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+',  0x76 },
> 
> +  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+',  0x6e },
> 
> +  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+',  0x6e },
> 
> +  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+',  0x6e },
> 
> 
> 
> -  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*' },
> 
> -  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+' },
> 
> +  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*',  0x61 },
> 
> +  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+',  0x61 },
> 
> 
> 
> -  { GEOMETRICSHAPE_UP_TRIANGLE,         '^', L'^' },
> 
> -  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>', L'>' },
> 
> -  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v', L'v' },
> 
> -  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<', L'<' },
> 
> +  { GEOMETRICSHAPE_UP_TRIANGLE,         '^',  L'^',  L'^' },
> 
> +  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>',  L'>',  L'>' },
> 
> +  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v',  L'v',  L'v' },
> 
> +  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<',  L'<',  L'<' },
> 
> 
> 
> -  { ARROW_LEFT,                         '<', L'<' },
> 
> -  { ARROW_UP,                           '^', L'^' },
> 
> -  { ARROW_RIGHT,                        '>', L'>' },
> 
> -  { ARROW_DOWN,                         'v', L'v' },
> 
> -
> 
> -  { 0x0000,                             0x00, L'\0' }
> 
> +  { ARROW_LEFT,                         '<',  L'<',  L'<' },
> 
> +  { ARROW_UP,                           '^',  L'^',  L'^' },
> 
> +  { ARROW_RIGHT,                        '>',  L'>',  L'>' },
> 
> +  { ARROW_DOWN,                         'v',  L'v',  L'v' },
> 
>  };
> 
> 
> 
>  CHAR16 mSetModeString[]            = { ESC, '[', '=', '3', 'h', 0 };
> 
> @@ -80,6 +78,8 @@ CHAR16 mSetCursorPositionString[]  = { ESC, '[', '0', '0', ';',
> '0', '0', 'H', 0
>  CHAR16 mCursorForwardString[]      = { ESC, '[', '0', '0', 'C', 0 };
> 
>  CHAR16 mCursorBackwardString[]     = { ESC, '[', '0', '0', 'D', 0 };
> 
> 
> 
> +CHAR8  SetDecModeString[]          = {ESC, 0x28, 0x30};
> 
> +CHAR8  ExitDecModeString[]         = {ESC, 0x28, 0x42};
> 
>  //
> 
>  // Body of the ConOut functions
> 
>  //
> 
> @@ -183,16 +183,19 @@ TerminalConOutOutputString (
>    EFI_STATUS                  Status;
> 
>    UINT8                       ValidBytes;
> 
>    CHAR8                       CrLfStr[2];
> 
> +  CHAR8                       DecChar;
> 
> +  UINTN                       ModeSwitchLength;
> 
>    //
> 
>    //  flag used to indicate whether condition happens which will cause
> 
>    //  return EFI_WARN_UNKNOWN_GLYPH
> 
>    //
> 
>    BOOLEAN                     Warning;
> 
> 
> 
> -  ValidBytes  = 0;
> 
> -  Warning     = FALSE;
> 
> -  AsciiChar   = 0;
> 
> -
> 
> +  ValidBytes       = 0;
> 
> +  Warning          = FALSE;
> 
> +  AsciiChar        = 0;
> 
> +  DecChar          = 0;
> 
> +  ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
>    //
> 
>    //  get Terminal device data structure pointer.
> 
>    //
> 
> @@ -217,17 +220,136 @@ TerminalConOutOutputString (
>    for (; *WString != CHAR_NULL; WString++) {
> 
> 
> 
>      switch (TerminalDevice->TerminalType) {
> 
> -
> 
>      case TerminalTypePcAnsi:
> 
> -    case TerminalTypeVt100:
> 
> -    case TerminalTypeVt100Plus:
> 
> -    case TerminalTypeTtyTerm:
> 
> -    case TerminalTypeLinux:
> 
> +      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
> 
> +        //
> 
> +        // If it's not a graphic character convert Unicode to ASCII.
> 
> +        //
> 
> +          GraphicChar = (CHAR8)*WString;
> 
> +
> 
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
> 
> +          //
> 
> +          // when this driver use the OutputString to output control string,
> 
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
> 
> +          // to be output to the terminal emulation software.
> 
> +          //
> 
> +          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
> 
> +            GraphicChar = 27;
> 
> +          } else {
> 
> +            GraphicChar = '?';
> 
> +            Warning     = TRUE;
> 
> +          }
> 
> +        }
> 
> +
> 
> +        AsciiChar = GraphicChar;
> 
> +      }
> 
> +      Length = 1;
> 
> +      Status = TerminalDevice->SerialIo->Write (
> 
> +                                          TerminalDevice->SerialIo,
> 
> +                                          &Length,
> 
> +                                          &GraphicChar
> 
> +                                          );
> 
> +
> 
> +      if (EFI_ERROR (Status)) {
> 
> +        goto OutputError;
> 
> +      }
> 
> +
> 
> +      break;
> 
>      case TerminalTypeXtermR6:
> 
> -    case TerminalTypeVt400:
> 
>      case TerminalTypeSCO:
> 
> +        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)
> && !TerminalIsValidAscii (*WString)) {
> 
> +          //Box graphics are split into 2 types simple and advanced
> 
> +          //simple are drawn with dec special graphics
> 
> +          //advanced are drawn with utf8
> 
> +          //This checks for simple because they have a lower value than the
> advanced
> 
> +          if(*WString < BOXDRAW_DOUBLE_HORIZONTAL) {
> 
> +            if (!TerminalDevice->DecSpecialGraphicsMode) {
> 
> +              ValidBytes = 0;
> 
> +              ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +              Status = TerminalDevice->SerialIo->Write (
> 
> +                                                   TerminalDevice->SerialIo,
> 
> +                                                   &ModeSwitchLength,
> 
> +                                                   (UINT8 *)SetDecModeString
> 
> +                                          );
> 
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
> 
> +        }
> 
> 
> 
> -      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
> 
> +        GraphicChar = DecChar;
> 
> +        Length = 1;
> 
> +          } else {
> 
> +            if (TerminalDevice->DecSpecialGraphicsMode) {
> 
> +              ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +              Status = TerminalDevice->SerialIo->Write (
> 
> +                                                   TerminalDevice->SerialIo,
> 
> +                                                   &ModeSwitchLength,
> 
> +                                                   (UINT8 *)ExitDecModeString
> 
> +                                                );
> 
> +                if (EFI_ERROR (Status)) {
> 
> +                  goto OutputError;
> 
> +                }
> 
> +
> 
> +            TerminalDevice->DecSpecialGraphicsMode = FALSE;
> 
> +          }
> 
> +
> 
> +          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
> 
> +          Length = ValidBytes;
> 
> +        }
> 
> +        } else {
> 
> +          if (TerminalDevice->DecSpecialGraphicsMode) {
> 
> +          Status = TerminalDevice->SerialIo->Write (
> 
> +                                          TerminalDevice->SerialIo,
> 
> +                                          &ModeSwitchLength,
> 
> +                                          (UINT8 *)ExitDecModeString
> 
> +                                          );
> 
> +          if (EFI_ERROR (Status)) {
> 
> +            goto OutputError;
> 
> +          }
> 
> +
> 
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
> 
> +        }
> 
> +
> 
> +          GraphicChar = (CHAR8)*WString;
> 
> +
> 
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
> 
> +          //
> 
> +          // when this driver use the OutputString to output control string,
> 
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
> 
> +          // to be output to the terminal emulation software.
> 
> +          //
> 
> +            if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
> 
> +              GraphicChar = 27;
> 
> +              Length = 1;
> 
> +            } else {
> 
> +          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
> 
> +          Length = ValidBytes;
> 
> +            }
> 
> +          } else {
> 
> +        Length = 1;
> 
> +      }
> 
> +      }
> 
> +
> 
> +        if (ValidBytes) {
> 
> +        Status = TerminalDevice->SerialIo->Write (
> 
> +                                        TerminalDevice->SerialIo,
> 
> +                                        &Length,
> 
> +                                               (UINT8 *)&Utf8Char
> 
> +                                        );
> 
> +          ValidBytes = 0;
> 
> +        } else {
> 
> +      Status = TerminalDevice->SerialIo->Write (
> 
> +                                      TerminalDevice->SerialIo,
> 
> +                                      &Length,
> 
> +                                      &GraphicChar
> 
> +                                      );
> 
> +        }
> 
> +      if (EFI_ERROR (Status)) {
> 
> +        goto OutputError;
> 
> +      }
> 
> +
> 
> +      break;
> 
> +    case TerminalTypeVt100:
> 
> +    case TerminalTypeTtyTerm:
> 
> +      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
> 
>          //
> 
>          // If it's not a graphic character convert Unicode to ASCII.
> 
>          //
> 
> @@ -248,12 +370,9 @@ TerminalConOutOutputString (
>          }
> 
> 
> 
>          AsciiChar = GraphicChar;
> 
> -
> 
>        }
> 
> 
> 
> -      if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {
> 
> -        GraphicChar = AsciiChar;
> 
> -      }
> 
> +      GraphicChar = AsciiChar;
> 
> 
> 
>        Length = 1;
> 
> 
> 
> @@ -267,8 +386,67 @@ TerminalConOutOutputString (
>          goto OutputError;
> 
>        }
> 
> 
> 
> -      break;
> 
> +        break;
> 
> +    case TerminalTypeVt100Plus:
> 
> +    case TerminalTypeVt400:
> 
> +        Length = 1;
> 
> +        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)) {
> 
> +          if (!TerminalDevice->DecSpecialGraphicsMode) {
> 
> +            ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +            Status = TerminalDevice->SerialIo->Write (
> 
> +                                                 TerminalDevice->SerialIo,
> 
> +                                                 &ModeSwitchLength,
> 
> +                                                 (UINT8 *)SetDecModeString
> 
> +                                          );
> 
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
> 
> +        }
> 
> 
> 
> +        GraphicChar = DecChar;
> 
> +        } else {
> 
> +          if (TerminalDevice->DecSpecialGraphicsMode) {
> 
> +            ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +            Status = TerminalDevice->SerialIo->Write (
> 
> +                                                 TerminalDevice->SerialIo,
> 
> +                                                 &ModeSwitchLength,
> 
> +                                                 (UINT8 *)ExitDecModeString
> 
> +                                          );
> 
> +
> 
> +          if (EFI_ERROR (Status)) {
> 
> +            goto OutputError;
> 
> +          }
> 
> +
> 
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
> 
> +        }
> 
> +
> 
> +          GraphicChar = (CHAR8)*WString;
> 
> +
> 
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
> 
> +          //
> 
> +          // when this driver use the OutputString to output control string,
> 
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
> 
> +          // to be output to the terminal emulation software.
> 
> +          //
> 
> +          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
> 
> +            GraphicChar = 27;
> 
> +          } else {
> 
> +            GraphicChar = '?';
> 
> +            Warning     = TRUE;
> 
> +          }
> 
> +        }
> 
> +      }
> 
> +
> 
> +    Status = TerminalDevice->SerialIo->Write (
> 
> +                                      TerminalDevice->SerialIo,
> 
> +                                      &Length,
> 
> +                                      &GraphicChar
> 
> +                                      );
> 
> +
> 
> +      if (EFI_ERROR (Status)) {
> 
> +        goto OutputError;
> 
> +      }
> 
> +
> 
> +        break;
> 
> +    case TerminalTypeLinux:
> 
>      case TerminalTypeVtUtf8:
> 
>        UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
> 
>        Length = ValidBytes;
> 
> @@ -280,8 +458,10 @@ TerminalConOutOutputString (
>        if (EFI_ERROR (Status)) {
> 
>          goto OutputError;
> 
>        }
> 
> +
> 
>        break;
> 
>      }
> 
> +
> 
>      //
> 
>      //  Update cursor position.
> 
>      //
> 
> @@ -875,7 +1055,8 @@ BOOLEAN
>  TerminalIsValidTextGraphics (
> 
>    IN  CHAR16  Graphic,
> 
>    OUT CHAR8   *PcAnsi, OPTIONAL
> 
> -  OUT CHAR8   *Ascii OPTIONAL
> 
> +  OUT CHAR8   *Ascii, OPTIONAL
> 
> +  OUT CHAR8   *DecSpecialGraphics OPTIONAL
> 
>    )
> 
>  {
> 
>    UNICODE_TO_CHAR *Table;
> 
> @@ -897,6 +1078,9 @@ TerminalIsValidTextGraphics (
>        if (Ascii != NULL) {
> 
>          *Ascii = Table->Ascii;
> 
>        }
> 
> +      if (DecSpecialGraphics != NULL){
> 
> +        *DecSpecialGraphics = Table->DecSpecialGraphics;
> 
> +      }
> 
> 
> 
>        return TRUE;
> 
>      }
> 
> --
> 2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
  2021-07-30  2:45 ` [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters Caden Kline
  2021-08-03  0:04   ` Wu, Hao A
@ 2021-08-05 23:18   ` Nate DeSimone
  2021-08-09  5:56   ` Wu, Hao A
  2 siblings, 0 replies; 6+ messages in thread
From: Nate DeSimone @ 2021-08-05 23:18 UTC (permalink / raw)
  To: devel@edk2.groups.io, cadenkline9@gmail.com
  Cc: Wang, Jian J, Wu, Hao A, Gao, Zhichao, Ni, Ray

Caden,

Please make sure you test this patch thoroughly. With that...

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

Thanks!
Nate

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Caden
> Kline
> Sent: Thursday, July 29, 2021 7:45 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH 1/1] MdeModulePkg/Console: Improve
> encoding of box drawing characters
> 
> Improved encoding of box drawing characters for different terminal types.
> This includes Dec special graphics mode and more utf8.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Caden Kline <cadenkline9@gmail.com>
> ---
>  .../Universal/Console/TerminalDxe/Terminal.h  |  24 +-
>  .../Universal/Console/TerminalDxe/Ansi.c      |   2 +-
>  .../Console/TerminalDxe/TerminalConOut.c      | 322 ++++++++++++++----
>  3 files changed, 269 insertions(+), 79 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 360e58e84743..83c3ea94a042 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -122,7 +122,10 @@ typedef struct {
>    EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;   LIST_ENTRY
> NotifyList;   EFI_EVENT                           KeyNotifyProcessEvent;+  BOOLEAN
> DecSpecialGraphicsMode; } TERMINAL_DEV;+// This the lenth the escape
> squences for entering and exiting Dec Special Graphics Mode+#define
> LENGTH_DEC_ESCAPE                 0x03  #define INPUT_STATE_DEFAULT
> 0x00 #define INPUT_STATE_ESC                   0x01@@ -169,6 +172,7 @@
> typedef struct {
>    UINT16  Unicode;   CHAR8   PcAnsi;   CHAR8   Ascii;+  CHAR8
> DecSpecialGraphics; } UNICODE_TO_CHAR;  //@@ -1367,20 +1371,22 @@
> Utf8ToUnicode (
>  /**   Detects if a Unicode char is for Box Drawing text graphics. -  @param
> Graphic      Unicode char to test.-  @param  PcAnsi       Optional pointer to
> return PCANSI equivalent of-                       Graphic.-  @param  Ascii        Optional
> pointer to return ASCII equivalent of-                       Graphic.--  @retval TRUE
> If Graphic is a supported Unicode Box Drawing character.+  @param  Graphic
> Unicode char to test.+  @param  PcAnsi              Optional pointer to return
> PCANSI equivalent of+                              Graphic.+  @param  Ascii
> Optional pointer to return ASCII equivalent of+                              Graphic.+
> @param  DecSpecialGraphics  Optional pointer to return Dec Special Graphics
> equivalent of+                              Graphic.+  @retval TRUE                If Graphic is a
> supported Unicode Box Drawing character.  **/ BOOLEAN
> TerminalIsValidTextGraphics (   IN  CHAR16  Graphic,-  OUT CHAR8   *PcAnsi,
> OPTIONAL-  OUT CHAR8   *Ascii OPTIONAL+  OUT CHAR8   *PcAnsi,
> OPTIONAL+  OUT CHAR8   *Ascii,             OPTIONAL+  OUT CHAR8
> *DecSpecialGraphics OPTIONAL   );  /**diff --git
> a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> index f117d90b9de3..5ae5a4f0212e 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> @@ -63,7 +63,7 @@ AnsiTestString (
>       if ( !(TerminalIsValidAscii (*WString) ||         TerminalIsValidEfiCntlChar
> (*WString) ||-        TerminalIsValidTextGraphics (*WString, &GraphicChar,
> NULL) )) {+        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL,
> NULL) )) {        return EFI_UNSUPPORTED;     }diff --git
> a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index aae470e9562c..1c22ed426715 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -16,61 +16,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  // // UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {-  {
> BOXDRAW_HORIZONTAL,                 0xc4, L'-' },-  { BOXDRAW_VERTICAL,
> 0xb3, L'|' },-  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/' },-  {
> BOXDRAW_DOWN_LEFT,                  0xbf, L'\\' },-  { BOXDRAW_UP_RIGHT,
> 0xc0, L'\\' },-  { BOXDRAW_UP_LEFT,                    0xd9, L'/' },-  {
> BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|' },-  {
> BOXDRAW_VERTICAL_LEFT,              0xb4, L'|' },-  {
> BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+' },-  {
> BOXDRAW_UP_HORIZONTAL,              0xc1, L'+' },-  {
> BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+' },-  {
> BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-' },-  {
> BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|' },-  {
> BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/' },-  {
> BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/' },-  {
> BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/' },-  {
> BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\' },-  {
> BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\' },-  {
> BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\' },-  {
> BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\' },-  {
> BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\' },-  {
> BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\' },-  {
> BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/' },-  {
> BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/' },-  {
> BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/' },-  {
> BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|' },-  {
> BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|' },-  {
> BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|' },-  {
> BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|' },-  {
> BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|' },-  {
> BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|' },-  {
> BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+' },-  {
> BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+' },-  {
> BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+' },-  {
> BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+' },-  {
> BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+' },-  {
> BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+' },-  {
> BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+' },-  {
> BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+' },-  {
> BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+' },+  {
> BOXDRAW_HORIZONTAL,                 0xc4, L'-',  0x71 },+  { BOXDRAW_VERTICAL,
> 0xb3, L'|',  0x78 },+  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/',  0x6c },+  {
> BOXDRAW_DOWN_LEFT,                  0xbf, L'\\', 0x6b },+  {
> BOXDRAW_UP_RIGHT,                   0xc0, L'\\', 0x6d },+  { BOXDRAW_UP_LEFT,
> 0xd9, L'/',  0x6a },+  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|',  0x74 },+  {
> BOXDRAW_VERTICAL_LEFT,              0xb4, L'|',  0x75 },+  {
> BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+',  0x77 },+  {
> BOXDRAW_UP_HORIZONTAL,              0xc1, L'+',  0x76 },+  {
> BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+',  0x6e },+  {
> BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-',  0x71 },+  {
> BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|',  0x78 },+  {
> BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/',  0x6c },+  {
> BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/',  0x6c },+  {
> BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/',  0x6c },+  {
> BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\', 0x6b },+  {
> BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\', 0x6b },+  {
> BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\', 0x6b },+  {
> BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\', 0x6d },+  {
> BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\', 0x6d },+  {
> BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\', 0x6d },+  {
> BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/',  0x6a },+  {
> BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/',  0x6a },+  {
> BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/',  0x6a },+  {
> BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|',  0x74 },+  {
> BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|',  0x74 },+  {
> BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|',  0x74 },+  {
> BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|',  0x75 },+  {
> BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|',  0x75 },+  {
> BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|',  0x75 },+  {
> BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+',  0x77 },+  {
> BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+',  0x77 },+  {
> BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+',  0x77 },+  {
> BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+',  0x76 },+  {
> BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+',  0x76 },+  {
> BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+',  0x76 },+  {
> BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+',  0x6e },+  {
> BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+',  0x6e },+  {
> BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+',  0x6e }, -  {
> BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*' },-  {
> BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+' },+  {
> BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*',  0x61 },+  {
> BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+',  0x61 }, -  {
> GEOMETRICSHAPE_UP_TRIANGLE,         '^', L'^' },-  {
> GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>', L'>' },-  {
> GEOMETRICSHAPE_DOWN_TRIANGLE,       'v', L'v' },-  {
> GEOMETRICSHAPE_LEFT_TRIANGLE,       '<', L'<' },+  {
> GEOMETRICSHAPE_UP_TRIANGLE,         '^',  L'^',  L'^' },+  {
> GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>',  L'>',  L'>' },+  {
> GEOMETRICSHAPE_DOWN_TRIANGLE,       'v',  L'v',  L'v' },+  {
> GEOMETRICSHAPE_LEFT_TRIANGLE,       '<',  L'<',  L'<' }, -  { ARROW_LEFT,
> '<', L'<' },-  { ARROW_UP,                           '^', L'^' },-  { ARROW_RIGHT,
> '>', L'>' },-  { ARROW_DOWN,                         'v', L'v' },--  { 0x0000,
> 0x00, L'\0' }+  { ARROW_LEFT,                         '<',  L'<',  L'<' },+  { ARROW_UP,
> '^',  L'^',  L'^' },+  { ARROW_RIGHT,                        '>',  L'>',  L'>' },+  {
> ARROW_DOWN,                         'v',  L'v',  L'v' }, };  CHAR16 mSetModeString[]
> = { ESC, '[', '=', '3', 'h', 0 };@@ -80,6 +78,8 @@ CHAR16
> mSetCursorPositionString[]  = { ESC, '[', '0', '0', ';', '0', '0', 'H', 0
>  CHAR16 mCursorForwardString[]      = { ESC, '[', '0', '0', 'C', 0 }; CHAR16
> mCursorBackwardString[]     = { ESC, '[', '0', '0', 'D', 0 }; +CHAR8
> SetDecModeString[]          = {ESC, 0x28, 0x30};+CHAR8  ExitDecModeString[]
> = {ESC, 0x28, 0x42}; // // Body of the ConOut functions //@@ -183,16 +183,19
> @@ TerminalConOutOutputString (
>    EFI_STATUS                  Status;   UINT8                       ValidBytes;   CHAR8
> CrLfStr[2];+  CHAR8                       DecChar;+  UINTN
> ModeSwitchLength;   //   //  flag used to indicate whether condition happens
> which will cause   //  return EFI_WARN_UNKNOWN_GLYPH   //   BOOLEAN
> Warning; -  ValidBytes  = 0;-  Warning     = FALSE;-  AsciiChar   = 0;-+  ValidBytes
> = 0;+  Warning          = FALSE;+  AsciiChar        = 0;+  DecChar          = 0;+
> ModeSwitchLength = LENGTH_DEC_ESCAPE;   //   //  get Terminal device data
> structure pointer.   //@@ -217,17 +220,136 @@ TerminalConOutOutputString
> (
>    for (; *WString != CHAR_NULL; WString++) {      switch (TerminalDevice-
> >TerminalType) {-     case TerminalTypePcAnsi:-    case TerminalTypeVt100:-
> case TerminalTypeVt100Plus:-    case TerminalTypeTtyTerm:-    case
> TerminalTypeLinux:+      if (!TerminalIsValidTextGraphics (*WString,
> &GraphicChar, &AsciiChar, NULL)) {+        //+        // If it's not a graphic
> character convert Unicode to ASCII.+        //+          GraphicChar =
> (CHAR8)*WString;++        if (!(TerminalIsValidAscii (GraphicChar) ||
> TerminalIsValidEfiCntlChar (GraphicChar))) {+          //+          // when this driver
> use the OutputString to output control string,+          // TerminalDevice-
> >OutputEscChar is set to let the Esc char+          // to be output to the terminal
> emulation software.+          //+          if ((GraphicChar == 27) &&
> TerminalDevice->OutputEscChar) {+            GraphicChar = 27;+          } else {+
> GraphicChar = '?';+            Warning     = TRUE;+          }+        }++        AsciiChar =
> GraphicChar;+      }+      Length = 1;+      Status = TerminalDevice->SerialIo-
> >Write (+                                          TerminalDevice->SerialIo,+
> &Length,+                                          &GraphicChar+                                          );++      if
> (EFI_ERROR (Status)) {+        goto OutputError;+      }++      break;     case
> TerminalTypeXtermR6:-    case TerminalTypeVt400:     case
> TerminalTypeSCO:+        if (TerminalIsValidTextGraphics (*WString, NULL,
> NULL, &DecChar) && !TerminalIsValidAscii (*WString)) {+          //Box graphics
> are split into 2 types simple and advanced+          //simple are drawn with dec
> special graphics+          //advanced are drawn with utf8+          //This checks for
> simple because they have a lower value than the advanced+          if(*WString
> < BOXDRAW_DOUBLE_HORIZONTAL) {+            if (!TerminalDevice-
> >DecSpecialGraphicsMode) {+              ValidBytes = 0;+
> ModeSwitchLength = LENGTH_DEC_ESCAPE;+              Status =
> TerminalDevice->SerialIo->Write (+                                                   TerminalDevice-
> >SerialIo,+                                                   &ModeSwitchLength,+
> (UINT8 *)SetDecModeString+                                          );+          TerminalDevice-
> >DecSpecialGraphicsMode = TRUE;+        } -      if (!TerminalIsValidTextGraphics
> (*WString, &GraphicChar, &AsciiChar)) {+        GraphicChar = DecChar;+
> Length = 1;+          } else {+            if (TerminalDevice-
> >DecSpecialGraphicsMode) {+              ModeSwitchLength =
> LENGTH_DEC_ESCAPE;+              Status = TerminalDevice->SerialIo->Write (+
> TerminalDevice->SerialIo,+                                                   &ModeSwitchLength,+
> (UINT8 *)ExitDecModeString+                                                );+                if
> (EFI_ERROR (Status)) {+                  goto OutputError;+                }++
> TerminalDevice->DecSpecialGraphicsMode = FALSE;+          }++
> UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);+          Length =
> ValidBytes;+        }+        } else {+          if (TerminalDevice-
> >DecSpecialGraphicsMode) {+          Status = TerminalDevice->SerialIo->Write
> (+                                          TerminalDevice->SerialIo,+
> &ModeSwitchLength,+                                          (UINT8 *)ExitDecModeString+
> );+          if (EFI_ERROR (Status)) {+            goto OutputError;+          }++
> TerminalDevice->DecSpecialGraphicsMode = FALSE;+        }++
> GraphicChar = (CHAR8)*WString;++        if (!(TerminalIsValidAscii
> (GraphicChar) || TerminalIsValidEfiCntlChar (GraphicChar))) {+          //+
> // when this driver use the OutputString to output control string,+          //
> TerminalDevice->OutputEscChar is set to let the Esc char+          // to be
> output to the terminal emulation software.+          //+            if ((GraphicChar
> == 27) && TerminalDevice->OutputEscChar) {+              GraphicChar = 27;+
> Length = 1;+            } else {+          UnicodeToUtf8 (*WString, &Utf8Char,
> &ValidBytes);+          Length = ValidBytes;+            }+          } else {+        Length =
> 1;+      }+      }++        if (ValidBytes) {+        Status = TerminalDevice->SerialIo-
> >Write (+                                        TerminalDevice->SerialIo,+
> &Length,+                                               (UINT8 *)&Utf8Char+
> );+          ValidBytes = 0;+        } else {+      Status = TerminalDevice->SerialIo-
> >Write (+                                      TerminalDevice->SerialIo,+
> &Length,+                                      &GraphicChar+                                      );+        }+      if
> (EFI_ERROR (Status)) {+        goto OutputError;+      }++      break;+    case
> TerminalTypeVt100:+    case TerminalTypeTtyTerm:+      if
> (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar, NULL)) {
> //         // If it's not a graphic character convert Unicode to ASCII.         //@@ -
> 248,12 +370,9 @@ TerminalConOutOutputString (
>          }          AsciiChar = GraphicChar;-       } -      if (TerminalDevice-
> >TerminalType != TerminalTypePcAnsi) {-        GraphicChar = AsciiChar;-      }+
> GraphicChar = AsciiChar;        Length = 1; @@ -267,8 +386,67 @@
> TerminalConOutOutputString (
>          goto OutputError;       } -      break;+        break;+    case
> TerminalTypeVt100Plus:+    case TerminalTypeVt400:+        Length = 1;+        if
> (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)) {+          if
> (!TerminalDevice->DecSpecialGraphicsMode) {+            ModeSwitchLength =
> LENGTH_DEC_ESCAPE;+            Status = TerminalDevice->SerialIo->Write (+
> TerminalDevice->SerialIo,+                                                 &ModeSwitchLength,+
> (UINT8 *)SetDecModeString+                                          );+          TerminalDevice-
> >DecSpecialGraphicsMode = TRUE;+        } +        GraphicChar = DecChar;+        }
> else {+          if (TerminalDevice->DecSpecialGraphicsMode) {+
> ModeSwitchLength = LENGTH_DEC_ESCAPE;+            Status = TerminalDevice-
> >SerialIo->Write (+                                                 TerminalDevice->SerialIo,+
> &ModeSwitchLength,+                                                 (UINT8
> *)ExitDecModeString+                                          );++          if (EFI_ERROR (Status))
> {+            goto OutputError;+          }++          TerminalDevice-
> >DecSpecialGraphicsMode = FALSE;+        }++          GraphicChar =
> (CHAR8)*WString;++        if (!(TerminalIsValidAscii (GraphicChar) ||
> TerminalIsValidEfiCntlChar (GraphicChar))) {+          //+          // when this driver
> use the OutputString to output control string,+          // TerminalDevice-
> >OutputEscChar is set to let the Esc char+          // to be output to the terminal
> emulation software.+          //+          if ((GraphicChar == 27) &&
> TerminalDevice->OutputEscChar) {+            GraphicChar = 27;+          } else {+
> GraphicChar = '?';+            Warning     = TRUE;+          }+        }+      }++    Status =
> TerminalDevice->SerialIo->Write (+                                      TerminalDevice-
> >SerialIo,+                                      &Length,+                                      &GraphicChar+
> );++      if (EFI_ERROR (Status)) {+        goto OutputError;+      }++        break;+
> case TerminalTypeLinux:     case TerminalTypeVtUtf8:       UnicodeToUtf8
> (*WString, &Utf8Char, &ValidBytes);       Length = ValidBytes;@@ -280,8
> +458,10 @@ TerminalConOutOutputString (
>        if (EFI_ERROR (Status)) {         goto OutputError;       }+       break;     }+     //
> //  Update cursor position.     //@@ -875,7 +1055,8 @@ BOOLEAN
>  TerminalIsValidTextGraphics (   IN  CHAR16  Graphic,   OUT CHAR8   *PcAnsi,
> OPTIONAL-  OUT CHAR8   *Ascii OPTIONAL+  OUT CHAR8   *Ascii, OPTIONAL+
> OUT CHAR8   *DecSpecialGraphics OPTIONAL   ) {   UNICODE_TO_CHAR
> *Table;@@ -897,6 +1078,9 @@ TerminalIsValidTextGraphics (
>        if (Ascii != NULL) {         *Ascii = Table->Ascii;       }+      if (DecSpecialGraphics
> != NULL){+        *DecSpecialGraphics = Table->DecSpecialGraphics;+      }
> return TRUE;     }--
> 2.32.0
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#78390): https://edk2.groups.io/g/devel/message/78390
> Mute This Topic: https://groups.io/mt/84545877/1767664
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [nathaniel.l.desimone@intel.com] -=-=-=-=-=-=
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
  2021-07-30  2:45 ` [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters Caden Kline
  2021-08-03  0:04   ` Wu, Hao A
  2021-08-05 23:18   ` [edk2-devel] " Nate DeSimone
@ 2021-08-09  5:56   ` Wu, Hao A
       [not found]     ` <CAC3Y4+Q+50YEwFkr+SuJ18x7=FJbXP_K4Z56Ex9dS3=8TDfWbg@mail.gmail.com>
  2 siblings, 1 reply; 6+ messages in thread
From: Wu, Hao A @ 2021-08-09  5:56 UTC (permalink / raw)
  To: Caden Kline, devel@edk2.groups.io, Gao, Zhichao, Ni, Ray; +Cc: Wang, Jian J

Sorry Zhichao and Ray, could you please help to check if you have comments for this patch. Thanks in advance.

Hello Caden Kline, could you help to provide the information on what kind of unit test was done for this patch?

Some inline comments for ONLY coding style issues:


> -----Original Message-----
> From: Caden Kline <cadenkline9@gmail.com>
> Sent: Friday, July 30, 2021 10:45 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box
> drawing characters
> 
> Improved encoding of box drawing characters for different terminal types.
> This includes Dec special graphics mode and more utf8.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Caden Kline <cadenkline9@gmail.com>
> ---
>  .../Universal/Console/TerminalDxe/Terminal.h  |  24 +-
>  .../Universal/Console/TerminalDxe/Ansi.c      |   2 +-
>  .../Console/TerminalDxe/TerminalConOut.c      | 322 ++++++++++++++----
>  3 files changed, 269 insertions(+), 79 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 360e58e84743..83c3ea94a042 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -122,7 +122,10 @@ typedef struct {
>    EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;
> 
>    LIST_ENTRY                          NotifyList;
> 
>    EFI_EVENT                           KeyNotifyProcessEvent;
> 
> +  BOOLEAN                             DecSpecialGraphicsMode;
> 
>  } TERMINAL_DEV;
> 
> +// This the lenth the escape squences for entering and exiting Dec Special


Typo: lenth -> length, squences -> sequences 

Also, please help to update the comments to style:
//
// This the length the escape sequences for entering and exiting Dec Special
//



> Graphics Mode
> 
> +#define LENGTH_DEC_ESCAPE                 0x03
> 
> 
> 
>  #define INPUT_STATE_DEFAULT               0x00
> 
>  #define INPUT_STATE_ESC                   0x01
> 
> @@ -169,6 +172,7 @@ typedef struct {
>    UINT16  Unicode;
> 
>    CHAR8   PcAnsi;
> 
>    CHAR8   Ascii;
> 
> +  CHAR8   DecSpecialGraphics;
> 
>  } UNICODE_TO_CHAR;
> 
> 
> 
>  //
> 
> @@ -1367,20 +1371,22 @@ Utf8ToUnicode (
>  /**
> 
>    Detects if a Unicode char is for Box Drawing text graphics.
> 
> 
> 
> -  @param  Graphic      Unicode char to test.
> 
> -  @param  PcAnsi       Optional pointer to return PCANSI equivalent of
> 
> -                       Graphic.
> 
> -  @param  Ascii        Optional pointer to return ASCII equivalent of
> 
> -                       Graphic.
> 
> -
> 
> -  @retval TRUE         If Graphic is a supported Unicode Box Drawing character.
> 
> +  @param  Graphic             Unicode char to test.
> 
> +  @param  PcAnsi              Optional pointer to return PCANSI equivalent of
> 
> +                              Graphic.
> 
> +  @param  Ascii               Optional pointer to return ASCII equivalent of
> 
> +                              Graphic.
> 
> +  @param  DecSpecialGraphics  Optional pointer to return Dec Special
> Graphics equivalent of
> 
> +                              Graphic.
> 
> +  @retval TRUE                If Graphic is a supported Unicode Box Drawing
> character.
> 
> 
> 
>  **/
> 
>  BOOLEAN
> 
>  TerminalIsValidTextGraphics (
> 
>    IN  CHAR16  Graphic,
> 
> -  OUT CHAR8   *PcAnsi, OPTIONAL
> 
> -  OUT CHAR8   *Ascii OPTIONAL
> 
> +  OUT CHAR8   *PcAnsi,            OPTIONAL
> 
> +  OUT CHAR8   *Ascii,             OPTIONAL
> 
> +  OUT CHAR8   *DecSpecialGraphics OPTIONAL
> 
>    );
> 
> 
> 
>  /**
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> index f117d90b9de3..5ae5a4f0212e 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> @@ -63,7 +63,7 @@ AnsiTestString (
> 
> 
>      if ( !(TerminalIsValidAscii (*WString) ||
> 
>          TerminalIsValidEfiCntlChar (*WString) ||
> 
> -        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL) )) {
> 
> +        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL, NULL) )) {
> 
> 
> 
>        return EFI_UNSUPPORTED;
> 
>      }
> 
> diff --git
> a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index aae470e9562c..1c22ed426715 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -16,61 +16,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
> 
>  //
> 
>  UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {
> 
> -  { BOXDRAW_HORIZONTAL,                 0xc4, L'-' },
> 
> -  { BOXDRAW_VERTICAL,                   0xb3, L'|' },
> 
> -  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/' },
> 
> -  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\' },
> 
> -  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\' },
> 
> -  { BOXDRAW_UP_LEFT,                    0xd9, L'/' },
> 
> -  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|' },
> 
> -  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|' },
> 
> -  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+' },
> 
> -  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+' },
> 
> -  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+' },
> 
> -  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-' },
> 
> -  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|' },
> 
> -  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/' },
> 
> -  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/' },
> 
> -  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/' },
> 
> -  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\' },
> 
> -  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\' },
> 
> -  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\' },
> 
> -  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\' },
> 
> -  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\' },
> 
> -  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\' },
> 
> -  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/' },
> 
> -  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/' },
> 
> -  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/' },
> 
> -  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|' },
> 
> -  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|' },
> 
> -  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|' },
> 
> -  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|' },
> 
> -  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|' },
> 
> -  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|' },
> 
> -  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+' },
> 
> -  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+' },
> 
> -  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+' },
> 
> -  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+' },
> 
> -  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+' },
> 
> -  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+' },
> 
> -  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+' },
> 
> -  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+' },
> 
> -  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+' },
> 
> +  { BOXDRAW_HORIZONTAL,                 0xc4, L'-',  0x71 },
> 
> +  { BOXDRAW_VERTICAL,                   0xb3, L'|',  0x78 },
> 
> +  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/',  0x6c },
> 
> +  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\', 0x6b },
> 
> +  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\', 0x6d },
> 
> +  { BOXDRAW_UP_LEFT,                    0xd9, L'/',  0x6a },
> 
> +  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|',  0x74 },
> 
> +  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|',  0x75 },
> 
> +  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+',  0x77 },
> 
> +  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+',  0x76 },
> 
> +  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+',  0x6e },
> 
> +  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-',  0x71 },
> 
> +  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|',  0x78 },
> 
> +  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/',  0x6c },
> 
> +  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/',  0x6c },
> 
> +  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/',  0x6c },
> 
> +  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\', 0x6b },
> 
> +  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\', 0x6b },
> 
> +  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\', 0x6b },
> 
> +  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\', 0x6d },
> 
> +  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\', 0x6d },
> 
> +  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\', 0x6d },
> 
> +  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/',  0x6a },
> 
> +  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/',  0x6a },
> 
> +  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/',  0x6a },
> 
> +  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|',  0x74 },
> 
> +  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|',  0x74 },
> 
> +  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|',  0x74 },
> 
> +  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|',  0x75 },
> 
> +  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|',  0x75 },
> 
> +  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|',  0x75 },
> 
> +  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+',  0x77 },
> 
> +  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+',  0x77 },
> 
> +  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+',  0x77 },
> 
> +  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+',  0x76 },
> 
> +  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+',  0x76 },
> 
> +  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+',  0x76 },
> 
> +  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+',  0x6e },
> 
> +  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+',  0x6e },
> 
> +  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+',  0x6e },
> 
> 
> 
> -  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*' },
> 
> -  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+' },
> 
> +  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*',  0x61 },
> 
> +  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+',  0x61 },
> 
> 
> 
> -  { GEOMETRICSHAPE_UP_TRIANGLE,         '^', L'^' },
> 
> -  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>', L'>' },
> 
> -  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v', L'v' },
> 
> -  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<', L'<' },
> 
> +  { GEOMETRICSHAPE_UP_TRIANGLE,         '^',  L'^',  L'^' },
> 
> +  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>',  L'>',  L'>' },
> 
> +  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v',  L'v',  L'v' },
> 
> +  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<',  L'<',  L'<' },
> 
> 
> 
> -  { ARROW_LEFT,                         '<', L'<' },
> 
> -  { ARROW_UP,                           '^', L'^' },
> 
> -  { ARROW_RIGHT,                        '>', L'>' },
> 
> -  { ARROW_DOWN,                         'v', L'v' },
> 
> -
> 
> -  { 0x0000,                             0x00, L'\0' }
> 
> +  { ARROW_LEFT,                         '<',  L'<',  L'<' },
> 
> +  { ARROW_UP,                           '^',  L'^',  L'^' },
> 
> +  { ARROW_RIGHT,                        '>',  L'>',  L'>' },
> 
> +  { ARROW_DOWN,                         'v',  L'v',  L'v' },
> 
>  };
> 
> 
> 
>  CHAR16 mSetModeString[]            = { ESC, '[', '=', '3', 'h', 0 };
> 
> @@ -80,6 +78,8 @@ CHAR16 mSetCursorPositionString[]  = { ESC, '[', '0', '0', ';',
> '0', '0', 'H', 0
>  CHAR16 mCursorForwardString[]      = { ESC, '[', '0', '0', 'C', 0 };
> 
>  CHAR16 mCursorBackwardString[]     = { ESC, '[', '0', '0', 'D', 0 };
> 
> 
> 
> +CHAR8  SetDecModeString[]          = {ESC, 0x28, 0x30};
> 
> +CHAR8  ExitDecModeString[]         = {ESC, 0x28, 0x42};
> 
>  //
> 
>  // Body of the ConOut functions
> 
>  //
> 
> @@ -183,16 +183,19 @@ TerminalConOutOutputString (
>    EFI_STATUS                  Status;
> 
>    UINT8                       ValidBytes;
> 
>    CHAR8                       CrLfStr[2];
> 
> +  CHAR8                       DecChar;
> 
> +  UINTN                       ModeSwitchLength;
> 
>    //
> 
>    //  flag used to indicate whether condition happens which will cause
> 
>    //  return EFI_WARN_UNKNOWN_GLYPH
> 
>    //
> 
>    BOOLEAN                     Warning;
> 
> 
> 
> -  ValidBytes  = 0;
> 
> -  Warning     = FALSE;
> 
> -  AsciiChar   = 0;
> 
> -
> 
> +  ValidBytes       = 0;
> 
> +  Warning          = FALSE;
> 
> +  AsciiChar        = 0;
> 
> +  DecChar          = 0;
> 
> +  ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
>    //
> 
>    //  get Terminal device data structure pointer.
> 
>    //
> 
> @@ -217,17 +220,136 @@ TerminalConOutOutputString (
>    for (; *WString != CHAR_NULL; WString++) {
> 
> 
> 
>      switch (TerminalDevice->TerminalType) {
> 
> -
> 
>      case TerminalTypePcAnsi:
> 
> -    case TerminalTypeVt100:
> 
> -    case TerminalTypeVt100Plus:
> 
> -    case TerminalTypeTtyTerm:
> 
> -    case TerminalTypeLinux:
> 
> +      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
> 
> +        //
> 
> +        // If it's not a graphic character convert Unicode to ASCII.
> 
> +        //
> 
> +          GraphicChar = (CHAR8)*WString;


Please help to fix the space indent (extra spaces) here to keep it aligned with the contexts.


> 
> +
> 
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
> 
> +          //
> 
> +          // when this driver use the OutputString to output control string,
> 
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
> 
> +          // to be output to the terminal emulation software.
> 
> +          //
> 
> +          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
> 
> +            GraphicChar = 27;
> 
> +          } else {
> 
> +            GraphicChar = '?';
> 
> +            Warning     = TRUE;
> 
> +          }
> 
> +        }
> 
> +
> 
> +        AsciiChar = GraphicChar;
> 
> +      }
> 
> +      Length = 1;
> 
> +      Status = TerminalDevice->SerialIo->Write (
> 
> +                                          TerminalDevice->SerialIo,
> 
> +                                          &Length,
> 
> +                                          &GraphicChar
> 
> +                                          );


Please help to add an extra space for the above 3 lines for space indent coding style.


> 
> +
> 
> +      if (EFI_ERROR (Status)) {
> 
> +        goto OutputError;
> 
> +      }
> 
> +
> 
> +      break;
> 
>      case TerminalTypeXtermR6:
> 
> -    case TerminalTypeVt400:
> 
>      case TerminalTypeSCO:
> 


For the below added codes (for case TerminalTypeXtermR6 & TerminalTypeSCO), please help to fix below coding style issues:
a) Codes within a 'case' or 'if-else' statement is recommended to add 2 spaces for indent. (The space indent of the below chunk of codes are messy, which impacts the readability.)
b) For multi-line function calls, the input parameters (including the closing bracket) are recommended to add 2 spaces from the function name in the 1st line.
c) Please update multi-line comment to align with style:
//
// Comments...
//


> +        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)
> && !TerminalIsValidAscii (*WString)) {
> 
> +          //Box graphics are split into 2 types simple and advanced
> 
> +          //simple are drawn with dec special graphics
> 
> +          //advanced are drawn with utf8
> 
> +          //This checks for simple because they have a lower value than the
> advanced
> 
> +          if(*WString < BOXDRAW_DOUBLE_HORIZONTAL) {
> 
> +            if (!TerminalDevice->DecSpecialGraphicsMode) {
> 
> +              ValidBytes = 0;
> 
> +              ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +              Status = TerminalDevice->SerialIo->Write (
> 
> +                                                   TerminalDevice->SerialIo,
> 
> +                                                   &ModeSwitchLength,
> 
> +                                                   (UINT8 *)SetDecModeString
> 
> +                                          );
> 
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
> 
> +        }
> 
> 
> 
> -      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
> 
> +        GraphicChar = DecChar;
> 
> +        Length = 1;
> 
> +          } else {
> 
> +            if (TerminalDevice->DecSpecialGraphicsMode) {
> 
> +              ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +              Status = TerminalDevice->SerialIo->Write (
> 
> +                                                   TerminalDevice->SerialIo,
> 
> +                                                   &ModeSwitchLength,
> 
> +                                                   (UINT8 *)ExitDecModeString
> 
> +                                                );
> 
> +                if (EFI_ERROR (Status)) {
> 
> +                  goto OutputError;
> 
> +                }
> 
> +
> 
> +            TerminalDevice->DecSpecialGraphicsMode = FALSE;
> 
> +          }
> 
> +
> 
> +          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
> 
> +          Length = ValidBytes;
> 
> +        }
> 
> +        } else {
> 
> +          if (TerminalDevice->DecSpecialGraphicsMode) {
> 
> +          Status = TerminalDevice->SerialIo->Write (
> 
> +                                          TerminalDevice->SerialIo,
> 
> +                                          &ModeSwitchLength,
> 
> +                                          (UINT8 *)ExitDecModeString
> 
> +                                          );
> 
> +          if (EFI_ERROR (Status)) {
> 
> +            goto OutputError;
> 
> +          }
> 
> +
> 
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
> 
> +        }
> 
> +
> 
> +          GraphicChar = (CHAR8)*WString;
> 
> +
> 
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
> 
> +          //
> 
> +          // when this driver use the OutputString to output control string,
> 
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
> 
> +          // to be output to the terminal emulation software.
> 
> +          //
> 
> +            if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
> 
> +              GraphicChar = 27;
> 
> +              Length = 1;
> 
> +            } else {
> 
> +          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
> 
> +          Length = ValidBytes;
> 
> +            }
> 
> +          } else {
> 
> +        Length = 1;
> 
> +      }
> 
> +      }
> 
> +
> 
> +        if (ValidBytes) {
> 
> +        Status = TerminalDevice->SerialIo->Write (
> 
> +                                        TerminalDevice->SerialIo,
> 
> +                                        &Length,
> 
> +                                               (UINT8 *)&Utf8Char
> 
> +                                        );
> 
> +          ValidBytes = 0;
> 
> +        } else {
> 
> +      Status = TerminalDevice->SerialIo->Write (
> 
> +                                      TerminalDevice->SerialIo,
> 
> +                                      &Length,
> 
> +                                      &GraphicChar
> 
> +                                      );
> 
> +        }
> 
> +      if (EFI_ERROR (Status)) {
> 
> +        goto OutputError;
> 
> +      }
> 
> +
> 
> +      break;
> 
> +    case TerminalTypeVt100:
> 
> +    case TerminalTypeTtyTerm:
> 
> +      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
> 
>          //
> 
>          // If it's not a graphic character convert Unicode to ASCII.
> 
>          //
> 
> @@ -248,12 +370,9 @@ TerminalConOutOutputString (
>          }
> 
> 
> 
>          AsciiChar = GraphicChar;
> 
> -
> 
>        }
> 
> 
> 
> -      if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {
> 
> -        GraphicChar = AsciiChar;
> 
> -      }
> 
> +      GraphicChar = AsciiChar;
> 
> 
> 
>        Length = 1;
> 
> 
> 
> @@ -267,8 +386,67 @@ TerminalConOutOutputString (
>          goto OutputError;
> 
>        }
> 
> 
> 
> -      break;
> 
> +        break;
> 
> +    case TerminalTypeVt100Plus:
> 
> +    case TerminalTypeVt400:


For the below added codes (for case TerminalTypeVt100Plus & TerminalTypeVt400), please help to fix below coding style issues:
a) Codes within a 'case' or 'if-else' statement is recommended to add 2 spaces for indent. (The space indent of the below chunk of codes are messy, which impacts the readability.)
b) For multi-line function calls, the input parameters (including the closing bracket) are recommended to add 2 spaces from the function name in the 1st line.

Best Regards,
Hao Wu


> 
> +        Length = 1;
> 
> +        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)) {
> 
> +          if (!TerminalDevice->DecSpecialGraphicsMode) {
> 
> +            ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +            Status = TerminalDevice->SerialIo->Write (
> 
> +                                                 TerminalDevice->SerialIo,
> 
> +                                                 &ModeSwitchLength,
> 
> +                                                 (UINT8 *)SetDecModeString
> 
> +                                          );
> 
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
> 
> +        }
> 
> 
> 
> +        GraphicChar = DecChar;
> 
> +        } else {
> 
> +          if (TerminalDevice->DecSpecialGraphicsMode) {
> 
> +            ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +            Status = TerminalDevice->SerialIo->Write (
> 
> +                                                 TerminalDevice->SerialIo,
> 
> +                                                 &ModeSwitchLength,
> 
> +                                                 (UINT8 *)ExitDecModeString
> 
> +                                          );
> 
> +
> 
> +          if (EFI_ERROR (Status)) {
> 
> +            goto OutputError;
> 
> +          }
> 
> +
> 
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
> 
> +        }
> 
> +
> 
> +          GraphicChar = (CHAR8)*WString;
> 
> +
> 
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
> 
> +          //
> 
> +          // when this driver use the OutputString to output control string,
> 
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
> 
> +          // to be output to the terminal emulation software.
> 
> +          //
> 
> +          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
> 
> +            GraphicChar = 27;
> 
> +          } else {
> 
> +            GraphicChar = '?';
> 
> +            Warning     = TRUE;
> 
> +          }
> 
> +        }
> 
> +      }
> 
> +
> 
> +    Status = TerminalDevice->SerialIo->Write (
> 
> +                                      TerminalDevice->SerialIo,
> 
> +                                      &Length,
> 
> +                                      &GraphicChar
> 
> +                                      );
> 
> +
> 
> +      if (EFI_ERROR (Status)) {
> 
> +        goto OutputError;
> 
> +      }
> 
> +
> 
> +        break;
> 
> +    case TerminalTypeLinux:
> 
>      case TerminalTypeVtUtf8:
> 
>        UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
> 
>        Length = ValidBytes;
> 
> @@ -280,8 +458,10 @@ TerminalConOutOutputString (
>        if (EFI_ERROR (Status)) {
> 
>          goto OutputError;
> 
>        }
> 
> +
> 
>        break;
> 
>      }
> 
> +
> 
>      //
> 
>      //  Update cursor position.
> 
>      //
> 
> @@ -875,7 +1055,8 @@ BOOLEAN
>  TerminalIsValidTextGraphics (
> 
>    IN  CHAR16  Graphic,
> 
>    OUT CHAR8   *PcAnsi, OPTIONAL
> 
> -  OUT CHAR8   *Ascii OPTIONAL
> 
> +  OUT CHAR8   *Ascii, OPTIONAL
> 
> +  OUT CHAR8   *DecSpecialGraphics OPTIONAL
> 
>    )
> 
>  {
> 
>    UNICODE_TO_CHAR *Table;
> 
> @@ -897,6 +1078,9 @@ TerminalIsValidTextGraphics (
>        if (Ascii != NULL) {
> 
>          *Ascii = Table->Ascii;
> 
>        }
> 
> +      if (DecSpecialGraphics != NULL){
> 
> +        *DecSpecialGraphics = Table->DecSpecialGraphics;
> 
> +      }
> 
> 
> 
>        return TRUE;
> 
>      }
> 
> --
> 2.32.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
       [not found]     ` <CAC3Y4+Q+50YEwFkr+SuJ18x7=FJbXP_K4Z56Ex9dS3=8TDfWbg@mail.gmail.com>
@ 2021-08-13  1:18       ` Wu, Hao A
  2021-08-13  1:20         ` [edk2-devel] " Wu, Hao A
  0 siblings, 1 reply; 6+ messages in thread
From: Wu, Hao A @ 2021-08-13  1:18 UTC (permalink / raw)
  To: devel@edk2.groups.io, Caden Kline

[-- Attachment #1: Type: text/plain, Size: 25841 bytes --]

Reposting to the mailing list.

Best Regards,
Hao Wu

From: Caden Kline <cadenkline9@gmail.com>
Sent: Friday, August 13, 2021 12:32 AM
To: Wu, Hao A <hao.a.wu@intel.com>
Subject: Re: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters

For testing I just ran a program that printed out the impacted characters and checked it against different terminals, putty for windows, xtrem and tillix for linux, and standard mac os terminal with all the effected terminal types.

On Mon, Aug 9, 2021 at 1:57 AM Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote:
Sorry Zhichao and Ray, could you please help to check if you have comments for this patch. Thanks in advance.

Hello Caden Kline, could you help to provide the information on what kind of unit test was done for this patch?

Some inline comments for ONLY coding style issues:


> -----Original Message-----
> From: Caden Kline <cadenkline9@gmail.com<mailto:cadenkline9@gmail.com>>
> Sent: Friday, July 30, 2021 10:45 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>;
> Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Subject: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box
> drawing characters
>
> Improved encoding of box drawing characters for different terminal types.
> This includes Dec special graphics mode and more utf8.
>
> Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
> Cc: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Cc: Zhichao Gao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Signed-off-by: Caden Kline <cadenkline9@gmail.com<mailto:cadenkline9@gmail.com>>
> ---
>  .../Universal/Console/TerminalDxe/Terminal.h  |  24 +-
>  .../Universal/Console/TerminalDxe/Ansi.c      |   2 +-
>  .../Console/TerminalDxe/TerminalConOut.c      | 322 ++++++++++++++----
>  3 files changed, 269 insertions(+), 79 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 360e58e84743..83c3ea94a042 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -122,7 +122,10 @@ typedef struct {
>    EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;
>
>    LIST_ENTRY                          NotifyList;
>
>    EFI_EVENT                           KeyNotifyProcessEvent;
>
> +  BOOLEAN                             DecSpecialGraphicsMode;
>
>  } TERMINAL_DEV;
>
> +// This the lenth the escape squences for entering and exiting Dec Special


Typo: lenth -> length, squences -> sequences

Also, please help to update the comments to style:
//
// This the length the escape sequences for entering and exiting Dec Special
//



> Graphics Mode
>
> +#define LENGTH_DEC_ESCAPE                 0x03
>
>
>
>  #define INPUT_STATE_DEFAULT               0x00
>
>  #define INPUT_STATE_ESC                   0x01
>
> @@ -169,6 +172,7 @@ typedef struct {
>    UINT16  Unicode;
>
>    CHAR8   PcAnsi;
>
>    CHAR8   Ascii;
>
> +  CHAR8   DecSpecialGraphics;
>
>  } UNICODE_TO_CHAR;
>
>
>
>  //
>
> @@ -1367,20 +1371,22 @@ Utf8ToUnicode (
>  /**
>
>    Detects if a Unicode char is for Box Drawing text graphics.
>
>
>
> -  @param  Graphic      Unicode char to test.
>
> -  @param  PcAnsi       Optional pointer to return PCANSI equivalent of
>
> -                       Graphic.
>
> -  @param  Ascii        Optional pointer to return ASCII equivalent of
>
> -                       Graphic.
>
> -
>
> -  @retval TRUE         If Graphic is a supported Unicode Box Drawing character.
>
> +  @param  Graphic             Unicode char to test.
>
> +  @param  PcAnsi              Optional pointer to return PCANSI equivalent of
>
> +                              Graphic.
>
> +  @param  Ascii               Optional pointer to return ASCII equivalent of
>
> +                              Graphic.
>
> +  @param  DecSpecialGraphics  Optional pointer to return Dec Special
> Graphics equivalent of
>
> +                              Graphic.
>
> +  @retval TRUE                If Graphic is a supported Unicode Box Drawing
> character.
>
>
>
>  **/
>
>  BOOLEAN
>
>  TerminalIsValidTextGraphics (
>
>    IN  CHAR16  Graphic,
>
> -  OUT CHAR8   *PcAnsi, OPTIONAL
>
> -  OUT CHAR8   *Ascii OPTIONAL
>
> +  OUT CHAR8   *PcAnsi,            OPTIONAL
>
> +  OUT CHAR8   *Ascii,             OPTIONAL
>
> +  OUT CHAR8   *DecSpecialGraphics OPTIONAL
>
>    );
>
>
>
>  /**
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> index f117d90b9de3..5ae5a4f0212e 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> @@ -63,7 +63,7 @@ AnsiTestString (
>
>
>      if ( !(TerminalIsValidAscii (*WString) ||
>
>          TerminalIsValidEfiCntlChar (*WString) ||
>
> -        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL) )) {
>
> +        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL, NULL) )) {
>
>
>
>        return EFI_UNSUPPORTED;
>
>      }
>
> diff --git
> a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index aae470e9562c..1c22ed426715 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -16,61 +16,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
>
>  //
>
>  UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {
>
> -  { BOXDRAW_HORIZONTAL,                 0xc4, L'-' },
>
> -  { BOXDRAW_VERTICAL,                   0xb3, L'|' },
>
> -  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/' },
>
> -  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\' },
>
> -  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\' },
>
> -  { BOXDRAW_UP_LEFT,                    0xd9, L'/' },
>
> -  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|' },
>
> -  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|' },
>
> -  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+' },
>
> -  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+' },
>
> -  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+' },
>
> -  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-' },
>
> -  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|' },
>
> -  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/' },
>
> -  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/' },
>
> -  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/' },
>
> -  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\' },
>
> -  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\' },
>
> -  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\' },
>
> -  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\' },
>
> -  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\' },
>
> -  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\' },
>
> -  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/' },
>
> -  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/' },
>
> -  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/' },
>
> -  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|' },
>
> -  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|' },
>
> -  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|' },
>
> -  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|' },
>
> -  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|' },
>
> -  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|' },
>
> -  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+' },
>
> -  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+' },
>
> -  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+' },
>
> -  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+' },
>
> -  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+' },
>
> -  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+' },
>
> -  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+' },
>
> -  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+' },
>
> -  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+' },
>
> +  { BOXDRAW_HORIZONTAL,                 0xc4, L'-',  0x71 },
>
> +  { BOXDRAW_VERTICAL,                   0xb3, L'|',  0x78 },
>
> +  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/',  0x6c },
>
> +  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\', 0x6b },
>
> +  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\', 0x6d },
>
> +  { BOXDRAW_UP_LEFT,                    0xd9, L'/',  0x6a },
>
> +  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|',  0x74 },
>
> +  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|',  0x75 },
>
> +  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+',  0x77 },
>
> +  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+',  0x76 },
>
> +  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+',  0x6e },
>
> +  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-',  0x71 },
>
> +  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|',  0x78 },
>
> +  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/',  0x6c },
>
> +  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/',  0x6c },
>
> +  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/',  0x6c },
>
> +  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\', 0x6b },
>
> +  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\', 0x6b },
>
> +  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\', 0x6b },
>
> +  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\', 0x6d },
>
> +  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\', 0x6d },
>
> +  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\', 0x6d },
>
> +  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/',  0x6a },
>
> +  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/',  0x6a },
>
> +  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/',  0x6a },
>
> +  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|',  0x74 },
>
> +  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|',  0x74 },
>
> +  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|',  0x74 },
>
> +  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|',  0x75 },
>
> +  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|',  0x75 },
>
> +  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|',  0x75 },
>
> +  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+',  0x77 },
>
> +  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+',  0x77 },
>
> +  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+',  0x77 },
>
> +  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+',  0x76 },
>
> +  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+',  0x76 },
>
> +  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+',  0x76 },
>
> +  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+',  0x6e },
>
> +  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+',  0x6e },
>
> +  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+',  0x6e },
>
>
>
> -  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*' },
>
> -  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+' },
>
> +  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*',  0x61 },
>
> +  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+',  0x61 },
>
>
>
> -  { GEOMETRICSHAPE_UP_TRIANGLE,         '^', L'^' },
>
> -  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>', L'>' },
>
> -  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v', L'v' },
>
> -  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<', L'<' },
>
> +  { GEOMETRICSHAPE_UP_TRIANGLE,         '^',  L'^',  L'^' },
>
> +  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>',  L'>',  L'>' },
>
> +  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v',  L'v',  L'v' },
>
> +  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<',  L'<',  L'<' },
>
>
>
> -  { ARROW_LEFT,                         '<', L'<' },
>
> -  { ARROW_UP,                           '^', L'^' },
>
> -  { ARROW_RIGHT,                        '>', L'>' },
>
> -  { ARROW_DOWN,                         'v', L'v' },
>
> -
>
> -  { 0x0000,                             0x00, L'\0' }
>
> +  { ARROW_LEFT,                         '<',  L'<',  L'<' },
>
> +  { ARROW_UP,                           '^',  L'^',  L'^' },
>
> +  { ARROW_RIGHT,                        '>',  L'>',  L'>' },
>
> +  { ARROW_DOWN,                         'v',  L'v',  L'v' },
>
>  };
>
>
>
>  CHAR16 mSetModeString[]            = { ESC, '[', '=', '3', 'h', 0 };
>
> @@ -80,6 +78,8 @@ CHAR16 mSetCursorPositionString[]  = { ESC, '[', '0', '0', ';',
> '0', '0', 'H', 0
>  CHAR16 mCursorForwardString[]      = { ESC, '[', '0', '0', 'C', 0 };
>
>  CHAR16 mCursorBackwardString[]     = { ESC, '[', '0', '0', 'D', 0 };
>
>
>
> +CHAR8  SetDecModeString[]          = {ESC, 0x28, 0x30};
>
> +CHAR8  ExitDecModeString[]         = {ESC, 0x28, 0x42};
>
>  //
>
>  // Body of the ConOut functions
>
>  //
>
> @@ -183,16 +183,19 @@ TerminalConOutOutputString (
>    EFI_STATUS                  Status;
>
>    UINT8                       ValidBytes;
>
>    CHAR8                       CrLfStr[2];
>
> +  CHAR8                       DecChar;
>
> +  UINTN                       ModeSwitchLength;
>
>    //
>
>    //  flag used to indicate whether condition happens which will cause
>
>    //  return EFI_WARN_UNKNOWN_GLYPH
>
>    //
>
>    BOOLEAN                     Warning;
>
>
>
> -  ValidBytes  = 0;
>
> -  Warning     = FALSE;
>
> -  AsciiChar   = 0;
>
> -
>
> +  ValidBytes       = 0;
>
> +  Warning          = FALSE;
>
> +  AsciiChar        = 0;
>
> +  DecChar          = 0;
>
> +  ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
>    //
>
>    //  get Terminal device data structure pointer.
>
>    //
>
> @@ -217,17 +220,136 @@ TerminalConOutOutputString (
>    for (; *WString != CHAR_NULL; WString++) {
>
>
>
>      switch (TerminalDevice->TerminalType) {
>
> -
>
>      case TerminalTypePcAnsi:
>
> -    case TerminalTypeVt100:
>
> -    case TerminalTypeVt100Plus:
>
> -    case TerminalTypeTtyTerm:
>
> -    case TerminalTypeLinux:
>
> +      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
>
> +        //
>
> +        // If it's not a graphic character convert Unicode to ASCII.
>
> +        //
>
> +          GraphicChar = (CHAR8)*WString;


Please help to fix the space indent (extra spaces) here to keep it aligned with the contexts.


>
> +
>
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
>
> +          //
>
> +          // when this driver use the OutputString to output control string,
>
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
>
> +          // to be output to the terminal emulation software.
>
> +          //
>
> +          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
>
> +            GraphicChar = 27;
>
> +          } else {
>
> +            GraphicChar = '?';
>
> +            Warning     = TRUE;
>
> +          }
>
> +        }
>
> +
>
> +        AsciiChar = GraphicChar;
>
> +      }
>
> +      Length = 1;
>
> +      Status = TerminalDevice->SerialIo->Write (
>
> +                                          TerminalDevice->SerialIo,
>
> +                                          &Length,
>
> +                                          &GraphicChar
>
> +                                          );


Please help to add an extra space for the above 3 lines for space indent coding style.


>
> +
>
> +      if (EFI_ERROR (Status)) {
>
> +        goto OutputError;
>
> +      }
>
> +
>
> +      break;
>
>      case TerminalTypeXtermR6:
>
> -    case TerminalTypeVt400:
>
>      case TerminalTypeSCO:
>


For the below added codes (for case TerminalTypeXtermR6 & TerminalTypeSCO), please help to fix below coding style issues:
a) Codes within a 'case' or 'if-else' statement is recommended to add 2 spaces for indent. (The space indent of the below chunk of codes are messy, which impacts the readability.)
b) For multi-line function calls, the input parameters (including the closing bracket) are recommended to add 2 spaces from the function name in the 1st line.
c) Please update multi-line comment to align with style:
//
// Comments...
//


> +        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)
> && !TerminalIsValidAscii (*WString)) {
>
> +          //Box graphics are split into 2 types simple and advanced
>
> +          //simple are drawn with dec special graphics
>
> +          //advanced are drawn with utf8
>
> +          //This checks for simple because they have a lower value than the
> advanced
>
> +          if(*WString < BOXDRAW_DOUBLE_HORIZONTAL) {
>
> +            if (!TerminalDevice->DecSpecialGraphicsMode) {
>
> +              ValidBytes = 0;
>
> +              ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> +              Status = TerminalDevice->SerialIo->Write (
>
> +                                                   TerminalDevice->SerialIo,
>
> +                                                   &ModeSwitchLength,
>
> +                                                   (UINT8 *)SetDecModeString
>
> +                                          );
>
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
>
> +        }
>
>
>
> -      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
>
> +        GraphicChar = DecChar;
>
> +        Length = 1;
>
> +          } else {
>
> +            if (TerminalDevice->DecSpecialGraphicsMode) {
>
> +              ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> +              Status = TerminalDevice->SerialIo->Write (
>
> +                                                   TerminalDevice->SerialIo,
>
> +                                                   &ModeSwitchLength,
>
> +                                                   (UINT8 *)ExitDecModeString
>
> +                                                );
>
> +                if (EFI_ERROR (Status)) {
>
> +                  goto OutputError;
>
> +                }
>
> +
>
> +            TerminalDevice->DecSpecialGraphicsMode = FALSE;
>
> +          }
>
> +
>
> +          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
>
> +          Length = ValidBytes;
>
> +        }
>
> +        } else {
>
> +          if (TerminalDevice->DecSpecialGraphicsMode) {
>
> +          Status = TerminalDevice->SerialIo->Write (
>
> +                                          TerminalDevice->SerialIo,
>
> +                                          &ModeSwitchLength,
>
> +                                          (UINT8 *)ExitDecModeString
>
> +                                          );
>
> +          if (EFI_ERROR (Status)) {
>
> +            goto OutputError;
>
> +          }
>
> +
>
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
>
> +        }
>
> +
>
> +          GraphicChar = (CHAR8)*WString;
>
> +
>
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
>
> +          //
>
> +          // when this driver use the OutputString to output control string,
>
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
>
> +          // to be output to the terminal emulation software.
>
> +          //
>
> +            if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
>
> +              GraphicChar = 27;
>
> +              Length = 1;
>
> +            } else {
>
> +          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
>
> +          Length = ValidBytes;
>
> +            }
>
> +          } else {
>
> +        Length = 1;
>
> +      }
>
> +      }
>
> +
>
> +        if (ValidBytes) {
>
> +        Status = TerminalDevice->SerialIo->Write (
>
> +                                        TerminalDevice->SerialIo,
>
> +                                        &Length,
>
> +                                               (UINT8 *)&Utf8Char
>
> +                                        );
>
> +          ValidBytes = 0;
>
> +        } else {
>
> +      Status = TerminalDevice->SerialIo->Write (
>
> +                                      TerminalDevice->SerialIo,
>
> +                                      &Length,
>
> +                                      &GraphicChar
>
> +                                      );
>
> +        }
>
> +      if (EFI_ERROR (Status)) {
>
> +        goto OutputError;
>
> +      }
>
> +
>
> +      break;
>
> +    case TerminalTypeVt100:
>
> +    case TerminalTypeTtyTerm:
>
> +      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
>
>          //
>
>          // If it's not a graphic character convert Unicode to ASCII.
>
>          //
>
> @@ -248,12 +370,9 @@ TerminalConOutOutputString (
>          }
>
>
>
>          AsciiChar = GraphicChar;
>
> -
>
>        }
>
>
>
> -      if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {
>
> -        GraphicChar = AsciiChar;
>
> -      }
>
> +      GraphicChar = AsciiChar;
>
>
>
>        Length = 1;
>
>
>
> @@ -267,8 +386,67 @@ TerminalConOutOutputString (
>          goto OutputError;
>
>        }
>
>
>
> -      break;
>
> +        break;
>
> +    case TerminalTypeVt100Plus:
>
> +    case TerminalTypeVt400:


For the below added codes (for case TerminalTypeVt100Plus & TerminalTypeVt400), please help to fix below coding style issues:
a) Codes within a 'case' or 'if-else' statement is recommended to add 2 spaces for indent. (The space indent of the below chunk of codes are messy, which impacts the readability.)
b) For multi-line function calls, the input parameters (including the closing bracket) are recommended to add 2 spaces from the function name in the 1st line.

Best Regards,
Hao Wu


>
> +        Length = 1;
>
> +        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)) {
>
> +          if (!TerminalDevice->DecSpecialGraphicsMode) {
>
> +            ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> +            Status = TerminalDevice->SerialIo->Write (
>
> +                                                 TerminalDevice->SerialIo,
>
> +                                                 &ModeSwitchLength,
>
> +                                                 (UINT8 *)SetDecModeString
>
> +                                          );
>
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
>
> +        }
>
>
>
> +        GraphicChar = DecChar;
>
> +        } else {
>
> +          if (TerminalDevice->DecSpecialGraphicsMode) {
>
> +            ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> +            Status = TerminalDevice->SerialIo->Write (
>
> +                                                 TerminalDevice->SerialIo,
>
> +                                                 &ModeSwitchLength,
>
> +                                                 (UINT8 *)ExitDecModeString
>
> +                                          );
>
> +
>
> +          if (EFI_ERROR (Status)) {
>
> +            goto OutputError;
>
> +          }
>
> +
>
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
>
> +        }
>
> +
>
> +          GraphicChar = (CHAR8)*WString;
>
> +
>
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
>
> +          //
>
> +          // when this driver use the OutputString to output control string,
>
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
>
> +          // to be output to the terminal emulation software.
>
> +          //
>
> +          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
>
> +            GraphicChar = 27;
>
> +          } else {
>
> +            GraphicChar = '?';
>
> +            Warning     = TRUE;
>
> +          }
>
> +        }
>
> +      }
>
> +
>
> +    Status = TerminalDevice->SerialIo->Write (
>
> +                                      TerminalDevice->SerialIo,
>
> +                                      &Length,
>
> +                                      &GraphicChar
>
> +                                      );
>
> +
>
> +      if (EFI_ERROR (Status)) {
>
> +        goto OutputError;
>
> +      }
>
> +
>
> +        break;
>
> +    case TerminalTypeLinux:
>
>      case TerminalTypeVtUtf8:
>
>        UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
>
>        Length = ValidBytes;
>
> @@ -280,8 +458,10 @@ TerminalConOutOutputString (
>        if (EFI_ERROR (Status)) {
>
>          goto OutputError;
>
>        }
>
> +
>
>        break;
>
>      }
>
> +
>
>      //
>
>      //  Update cursor position.
>
>      //
>
> @@ -875,7 +1055,8 @@ BOOLEAN
>  TerminalIsValidTextGraphics (
>
>    IN  CHAR16  Graphic,
>
>    OUT CHAR8   *PcAnsi, OPTIONAL
>
> -  OUT CHAR8   *Ascii OPTIONAL
>
> +  OUT CHAR8   *Ascii, OPTIONAL
>
> +  OUT CHAR8   *DecSpecialGraphics OPTIONAL
>
>    )
>
>  {
>
>    UNICODE_TO_CHAR *Table;
>
> @@ -897,6 +1078,9 @@ TerminalIsValidTextGraphics (
>        if (Ascii != NULL) {
>
>          *Ascii = Table->Ascii;
>
>        }
>
> +      if (DecSpecialGraphics != NULL){
>
> +        *DecSpecialGraphics = Table->DecSpecialGraphics;
>
> +      }
>
>
>
>        return TRUE;
>
>      }
>
> --
> 2.32.0

[-- Attachment #2: Type: text/html, Size: 50727 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
  2021-08-13  1:18       ` Wu, Hao A
@ 2021-08-13  1:20         ` Wu, Hao A
  0 siblings, 0 replies; 6+ messages in thread
From: Wu, Hao A @ 2021-08-13  1:20 UTC (permalink / raw)
  To: devel@edk2.groups.io, Ni, Ray, Gao, Zhichao; +Cc: Wu, Hao A, Caden Kline

[-- Attachment #1: Type: text/plain, Size: 26254 bytes --]

Add Console and Graphics modules reviewers.

Best Regards,
Hao Wu

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao A
Sent: Friday, August 13, 2021 9:19 AM
To: devel@edk2.groups.io; Caden Kline <cadenkline9@gmail.com>
Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters

Reposting to the mailing list.

Best Regards,
Hao Wu

From: Caden Kline <cadenkline9@gmail.com<mailto:cadenkline9@gmail.com>>
Sent: Friday, August 13, 2021 12:32 AM
To: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Subject: Re: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters

For testing I just ran a program that printed out the impacted characters and checked it against different terminals, putty for windows, xtrem and tillix for linux, and standard mac os terminal with all the effected terminal types.

On Mon, Aug 9, 2021 at 1:57 AM Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote:
Sorry Zhichao and Ray, could you please help to check if you have comments for this patch. Thanks in advance.

Hello Caden Kline, could you help to provide the information on what kind of unit test was done for this patch?

Some inline comments for ONLY coding style issues:


> -----Original Message-----
> From: Caden Kline <cadenkline9@gmail.com<mailto:cadenkline9@gmail.com>>
> Sent: Friday, July 30, 2021 10:45 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>;
> Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Subject: [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box
> drawing characters
>
> Improved encoding of box drawing characters for different terminal types.
> This includes Dec special graphics mode and more utf8.
>
> Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
> Cc: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Cc: Zhichao Gao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Signed-off-by: Caden Kline <cadenkline9@gmail.com<mailto:cadenkline9@gmail.com>>
> ---
>  .../Universal/Console/TerminalDxe/Terminal.h  |  24 +-
>  .../Universal/Console/TerminalDxe/Ansi.c      |   2 +-
>  .../Console/TerminalDxe/TerminalConOut.c      | 322 ++++++++++++++----
>  3 files changed, 269 insertions(+), 79 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 360e58e84743..83c3ea94a042 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -122,7 +122,10 @@ typedef struct {
>    EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;
>
>    LIST_ENTRY                          NotifyList;
>
>    EFI_EVENT                           KeyNotifyProcessEvent;
>
> +  BOOLEAN                             DecSpecialGraphicsMode;
>
>  } TERMINAL_DEV;
>
> +// This the lenth the escape squences for entering and exiting Dec Special


Typo: lenth -> length, squences -> sequences

Also, please help to update the comments to style:
//
// This the length the escape sequences for entering and exiting Dec Special
//



> Graphics Mode
>
> +#define LENGTH_DEC_ESCAPE                 0x03
>
>
>
>  #define INPUT_STATE_DEFAULT               0x00
>
>  #define INPUT_STATE_ESC                   0x01
>
> @@ -169,6 +172,7 @@ typedef struct {
>    UINT16  Unicode;
>
>    CHAR8   PcAnsi;
>
>    CHAR8   Ascii;
>
> +  CHAR8   DecSpecialGraphics;
>
>  } UNICODE_TO_CHAR;
>
>
>
>  //
>
> @@ -1367,20 +1371,22 @@ Utf8ToUnicode (
>  /**
>
>    Detects if a Unicode char is for Box Drawing text graphics.
>
>
>
> -  @param  Graphic      Unicode char to test.
>
> -  @param  PcAnsi       Optional pointer to return PCANSI equivalent of
>
> -                       Graphic.
>
> -  @param  Ascii        Optional pointer to return ASCII equivalent of
>
> -                       Graphic.
>
> -
>
> -  @retval TRUE         If Graphic is a supported Unicode Box Drawing character.
>
> +  @param  Graphic             Unicode char to test.
>
> +  @param  PcAnsi              Optional pointer to return PCANSI equivalent of
>
> +                              Graphic.
>
> +  @param  Ascii               Optional pointer to return ASCII equivalent of
>
> +                              Graphic.
>
> +  @param  DecSpecialGraphics  Optional pointer to return Dec Special
> Graphics equivalent of
>
> +                              Graphic.
>
> +  @retval TRUE                If Graphic is a supported Unicode Box Drawing
> character.
>
>
>
>  **/
>
>  BOOLEAN
>
>  TerminalIsValidTextGraphics (
>
>    IN  CHAR16  Graphic,
>
> -  OUT CHAR8   *PcAnsi, OPTIONAL
>
> -  OUT CHAR8   *Ascii OPTIONAL
>
> +  OUT CHAR8   *PcAnsi,            OPTIONAL
>
> +  OUT CHAR8   *Ascii,             OPTIONAL
>
> +  OUT CHAR8   *DecSpecialGraphics OPTIONAL
>
>    );
>
>
>
>  /**
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> index f117d90b9de3..5ae5a4f0212e 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> @@ -63,7 +63,7 @@ AnsiTestString (
>
>
>      if ( !(TerminalIsValidAscii (*WString) ||
>
>          TerminalIsValidEfiCntlChar (*WString) ||
>
> -        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL) )) {
>
> +        TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL, NULL) )) {
>
>
>
>        return EFI_UNSUPPORTED;
>
>      }
>
> diff --git
> a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index aae470e9562c..1c22ed426715 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -16,61 +16,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
>
>  //
>
>  UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {
>
> -  { BOXDRAW_HORIZONTAL,                 0xc4, L'-' },
>
> -  { BOXDRAW_VERTICAL,                   0xb3, L'|' },
>
> -  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/' },
>
> -  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\' },
>
> -  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\' },
>
> -  { BOXDRAW_UP_LEFT,                    0xd9, L'/' },
>
> -  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|' },
>
> -  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|' },
>
> -  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+' },
>
> -  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+' },
>
> -  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+' },
>
> -  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-' },
>
> -  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|' },
>
> -  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/' },
>
> -  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/' },
>
> -  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/' },
>
> -  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\' },
>
> -  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\' },
>
> -  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\' },
>
> -  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\' },
>
> -  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\' },
>
> -  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\' },
>
> -  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/' },
>
> -  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/' },
>
> -  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/' },
>
> -  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|' },
>
> -  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|' },
>
> -  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|' },
>
> -  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|' },
>
> -  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|' },
>
> -  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|' },
>
> -  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+' },
>
> -  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+' },
>
> -  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+' },
>
> -  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+' },
>
> -  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+' },
>
> -  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+' },
>
> -  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+' },
>
> -  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+' },
>
> -  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+' },
>
> +  { BOXDRAW_HORIZONTAL,                 0xc4, L'-',  0x71 },
>
> +  { BOXDRAW_VERTICAL,                   0xb3, L'|',  0x78 },
>
> +  { BOXDRAW_DOWN_RIGHT,                 0xda, L'/',  0x6c },
>
> +  { BOXDRAW_DOWN_LEFT,                  0xbf, L'\\', 0x6b },
>
> +  { BOXDRAW_UP_RIGHT,                   0xc0, L'\\', 0x6d },
>
> +  { BOXDRAW_UP_LEFT,                    0xd9, L'/',  0x6a },
>
> +  { BOXDRAW_VERTICAL_RIGHT,             0xc3, L'|',  0x74 },
>
> +  { BOXDRAW_VERTICAL_LEFT,              0xb4, L'|',  0x75 },
>
> +  { BOXDRAW_DOWN_HORIZONTAL,            0xc2, L'+',  0x77 },
>
> +  { BOXDRAW_UP_HORIZONTAL,              0xc1, L'+',  0x76 },
>
> +  { BOXDRAW_VERTICAL_HORIZONTAL,        0xc5, L'+',  0x6e },
>
> +  { BOXDRAW_DOUBLE_HORIZONTAL,          0xcd, L'-',  0x71 },
>
> +  { BOXDRAW_DOUBLE_VERTICAL,            0xba, L'|',  0x78 },
>
> +  { BOXDRAW_DOWN_RIGHT_DOUBLE,          0xd5, L'/',  0x6c },
>
> +  { BOXDRAW_DOWN_DOUBLE_RIGHT,          0xd6, L'/',  0x6c },
>
> +  { BOXDRAW_DOUBLE_DOWN_RIGHT,          0xc9, L'/',  0x6c },
>
> +  { BOXDRAW_DOWN_LEFT_DOUBLE,           0xb8, L'\\', 0x6b },
>
> +  { BOXDRAW_DOWN_DOUBLE_LEFT,           0xb7, L'\\', 0x6b },
>
> +  { BOXDRAW_DOUBLE_DOWN_LEFT,           0xbb, L'\\', 0x6b },
>
> +  { BOXDRAW_UP_RIGHT_DOUBLE,            0xd4, L'\\', 0x6d },
>
> +  { BOXDRAW_UP_DOUBLE_RIGHT,            0xd3, L'\\', 0x6d },
>
> +  { BOXDRAW_DOUBLE_UP_RIGHT,            0xc8, L'\\', 0x6d },
>
> +  { BOXDRAW_UP_LEFT_DOUBLE,             0xbe, L'/',  0x6a },
>
> +  { BOXDRAW_UP_DOUBLE_LEFT,             0xbd, L'/',  0x6a },
>
> +  { BOXDRAW_DOUBLE_UP_LEFT,             0xbc, L'/',  0x6a },
>
> +  { BOXDRAW_VERTICAL_RIGHT_DOUBLE,      0xc6, L'|',  0x74 },
>
> +  { BOXDRAW_VERTICAL_DOUBLE_RIGHT,      0xc7, L'|',  0x74 },
>
> +  { BOXDRAW_DOUBLE_VERTICAL_RIGHT,      0xcc, L'|',  0x74 },
>
> +  { BOXDRAW_VERTICAL_LEFT_DOUBLE,       0xb5, L'|',  0x75 },
>
> +  { BOXDRAW_VERTICAL_DOUBLE_LEFT,       0xb6, L'|',  0x75 },
>
> +  { BOXDRAW_DOUBLE_VERTICAL_LEFT,       0xb9, L'|',  0x75 },
>
> +  { BOXDRAW_DOWN_HORIZONTAL_DOUBLE,     0xd1, L'+',  0x77 },
>
> +  { BOXDRAW_DOWN_DOUBLE_HORIZONTAL,     0xd2, L'+',  0x77 },
>
> +  { BOXDRAW_DOUBLE_DOWN_HORIZONTAL,     0xcb, L'+',  0x77 },
>
> +  { BOXDRAW_UP_HORIZONTAL_DOUBLE,       0xcf, L'+',  0x76 },
>
> +  { BOXDRAW_UP_DOUBLE_HORIZONTAL,       0xd0, L'+',  0x76 },
>
> +  { BOXDRAW_DOUBLE_UP_HORIZONTAL,       0xca, L'+',  0x76 },
>
> +  { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+',  0x6e },
>
> +  { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+',  0x6e },
>
> +  { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+',  0x6e },
>
>
>
> -  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*' },
>
> -  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+' },
>
> +  { BLOCKELEMENT_FULL_BLOCK,            0xdb, L'*',  0x61 },
>
> +  { BLOCKELEMENT_LIGHT_SHADE,           0xb0, L'+',  0x61 },
>
>
>
> -  { GEOMETRICSHAPE_UP_TRIANGLE,         '^', L'^' },
>
> -  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>', L'>' },
>
> -  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v', L'v' },
>
> -  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<', L'<' },
>
> +  { GEOMETRICSHAPE_UP_TRIANGLE,         '^',  L'^',  L'^' },
>
> +  { GEOMETRICSHAPE_RIGHT_TRIANGLE,      '>',  L'>',  L'>' },
>
> +  { GEOMETRICSHAPE_DOWN_TRIANGLE,       'v',  L'v',  L'v' },
>
> +  { GEOMETRICSHAPE_LEFT_TRIANGLE,       '<',  L'<',  L'<' },
>
>
>
> -  { ARROW_LEFT,                         '<', L'<' },
>
> -  { ARROW_UP,                           '^', L'^' },
>
> -  { ARROW_RIGHT,                        '>', L'>' },
>
> -  { ARROW_DOWN,                         'v', L'v' },
>
> -
>
> -  { 0x0000,                             0x00, L'\0' }
>
> +  { ARROW_LEFT,                         '<',  L'<',  L'<' },
>
> +  { ARROW_UP,                           '^',  L'^',  L'^' },
>
> +  { ARROW_RIGHT,                        '>',  L'>',  L'>' },
>
> +  { ARROW_DOWN,                         'v',  L'v',  L'v' },
>
>  };
>
>
>
>  CHAR16 mSetModeString[]            = { ESC, '[', '=', '3', 'h', 0 };
>
> @@ -80,6 +78,8 @@ CHAR16 mSetCursorPositionString[]  = { ESC, '[', '0', '0', ';',
> '0', '0', 'H', 0
>  CHAR16 mCursorForwardString[]      = { ESC, '[', '0', '0', 'C', 0 };
>
>  CHAR16 mCursorBackwardString[]     = { ESC, '[', '0', '0', 'D', 0 };
>
>
>
> +CHAR8  SetDecModeString[]          = {ESC, 0x28, 0x30};
>
> +CHAR8  ExitDecModeString[]         = {ESC, 0x28, 0x42};
>
>  //
>
>  // Body of the ConOut functions
>
>  //
>
> @@ -183,16 +183,19 @@ TerminalConOutOutputString (
>    EFI_STATUS                  Status;
>
>    UINT8                       ValidBytes;
>
>    CHAR8                       CrLfStr[2];
>
> +  CHAR8                       DecChar;
>
> +  UINTN                       ModeSwitchLength;
>
>    //
>
>    //  flag used to indicate whether condition happens which will cause
>
>    //  return EFI_WARN_UNKNOWN_GLYPH
>
>    //
>
>    BOOLEAN                     Warning;
>
>
>
> -  ValidBytes  = 0;
>
> -  Warning     = FALSE;
>
> -  AsciiChar   = 0;
>
> -
>
> +  ValidBytes       = 0;
>
> +  Warning          = FALSE;
>
> +  AsciiChar        = 0;
>
> +  DecChar          = 0;
>
> +  ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
>    //
>
>    //  get Terminal device data structure pointer.
>
>    //
>
> @@ -217,17 +220,136 @@ TerminalConOutOutputString (
>    for (; *WString != CHAR_NULL; WString++) {
>
>
>
>      switch (TerminalDevice->TerminalType) {
>
> -
>
>      case TerminalTypePcAnsi:
>
> -    case TerminalTypeVt100:
>
> -    case TerminalTypeVt100Plus:
>
> -    case TerminalTypeTtyTerm:
>
> -    case TerminalTypeLinux:
>
> +      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
>
> +        //
>
> +        // If it's not a graphic character convert Unicode to ASCII.
>
> +        //
>
> +          GraphicChar = (CHAR8)*WString;


Please help to fix the space indent (extra spaces) here to keep it aligned with the contexts.


>
> +
>
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
>
> +          //
>
> +          // when this driver use the OutputString to output control string,
>
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
>
> +          // to be output to the terminal emulation software.
>
> +          //
>
> +          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
>
> +            GraphicChar = 27;
>
> +          } else {
>
> +            GraphicChar = '?';
>
> +            Warning     = TRUE;
>
> +          }
>
> +        }
>
> +
>
> +        AsciiChar = GraphicChar;
>
> +      }
>
> +      Length = 1;
>
> +      Status = TerminalDevice->SerialIo->Write (
>
> +                                          TerminalDevice->SerialIo,
>
> +                                          &Length,
>
> +                                          &GraphicChar
>
> +                                          );


Please help to add an extra space for the above 3 lines for space indent coding style.


>
> +
>
> +      if (EFI_ERROR (Status)) {
>
> +        goto OutputError;
>
> +      }
>
> +
>
> +      break;
>
>      case TerminalTypeXtermR6:
>
> -    case TerminalTypeVt400:
>
>      case TerminalTypeSCO:
>


For the below added codes (for case TerminalTypeXtermR6 & TerminalTypeSCO), please help to fix below coding style issues:
a) Codes within a 'case' or 'if-else' statement is recommended to add 2 spaces for indent. (The space indent of the below chunk of codes are messy, which impacts the readability.)
b) For multi-line function calls, the input parameters (including the closing bracket) are recommended to add 2 spaces from the function name in the 1st line.
c) Please update multi-line comment to align with style:
//
// Comments...
//


> +        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)
> && !TerminalIsValidAscii (*WString)) {
>
> +          //Box graphics are split into 2 types simple and advanced
>
> +          //simple are drawn with dec special graphics
>
> +          //advanced are drawn with utf8
>
> +          //This checks for simple because they have a lower value than the
> advanced
>
> +          if(*WString < BOXDRAW_DOUBLE_HORIZONTAL) {
>
> +            if (!TerminalDevice->DecSpecialGraphicsMode) {
>
> +              ValidBytes = 0;
>
> +              ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> +              Status = TerminalDevice->SerialIo->Write (
>
> +                                                   TerminalDevice->SerialIo,
>
> +                                                   &ModeSwitchLength,
>
> +                                                   (UINT8 *)SetDecModeString
>
> +                                          );
>
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
>
> +        }
>
>
>
> -      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
>
> +        GraphicChar = DecChar;
>
> +        Length = 1;
>
> +          } else {
>
> +            if (TerminalDevice->DecSpecialGraphicsMode) {
>
> +              ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> +              Status = TerminalDevice->SerialIo->Write (
>
> +                                                   TerminalDevice->SerialIo,
>
> +                                                   &ModeSwitchLength,
>
> +                                                   (UINT8 *)ExitDecModeString
>
> +                                                );
>
> +                if (EFI_ERROR (Status)) {
>
> +                  goto OutputError;
>
> +                }
>
> +
>
> +            TerminalDevice->DecSpecialGraphicsMode = FALSE;
>
> +          }
>
> +
>
> +          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
>
> +          Length = ValidBytes;
>
> +        }
>
> +        } else {
>
> +          if (TerminalDevice->DecSpecialGraphicsMode) {
>
> +          Status = TerminalDevice->SerialIo->Write (
>
> +                                          TerminalDevice->SerialIo,
>
> +                                          &ModeSwitchLength,
>
> +                                          (UINT8 *)ExitDecModeString
>
> +                                          );
>
> +          if (EFI_ERROR (Status)) {
>
> +            goto OutputError;
>
> +          }
>
> +
>
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
>
> +        }
>
> +
>
> +          GraphicChar = (CHAR8)*WString;
>
> +
>
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
>
> +          //
>
> +          // when this driver use the OutputString to output control string,
>
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
>
> +          // to be output to the terminal emulation software.
>
> +          //
>
> +            if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
>
> +              GraphicChar = 27;
>
> +              Length = 1;
>
> +            } else {
>
> +          UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
>
> +          Length = ValidBytes;
>
> +            }
>
> +          } else {
>
> +        Length = 1;
>
> +      }
>
> +      }
>
> +
>
> +        if (ValidBytes) {
>
> +        Status = TerminalDevice->SerialIo->Write (
>
> +                                        TerminalDevice->SerialIo,
>
> +                                        &Length,
>
> +                                               (UINT8 *)&Utf8Char
>
> +                                        );
>
> +          ValidBytes = 0;
>
> +        } else {
>
> +      Status = TerminalDevice->SerialIo->Write (
>
> +                                      TerminalDevice->SerialIo,
>
> +                                      &Length,
>
> +                                      &GraphicChar
>
> +                                      );
>
> +        }
>
> +      if (EFI_ERROR (Status)) {
>
> +        goto OutputError;
>
> +      }
>
> +
>
> +      break;
>
> +    case TerminalTypeVt100:
>
> +    case TerminalTypeTtyTerm:
>
> +      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
>
>          //
>
>          // If it's not a graphic character convert Unicode to ASCII.
>
>          //
>
> @@ -248,12 +370,9 @@ TerminalConOutOutputString (
>          }
>
>
>
>          AsciiChar = GraphicChar;
>
> -
>
>        }
>
>
>
> -      if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {
>
> -        GraphicChar = AsciiChar;
>
> -      }
>
> +      GraphicChar = AsciiChar;
>
>
>
>        Length = 1;
>
>
>
> @@ -267,8 +386,67 @@ TerminalConOutOutputString (
>          goto OutputError;
>
>        }
>
>
>
> -      break;
>
> +        break;
>
> +    case TerminalTypeVt100Plus:
>
> +    case TerminalTypeVt400:


For the below added codes (for case TerminalTypeVt100Plus & TerminalTypeVt400), please help to fix below coding style issues:
a) Codes within a 'case' or 'if-else' statement is recommended to add 2 spaces for indent. (The space indent of the below chunk of codes are messy, which impacts the readability.)
b) For multi-line function calls, the input parameters (including the closing bracket) are recommended to add 2 spaces from the function name in the 1st line.

Best Regards,
Hao Wu


>
> +        Length = 1;
>
> +        if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)) {
>
> +          if (!TerminalDevice->DecSpecialGraphicsMode) {
>
> +            ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> +            Status = TerminalDevice->SerialIo->Write (
>
> +                                                 TerminalDevice->SerialIo,
>
> +                                                 &ModeSwitchLength,
>
> +                                                 (UINT8 *)SetDecModeString
>
> +                                          );
>
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
>
> +        }
>
>
>
> +        GraphicChar = DecChar;
>
> +        } else {
>
> +          if (TerminalDevice->DecSpecialGraphicsMode) {
>
> +            ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> +            Status = TerminalDevice->SerialIo->Write (
>
> +                                                 TerminalDevice->SerialIo,
>
> +                                                 &ModeSwitchLength,
>
> +                                                 (UINT8 *)ExitDecModeString
>
> +                                          );
>
> +
>
> +          if (EFI_ERROR (Status)) {
>
> +            goto OutputError;
>
> +          }
>
> +
>
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
>
> +        }
>
> +
>
> +          GraphicChar = (CHAR8)*WString;
>
> +
>
> +        if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
>
> +          //
>
> +          // when this driver use the OutputString to output control string,
>
> +          // TerminalDevice->OutputEscChar is set to let the Esc char
>
> +          // to be output to the terminal emulation software.
>
> +          //
>
> +          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
>
> +            GraphicChar = 27;
>
> +          } else {
>
> +            GraphicChar = '?';
>
> +            Warning     = TRUE;
>
> +          }
>
> +        }
>
> +      }
>
> +
>
> +    Status = TerminalDevice->SerialIo->Write (
>
> +                                      TerminalDevice->SerialIo,
>
> +                                      &Length,
>
> +                                      &GraphicChar
>
> +                                      );
>
> +
>
> +      if (EFI_ERROR (Status)) {
>
> +        goto OutputError;
>
> +      }
>
> +
>
> +        break;
>
> +    case TerminalTypeLinux:
>
>      case TerminalTypeVtUtf8:
>
>        UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
>
>        Length = ValidBytes;
>
> @@ -280,8 +458,10 @@ TerminalConOutOutputString (
>        if (EFI_ERROR (Status)) {
>
>          goto OutputError;
>
>        }
>
> +
>
>        break;
>
>      }
>
> +
>
>      //
>
>      //  Update cursor position.
>
>      //
>
> @@ -875,7 +1055,8 @@ BOOLEAN
>  TerminalIsValidTextGraphics (
>
>    IN  CHAR16  Graphic,
>
>    OUT CHAR8   *PcAnsi, OPTIONAL
>
> -  OUT CHAR8   *Ascii OPTIONAL
>
> +  OUT CHAR8   *Ascii, OPTIONAL
>
> +  OUT CHAR8   *DecSpecialGraphics OPTIONAL
>
>    )
>
>  {
>
>    UNICODE_TO_CHAR *Table;
>
> @@ -897,6 +1078,9 @@ TerminalIsValidTextGraphics (
>        if (Ascii != NULL) {
>
>          *Ascii = Table->Ascii;
>
>        }
>
> +      if (DecSpecialGraphics != NULL){
>
> +        *DecSpecialGraphics = Table->DecSpecialGraphics;
>
> +      }
>
>
>
>        return TRUE;
>
>      }
>
> --
> 2.32.0


[-- Attachment #2: Type: text/html, Size: 51881 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-13  1:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1627356917.git.cadenkline9@gmail.com>
2021-07-30  2:45 ` [PATCH 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters Caden Kline
2021-08-03  0:04   ` Wu, Hao A
2021-08-05 23:18   ` [edk2-devel] " Nate DeSimone
2021-08-09  5:56   ` Wu, Hao A
     [not found]     ` <CAC3Y4+Q+50YEwFkr+SuJ18x7=FJbXP_K4Z56Ex9dS3=8TDfWbg@mail.gmail.com>
2021-08-13  1:18       ` Wu, Hao A
2021-08-13  1:20         ` [edk2-devel] " Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox