public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/1] MdeModulePkg/Console: Improve encoding of box drawing characters
@ 2021-08-23  2:44 Caden Kline
  2021-08-23  2:44 ` [PATCH v3 1/1] " Caden Kline
  0 siblings, 1 reply; 4+ messages in thread
From: Caden Kline @ 2021-08-23  2:44 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Hao A Wu, Zhichao Gao, Ray Ni, Nate DeSimone

Improved encoding of box drawing characters for different terminal types.
This includes Dec special graphics mode and more utf8.
According to this table https://github.com/tianocore/tianocore.github.io/wiki/Tasks-Terminal-driver-improvements 

v1 mail: https://edk2.groups.io/g/devel/message/78390 
v1 github: https://github.com/Pokemod97/edk2/tree/terminal-driver-characters
v2 mail: https://edk2.groups.io/g/devel/message/79214 
v2 github: https://github.com/Pokemod97/edk2/tree/terminal-driver-characters-v2
v3 github: https://github.com/Pokemod97/edk2/tree/terminal-driver-characters-v3

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>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

Caden Kline (1):
  MdeModulePkg/Console: Improve encoding of box drawing characters

 MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h       |  26 +-
 MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c           |   2 +-
 MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c | 295 +++++++++++++++-----
 3 files changed, 243 insertions(+), 80 deletions(-)

-- 
2.33.0


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

* [PATCH v3 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
  2021-08-23  2:44 [PATCH v3 0/1] MdeModulePkg/Console: Improve encoding of box drawing characters Caden Kline
@ 2021-08-23  2:44 ` Caden Kline
  2021-08-26 21:53   ` Nate DeSimone
  2021-09-09  3:28   ` Gao, Zhichao
  0 siblings, 2 replies; 4+ messages in thread
From: Caden Kline @ 2021-08-23  2:44 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Hao A Wu, Zhichao Gao, Ray Ni, Nate DeSimone

Improved encoding of box drawing characters for different terminal types.
This includes Dec special graphics mode and more utf8.
Changes are made according to the below issue

https://bugzilla.tianocore.org/show_bug.cgi?id=3580

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>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
 MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h       |  26 +-
 MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c           |   2 +-
 MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c | 295 +++++++++++++++-----
 3 files changed, 243 insertions(+), 80 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 360e58e84743..1eab439531dc 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -122,7 +122,12 @@ typedef struct {
   EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;
   LIST_ENTRY                          NotifyList;
   EFI_EVENT                           KeyNotifyProcessEvent;
+  BOOLEAN                             DecSpecialGraphicsMode;
 } TERMINAL_DEV;
+//
+// 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 +174,7 @@ typedef struct {
   UINT16  Unicode;
   CHAR8   PcAnsi;
   CHAR8   Ascii;
+  CHAR8   DecSpecialGraphics;
 } UNICODE_TO_CHAR;
 
 //
@@ -1367,20 +1373,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..7b328162325e 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,104 @@ 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 == ESC) && TerminalDevice->OutputEscChar) {
+            GraphicChar = ESC;
+          } 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:
+      //
+      // 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 (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar) && *WString < BOXDRAW_DOUBLE_HORIZONTAL) {
+        if (!TerminalDevice->DecSpecialGraphicsMode) {
+          ModeSwitchLength = LENGTH_DEC_ESCAPE;
+          Status = TerminalDevice->SerialIo->Write (
+                                              TerminalDevice->SerialIo,
+                                              &ModeSwitchLength,
+                                              (UINT8 *)SetDecModeString
+                                              );
+          if (EFI_ERROR (Status)) {
+            goto OutputError;
+          }
+          TerminalDevice->DecSpecialGraphicsMode = TRUE;
+        }
 
-      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
+        GraphicChar = DecChar;
+        Length = 1;
+      } else {
+        if (TerminalDevice->DecSpecialGraphicsMode) {
+          Status = TerminalDevice->SerialIo->Write (
+                                              TerminalDevice->SerialIo,
+                                              &ModeSwitchLength,
+                                              (UINT8 *)ExitDecModeString
+                                              );
+          if (EFI_ERROR (Status)) {
+            goto OutputError;
+          }
+
+          TerminalDevice->DecSpecialGraphicsMode = FALSE;
+        }
+        UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
+        Length = ValidBytes;
+      }
+
+      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.
         //
@@ -239,8 +329,8 @@ TerminalConOutOutputString (
           // TerminalDevice->OutputEscChar is set to let the Esc char
           // to be output to the terminal emulation software.
           //
-          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
-            GraphicChar = 27;
+          if ((GraphicChar == ESC) && TerminalDevice->OutputEscChar) {
+            GraphicChar = ESC;
           } else {
             GraphicChar = '?';
             Warning     = TRUE;
@@ -248,14 +338,73 @@ TerminalConOutOutputString (
         }
 
         AsciiChar = GraphicChar;
-
       }
 
-      if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {
-        GraphicChar = AsciiChar;
+      GraphicChar = AsciiChar;
+
+      Length = 1;
+
+      Status = TerminalDevice->SerialIo->Write (
+                                          TerminalDevice->SerialIo,
+                                          &Length,
+                                          &GraphicChar
+                                          );
+
+      if (EFI_ERROR (Status)) {
+        goto OutputError;
       }
 
+        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
+                                              );
+          if (EFI_ERROR (Status)) {
+            goto OutputError;
+          }                                    
+          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 == ESC) && TerminalDevice->OutputEscChar) {
+            GraphicChar = ESC;
+          } else {
+            GraphicChar = '?';
+            Warning     = TRUE;
+          }
+        }
+      }
 
       Status = TerminalDevice->SerialIo->Write (
                                           TerminalDevice->SerialIo,
@@ -268,7 +417,7 @@ TerminalConOutOutputString (
       }
 
       break;
-
+    case TerminalTypeLinux:
     case TerminalTypeVtUtf8:
       UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
       Length = ValidBytes;
@@ -280,8 +429,10 @@ TerminalConOutOutputString (
       if (EFI_ERROR (Status)) {
         goto OutputError;
       }
+
       break;
     }
+
     //
     //  Update cursor position.
     //
@@ -875,7 +1026,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 +1049,9 @@ TerminalIsValidTextGraphics (
       if (Ascii != NULL) {
         *Ascii = Table->Ascii;
       }
+      if (DecSpecialGraphics != NULL){
+        *DecSpecialGraphics = Table->DecSpecialGraphics;
+      }
 
       return TRUE;
     }
-- 
2.33.0


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

* Re: [PATCH v3 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
  2021-08-23  2:44 ` [PATCH v3 1/1] " Caden Kline
@ 2021-08-26 21:53   ` Nate DeSimone
  2021-09-09  3:28   ` Gao, Zhichao
  1 sibling, 0 replies; 4+ messages in thread
From: Nate DeSimone @ 2021-08-26 21:53 UTC (permalink / raw)
  To: Caden Kline, devel@edk2.groups.io
  Cc: Wang, Jian J, Wu, Hao A, Gao, Zhichao, Ni, Ray

HI Caden,

You accidentally added a bunch of trailing white space on line 371, but that should be fixable during the check-in process.

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

-----Original Message-----
From: Caden Kline <cadenkline9@gmail.com> 
Sent: Sunday, August 22, 2021 7:44 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>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: [PATCH v3 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.
Changes are made according to the below issue

https://bugzilla.tianocore.org/show_bug.cgi?id=3580

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>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
 MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h       |  26 +-
 MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c           |   2 +-
 MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c | 295 +++++++++++++++-----
 3 files changed, 243 insertions(+), 80 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 360e58e84743..1eab439531dc 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -122,7 +122,12 @@ typedef struct {
   EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;

   LIST_ENTRY                          NotifyList;

   EFI_EVENT                           KeyNotifyProcessEvent;

+  BOOLEAN                             DecSpecialGraphicsMode;

 } TERMINAL_DEV;

+//

+// 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 +174,7 @@ typedef struct {
   UINT16  Unicode;

   CHAR8   PcAnsi;

   CHAR8   Ascii;

+  CHAR8   DecSpecialGraphics;

 } UNICODE_TO_CHAR;

 

 //

@@ -1367,20 +1373,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..7b328162325e 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,104 @@ 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 == ESC) && TerminalDevice->OutputEscChar) {

+            GraphicChar = ESC;

+          } 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:

+      //

+      // 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 (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar) && *WString < BOXDRAW_DOUBLE_HORIZONTAL) {

+        if (!TerminalDevice->DecSpecialGraphicsMode) {

+          ModeSwitchLength = LENGTH_DEC_ESCAPE;

+          Status = TerminalDevice->SerialIo->Write (

+                                              TerminalDevice->SerialIo,

+                                              &ModeSwitchLength,

+                                              (UINT8 *)SetDecModeString

+                                              );

+          if (EFI_ERROR (Status)) {

+            goto OutputError;

+          }

+          TerminalDevice->DecSpecialGraphicsMode = TRUE;

+        }

 

-      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {

+        GraphicChar = DecChar;

+        Length = 1;

+      } else {

+        if (TerminalDevice->DecSpecialGraphicsMode) {

+          Status = TerminalDevice->SerialIo->Write (

+                                              TerminalDevice->SerialIo,

+                                              &ModeSwitchLength,

+                                              (UINT8 *)ExitDecModeString

+                                              );

+          if (EFI_ERROR (Status)) {

+            goto OutputError;

+          }

+

+          TerminalDevice->DecSpecialGraphicsMode = FALSE;

+        }

+        UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);

+        Length = ValidBytes;

+      }

+

+      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.

         //

@@ -239,8 +329,8 @@ TerminalConOutOutputString (
           // TerminalDevice->OutputEscChar is set to let the Esc char

           // to be output to the terminal emulation software.

           //

-          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {

-            GraphicChar = 27;

+          if ((GraphicChar == ESC) && TerminalDevice->OutputEscChar) {

+            GraphicChar = ESC;

           } else {

             GraphicChar = '?';

             Warning     = TRUE;

@@ -248,14 +338,73 @@ TerminalConOutOutputString (
         }

 

         AsciiChar = GraphicChar;

-

       }

 

-      if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {

-        GraphicChar = AsciiChar;

+      GraphicChar = AsciiChar;

+

+      Length = 1;

+

+      Status = TerminalDevice->SerialIo->Write (

+                                          TerminalDevice->SerialIo,

+                                          &Length,

+                                          &GraphicChar

+                                          );

+

+      if (EFI_ERROR (Status)) {

+        goto OutputError;

       }

 

+        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

+                                              );

+          if (EFI_ERROR (Status)) {

+            goto OutputError;

+          }                                    

+          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 == ESC) && TerminalDevice->OutputEscChar) {

+            GraphicChar = ESC;

+          } else {

+            GraphicChar = '?';

+            Warning     = TRUE;

+          }

+        }

+      }

 

       Status = TerminalDevice->SerialIo->Write (

                                           TerminalDevice->SerialIo,

@@ -268,7 +417,7 @@ TerminalConOutOutputString (
       }

 

       break;

-

+    case TerminalTypeLinux:

     case TerminalTypeVtUtf8:

       UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);

       Length = ValidBytes;

@@ -280,8 +429,10 @@ TerminalConOutOutputString (
       if (EFI_ERROR (Status)) {

         goto OutputError;

       }

+

       break;

     }

+

     //

     //  Update cursor position.

     //

@@ -875,7 +1026,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 +1049,9 @@ TerminalIsValidTextGraphics (
       if (Ascii != NULL) {

         *Ascii = Table->Ascii;

       }

+      if (DecSpecialGraphics != NULL){

+        *DecSpecialGraphics = Table->DecSpecialGraphics;

+      }

 

       return TRUE;

     }

-- 
2.33.0


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

* Re: [PATCH v3 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
  2021-08-23  2:44 ` [PATCH v3 1/1] " Caden Kline
  2021-08-26 21:53   ` Nate DeSimone
@ 2021-09-09  3:28   ` Gao, Zhichao
  1 sibling, 0 replies; 4+ messages in thread
From: Gao, Zhichao @ 2021-09-09  3:28 UTC (permalink / raw)
  To: Caden Kline, devel@edk2.groups.io
  Cc: Wang, Jian J, Wu, Hao A, Ni, Ray, Desimone, Nathaniel L

Hi Caden,

The patch doesn't address all the comment I point out. If you think I am incorrect, please feel free to point out. See below:

> -----Original Message-----
> From: Caden Kline <cadenkline9@gmail.com>
> Sent: Monday, August 23, 2021 10:44 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>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Subject: [PATCH v3 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.
> Changes are made according to the below issue
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3580
> 
> 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>
> Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> ---
>  MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h       |  26 +-
>  MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c           |   2 +-
>  MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c | 295
> +++++++++++++++-----
>  3 files changed, 243 insertions(+), 80 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 360e58e84743..1eab439531dc 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -122,7 +122,12 @@ typedef struct {
>    EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;
> 
>    LIST_ENTRY                          NotifyList;
> 
>    EFI_EVENT                           KeyNotifyProcessEvent;
> 
> +  BOOLEAN                             DecSpecialGraphicsMode;
> 
>  } TERMINAL_DEV;
> 
> +//
> 
> +// 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 +174,7 @@ typedef struct {
>    UINT16  Unicode;
> 
>    CHAR8   PcAnsi;
> 
>    CHAR8   Ascii;
> 
> +  CHAR8   DecSpecialGraphics;
> 
>  } UNICODE_TO_CHAR;
> 
> 
> 
>  //
> 
> @@ -1367,20 +1373,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..7b328162325e 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,104 @@ 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 == ESC) && TerminalDevice->OutputEscChar) {
> 
> +            GraphicChar = ESC;
> 
> +          } else {
> 
> +            GraphicChar = '?';
> 
> +            Warning     = TRUE;
> 
> +          }
> 
> +        }
> 
> +
> 
> +        AsciiChar = GraphicChar;
>From my point, the above assignment is useless. If you think same with me, please remove it.
> 
> +      }
> 
> +      Length = 1;
> 
> +      Status = TerminalDevice->SerialIo->Write (
> 
> +                                          TerminalDevice->SerialIo,
> 
> +                                          &Length,
> 
> +                                          &GraphicChar
> 
> +                                          );
> 
> +
> 
> +      if (EFI_ERROR (Status)) {
> 
> +        goto OutputError;
> 
> +      }
> 
> +
> 
> +      break;
> 
>      case TerminalTypeXtermR6:
> 
> -    case TerminalTypeVt400:
> 
>      case TerminalTypeSCO:
> 
> +      //
> 
> +      // 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 (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar) &&
> *WString < BOXDRAW_DOUBLE_HORIZONTAL) {
> 
> +        if (!TerminalDevice->DecSpecialGraphicsMode) {
> 
> +          ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +          Status = TerminalDevice->SerialIo->Write (
> 
> +                                              TerminalDevice->SerialIo,
> 
> +                                              &ModeSwitchLength,
> 
> +                                              (UINT8 *)SetDecModeString
> 
> +                                              );
> 
> +          if (EFI_ERROR (Status)) {
> 
> +            goto OutputError;
> 
> +          }
> 
> +          TerminalDevice->DecSpecialGraphicsMode = TRUE;
> 
> +        }
> 
> 
> 
> -      if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
> 
> +        GraphicChar = DecChar;
> 
> +        Length = 1;
> 
> +      } else {
> 
> +        if (TerminalDevice->DecSpecialGraphicsMode) {
> 
The ModeSwitchLength should be initialized. Because it value would be changed in the loop.
> +          Status = TerminalDevice->SerialIo->Write (
> 
> +                                              TerminalDevice->SerialIo,
> 
> +                                              &ModeSwitchLength,
> 
> +                                              (UINT8 *)ExitDecModeString
> 
> +                                              );
> 
> +          if (EFI_ERROR (Status)) {
> 
> +            goto OutputError;
> 
> +          }
> 
> +
> 
> +          TerminalDevice->DecSpecialGraphicsMode = FALSE;
> 
> +        }
> 
> +        UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
> 
> +        Length = ValidBytes;
> 
> +      }
> 
> +
> 
> +      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.
> 
>          //
> 
> @@ -239,8 +329,8 @@ TerminalConOutOutputString (
>            // TerminalDevice->OutputEscChar is set to let the Esc char
> 
>            // to be output to the terminal emulation software.
> 
>            //
> 
> -          if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
> 
> -            GraphicChar = 27;
> 
> +          if ((GraphicChar == ESC) && TerminalDevice->OutputEscChar) {
> 
> +            GraphicChar = ESC;
> 
>            } else {
> 
>              GraphicChar = '?';
> 
>              Warning     = TRUE;
> 
> @@ -248,14 +338,73 @@ TerminalConOutOutputString (
>          }
> 
> 
> 
>          AsciiChar = GraphicChar;
> 
> -
> 
>        }
> 
> 
> 
> -      if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {
> 
> -        GraphicChar = AsciiChar;
> 
> +      GraphicChar = AsciiChar;
> 
> +
> 
> +      Length = 1;
> 
> +
> 
> +      Status = TerminalDevice->SerialIo->Write (
> 
> +                                          TerminalDevice->SerialIo,
> 
> +                                          &Length,
> 
> +                                          &GraphicChar
> 
> +                                          );
> 
> +
> 
> +      if (EFI_ERROR (Status)) {
> 
> +        goto OutputError;
> 
>        }
> 
> 
> 
> +        break;
Indent alignment.
> 
> +    case TerminalTypeVt100Plus:
> 
> +    case TerminalTypeVt400:
> 
>        Length = 1;
> 
> +      if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)) {
> 
> +        if (!TerminalDevice->DecSpecialGraphicsMode) {
The above check cause the BLOCKELEMENT_FULL_BLOCK to ARROW_DOWN characters to be write with switch string. Is that expected?
> 
> +          ModeSwitchLength = LENGTH_DEC_ESCAPE;
> 
> +          Status = TerminalDevice->SerialIo->Write (
> 
> +                                              TerminalDevice->SerialIo,
> 
> +                                              &ModeSwitchLength,
> 
> +                                              (UINT8 *)SetDecModeString
> 
> +                                              );
> 
> +          if (EFI_ERROR (Status)) {
> 
> +            goto OutputError;
> 
> +          }
It doesn't show on the email, but it has tailing whitespace in above line. Please use "python ./BaseTools/Scripts/PatchCheck.py  -1" to check the coding issue.

Thanks,
Zhichao
> 
> +          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 == ESC) && TerminalDevice->OutputEscChar) {
> 
> +            GraphicChar = ESC;
> 
> +          } else {
> 
> +            GraphicChar = '?';
> 
> +            Warning     = TRUE;
> 
> +          }
> 
> +        }
> 
> +      }
> 
> 
> 
>        Status = TerminalDevice->SerialIo->Write (
> 
>                                            TerminalDevice->SerialIo,
> 
> @@ -268,7 +417,7 @@ TerminalConOutOutputString (
>        }
> 
> 
> 
>        break;
> 
> -
> 
> +    case TerminalTypeLinux:
> 
>      case TerminalTypeVtUtf8:
> 
>        UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
> 
>        Length = ValidBytes;
> 
> @@ -280,8 +429,10 @@ TerminalConOutOutputString (
>        if (EFI_ERROR (Status)) {
> 
>          goto OutputError;
> 
>        }
> 
> +
> 
>        break;
> 
>      }
> 
> +
> 
>      //
> 
>      //  Update cursor position.
> 
>      //
> 
> @@ -875,7 +1026,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 +1049,9 @@ TerminalIsValidTextGraphics (
>        if (Ascii != NULL) {
> 
>          *Ascii = Table->Ascii;
> 
>        }
> 
> +      if (DecSpecialGraphics != NULL){
> 
> +        *DecSpecialGraphics = Table->DecSpecialGraphics;
> 
> +      }
> 
> 
> 
>        return TRUE;
> 
>      }
> 
> --
> 2.33.0


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

end of thread, other threads:[~2021-09-09  3:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-23  2:44 [PATCH v3 0/1] MdeModulePkg/Console: Improve encoding of box drawing characters Caden Kline
2021-08-23  2:44 ` [PATCH v3 1/1] " Caden Kline
2021-08-26 21:53   ` Nate DeSimone
2021-09-09  3:28   ` Gao, Zhichao

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