phase34(display-engine): harden row rendering state model

CHANGELOG.md

@@ -14,6 +14,11 @@ this file as both a release log and a lightweight development progress record.
 
 ### Added
 
+- Phase34 DisplayEngine row rendering hardening: statement row GOP surfaces now
+  classify highlighted/selected, disabled/locked, read-only, changed, invalid,
+  modal, and action/text affordance state through the private FormModel helpers,
+  keeping native FormBrowser HII semantics intact while reducing scattered visual
+  role decisions.
 - Phase33 DisplayEngine form view-model foundation: `ModernUiCustomizedDisplayLib`
   now has a private `ModernDisplayFormModel` layer that classifies borrowed
   FormBrowser DisplayEngine form/statement data into Modern UI row metadata,

Library/ModernUiCustomizedDisplayLib/CustomizedDisplayLibInternal.c

@@ -401,26 +401,27 @@ ModernDisplayDrawPopupSurface (
   Draw a ModernSetup row background for one FormBrowser statement.
 
   The caller still prints all statement text through the native DisplayEngine
-  flow. This hook only paints the GOP row surface beneath that text.
+  flow. This hook classifies the already-materialized statement with the private
+  form row model and paints the GOP row surface beneath that text.
 
+  @param[in] FormData   DisplayEngine form that owns Statement. May be NULL.
+  @param[in] Statement  Statement to classify for row rendering. May be NULL.
   @param[in] Column     Text-grid column where the row starts.
   @param[in] Row        Text-grid row to paint.
   @param[in] Width      Text-grid column count to paint.
-  @param[in] Highlight  TRUE when the row is selected.
-  @param[in] GrayOut    TRUE when the statement is disabled or grayed.
-  @param[in] Action     TRUE when the statement is an action-like row.
-  @param[in] Subtitle   TRUE when the statement is a subtitle row.
+  @param[in] Highlight  TRUE when the row has keyboard highlight.
+  @param[in] Selected   TRUE when the row is in edit/selection mode.
 **/
 VOID
 EFIAPI
 ModernDisplayDrawStatementRow (
+  IN FORM_DISPLAY_ENGINE_FORM       *FormData OPTIONAL,
+  IN FORM_DISPLAY_ENGINE_STATEMENT  *Statement OPTIONAL,
   IN UINTN    Column,
   IN UINTN    Row,
   IN UINTN    Width,
   IN BOOLEAN  Highlight,
-  IN BOOLEAN  GrayOut,
-  IN BOOLEAN  Action,
-  IN BOOLEAN  Subtitle
+  IN BOOLEAN  Selected
   )
 {
   CONST MODERN_UI_THEME            *Theme;
@@ -431,6 +432,7 @@ ModernDisplayDrawStatementRow (
   UINTN                            PixelWidth;
   MODERN_UI_RECT                   RowRect;
   MODERN_UI_ROW_MODEL              RowModel;
+  MODERN_DISPLAY_FORM_ROW          FormRow;
 
   if ((Width == 0) || EFI_ERROR (ModernDisplayEnsureRenderer ())) {
     return;
@@ -447,16 +449,9 @@ ModernDisplayDrawStatementRow (
   RowModel.Prompt    = NULL;
   RowModel.Value     = NULL;
   RowModel.ValueType = ModernUiValueNone;
-  if (Highlight) {
-    RowModel.Role = ModernUiRowSelected;
-  } else if (GrayOut) {
-    RowModel.Role = ModernUiRowDisabled;
-  } else if (Subtitle) {
-    RowModel.Role = ModernUiRowSubtitle;
-  } else if (Action) {
-    RowModel.Role = ModernUiRowAction;
-  } else {
-    RowModel.Role = ModernUiRowNormal;
+  RowModel.Role      = ModernUiRowNormal;
+  if (!EFI_ERROR (ModernDisplayClassifyStatementForForm (FormData, Statement, Highlight, Selected, &FormRow))) {
+    RowModel.Role = ModernDisplayFormRowGetVisualRole (&FormRow);
   }
 
   ModernUiEngineDrawRows (&mModernRenderContext, &RowModel, 1, Theme);

Library/ModernUiCustomizedDisplayLib/ModernDisplayFormModel.c

@@ -176,6 +176,116 @@ ModernDisplayFormRowIsActionLike (
                     );
 }
 
+/**
+  Return whether a row kind behaves as editable value content.
+
+  @param[in] Kind  Row kind to test.
+
+  @retval TRUE   The row represents editable value content.
+  @retval FALSE  The row is text/action/chrome-like only.
+**/
+BOOLEAN
+ModernDisplayFormRowIsEditable (
+  IN MODERN_DISPLAY_FORM_ROW_KIND  Kind
+  )
+{
+  return (BOOLEAN)(
+                    ModernDisplayFormRowIsChoiceLike (Kind) ||
+                    (Kind == ModernDisplayFormRowCheckbox) ||
+                    (Kind == ModernDisplayFormRowPassword) ||
+                    (Kind == ModernDisplayFormRowString)
+                    );
+}
+
+/**
+  Return whether a row kind has no value/action affordance.
+
+  @param[in] Kind  Row kind to test.
+
+  @retval TRUE   The row is display text only.
+  @retval FALSE  The row has another affordance.
+**/
+BOOLEAN
+ModernDisplayFormRowIsTextOnly (
+  IN MODERN_DISPLAY_FORM_ROW_KIND  Kind
+  )
+{
+  return (BOOLEAN)(
+                    (Kind == ModernDisplayFormRowText) ||
+                    (Kind == ModernDisplayFormRowSubtitle) ||
+                    (Kind == ModernDisplayFormRowUnknown)
+                    );
+}
+
+/**
+  Return whether a BrowserStatus value indicates the current page has invalid
+  or blocked input feedback to show.
+
+  @param[in] BrowserStatus  DisplayEngine BrowserStatus value.
+
+  @retval TRUE   The status represents invalid/warning/no-submit feedback.
+  @retval FALSE  The status is success or non-row-specific action feedback.
+**/
+STATIC
+BOOLEAN
+ModernDisplayBrowserStatusIsInvalid (
+  IN UINT32  BrowserStatus
+  )
+{
+  switch (BrowserStatus) {
+    case BROWSER_WARNING_IF:
+    case BROWSER_NO_SUBMIT_IF:
+    case BROWSER_INCONSISTENT_IF:
+    case BROWSER_SUBMIT_FAIL:
+      return TRUE;
+    default:
+      return FALSE;
+  }
+}
+
+/**
+  Map a private form row model to the shared Modern UI renderer row role.
+
+  @param[in] Row  Row model to inspect. May be NULL.
+
+  @return Renderer row role for conservative DisplayEngine row painting.
+**/
+MODERN_UI_ROW_ROLE
+ModernDisplayFormRowGetVisualRole (
+  IN CONST MODERN_DISPLAY_FORM_ROW  *Row OPTIONAL
+  )
+{
+  if (Row == NULL) {
+    return ModernUiRowNormal;
+  }
+
+  if ((Row->State & ModernDisplayFormRowStateInvalid) != 0) {
+    return ModernUiRowWarning;
+  }
+
+  if ((Row->State & ModernDisplayFormRowStateHighlighted) != 0) {
+    return ModernUiRowSelected;
+  }
+
+  if ((Row->State & ModernDisplayFormRowStateDisabled) != 0) {
+    return ModernUiRowDisabled;
+  }
+
+  if ((Row->State & ModernDisplayFormRowStateReadOnly) != 0) {
+    return ModernUiRowReadOnly;
+  }
+
+  if (Row->Kind == ModernDisplayFormRowSubtitle) {
+    return ModernUiRowSubtitle;
+  }
+
+  if (ModernDisplayFormRowIsActionLike (Row->Kind) && !ModernDisplayFormRowIsTextOnly (Row->Kind)) {
+    return ModernUiRowAction;
+  }
+
+  return ModernUiRowNormal;
+}
+
 /**
   Classify a DisplayEngine statement into a private Modern UI row model.
 
@@ -193,6 +303,33 @@ ModernDisplayClassifyStatement (
   IN  BOOLEAN                        Selected,
   OUT MODERN_DISPLAY_FORM_ROW        *Row
   )
+{
+  return ModernDisplayClassifyStatementForForm (NULL, Statement, FALSE, Selected, Row);
+}
+
+/**
+  Classify a DisplayEngine statement into a private Modern UI row model with
+  optional page/form context.
+
+  @param[in]  FormData   DisplayEngine form that owns the statement. May be
+                         NULL when only statement-local state is available.
+  @param[in]  Statement  DisplayEngine statement to classify. May be NULL.
+  @param[in]  Highlight  TRUE when the row has keyboard highlight.
+  @param[in]  Selected   TRUE when the row is in edit/selection mode.
+  @param[out] Row        Row model to fill. Must not be NULL.
+
+  @retval EFI_SUCCESS            Row was filled. A NULL Statement produces an
+                                  Unknown row.
+  @retval EFI_INVALID_PARAMETER  Row is NULL.
+**/
+EFI_STATUS
+ModernDisplayClassifyStatementForForm (
+  IN  FORM_DISPLAY_ENGINE_FORM       *FormData OPTIONAL,
+  IN  FORM_DISPLAY_ENGINE_STATEMENT  *Statement OPTIONAL,
+  IN  BOOLEAN                        Highlight,
+  IN  BOOLEAN                        Selected,
+  OUT MODERN_DISPLAY_FORM_ROW        *Row
+  )
 {
   UINT32  State;
 
@@ -206,9 +343,20 @@ ModernDisplayClassifyStatement (
   Row->OpCode    = ((Statement != NULL) && (Statement->OpCode != NULL)) ? Statement->OpCode->OpCode : 0;
 
   State = 0;
+  ModernDisplayAddRowState (&State, ModernDisplayFormRowStateHighlighted, Highlight);
   ModernDisplayAddRowState (&State, ModernDisplayFormRowStateSelected, Selected);
+  if (FormData != NULL) {
+    ModernDisplayAddRowState (&State, ModernDisplayFormRowStateModal, (BOOLEAN)((FormData->Attribute & HII_DISPLAY_MODAL) != 0));
+    ModernDisplayAddRowState (&State, ModernDisplayFormRowStatePageChanged, FormData->SettingChangedFlag);
+    ModernDisplayAddRowState (
+      &State,
+      ModernDisplayFormRowStateInvalid,
+      (BOOLEAN)(Highlight && ModernDisplayBrowserStatusIsInvalid (FormData->BrowserStatus))
+      );
+  }
+
   if (Statement != NULL) {
-    ModernDisplayAddRowState (&State, ModernDisplayFormRowStateDisabled, (BOOLEAN)((Statement->Attribute & HII_DISPLAY_GRAYOUT) != 0));
+    ModernDisplayAddRowState (&State, ModernDisplayFormRowStateDisabled, (BOOLEAN)((Statement->Attribute & (HII_DISPLAY_GRAYOUT | HII_DISPLAY_LOCK)) != 0));
     ModernDisplayAddRowState (&State, ModernDisplayFormRowStateReadOnly, (BOOLEAN)((Statement->Attribute & HII_DISPLAY_READONLY) != 0));
     ModernDisplayAddRowState (&State, ModernDisplayFormRowStateChanged, Statement->SettingChangedFlag);
     ModernDisplayAddRowState (&State, ModernDisplayFormRowStateHexInput, ModernDisplayStatementUsesHexInput (Statement));
@@ -277,16 +425,18 @@ ModernDisplayFormModelBuild (
     Model->RowCount++;
   }
 
-  Status = ModernDisplayClassifyStatement (HighlightedStatement, Selected, &Model->HighlightedRow);
+  Status = ModernDisplayClassifyStatementForForm (
+             FormData,
+             HighlightedStatement,
+             Model->HasHighlightedRow,
+             Selected,
+             &Model->HighlightedRow
+             );
   if (EFI_ERROR (Status)) {
     ModernDisplayFormModelClear (Model);
     return Status;
   }
 
-  if (Model->HasHighlightedRow) {
-    Model->HighlightedRow.State |= ModernDisplayFormRowStateHighlighted;
-  }
-
   return EFI_SUCCESS;
 }
 

Library/ModernUiCustomizedDisplayLib/ModernDisplayFormModel.h

@@ -50,7 +50,10 @@ typedef enum {
   ModernDisplayFormRowStateReadOnly   = BIT3,
   ModernDisplayFormRowStateChanged    = BIT4,
   ModernDisplayFormRowStateHexInput   = BIT5,
-  ModernDisplayFormRowStateAdjustable = BIT6
+  ModernDisplayFormRowStateAdjustable = BIT6,
+  ModernDisplayFormRowStateInvalid    = BIT7,
+  ModernDisplayFormRowStateModal      = BIT8,
+  ModernDisplayFormRowStatePageChanged = BIT9
 } MODERN_DISPLAY_FORM_ROW_STATE;
 
 /**
@@ -104,6 +107,30 @@ ModernDisplayClassifyStatement (
   OUT MODERN_DISPLAY_FORM_ROW        *Row
   );
 
+/**
+  Classify a DisplayEngine statement into a private Modern UI row model with
+  optional page/form context.
+
+  @param[in]  FormData   DisplayEngine form that owns the statement. May be
+                         NULL when only statement-local state is available.
+  @param[in]  Statement  DisplayEngine statement to classify. May be NULL.
+  @param[in]  Highlight  TRUE when the row has keyboard highlight.
+  @param[in]  Selected   TRUE when the row is in edit/selection mode.
+  @param[out] Row        Row model to fill. Must not be NULL.
+
+  @retval EFI_SUCCESS            Row was filled. A NULL Statement produces an
+                                  Unknown row.
+  @retval EFI_INVALID_PARAMETER  Row is NULL.
+**/
+EFI_STATUS
+ModernDisplayClassifyStatementForForm (
+  IN  FORM_DISPLAY_ENGINE_FORM       *FormData OPTIONAL,
+  IN  FORM_DISPLAY_ENGINE_STATEMENT  *Statement OPTIONAL,
+  IN  BOOLEAN                        Highlight,
+  IN  BOOLEAN                        Selected,
+  OUT MODERN_DISPLAY_FORM_ROW        *Row
+  );
+
 /**
   Return whether a row kind uses choice/list-style key help.
 
@@ -130,6 +157,44 @@ ModernDisplayFormRowIsActionLike (
   IN MODERN_DISPLAY_FORM_ROW_KIND  Kind
   );
 
+/**
+  Return whether a row kind behaves as editable value content.
+
+  @param[in] Kind  Row kind to test.
+
+  @retval TRUE   The row represents editable value content.
+  @retval FALSE  The row is text/action/chrome-like only.
+**/
+BOOLEAN
+ModernDisplayFormRowIsEditable (
+  IN MODERN_DISPLAY_FORM_ROW_KIND  Kind
+  );
+
+/**
+  Return whether a row kind has no value/action affordance.
+
+  @param[in] Kind  Row kind to test.
+
+  @retval TRUE   The row is display text only.
+  @retval FALSE  The row has another affordance.
+**/
+BOOLEAN
+ModernDisplayFormRowIsTextOnly (
+  IN MODERN_DISPLAY_FORM_ROW_KIND  Kind
+  );
+
+/**
+  Map a private form row model to the shared Modern UI renderer row role.
+
+  @param[in] Row  Row model to inspect. May be NULL.
+
+  @return Renderer row role for conservative DisplayEngine row painting.
+**/
+MODERN_UI_ROW_ROLE
+ModernDisplayFormRowGetVisualRole (
+  IN CONST MODERN_DISPLAY_FORM_ROW  *Row OPTIONAL
+  );
+
 /**
   Build a private Modern UI view model from DisplayEngine form data.
 

Tests/Smoke/smoke_validate.py

@@ -1931,7 +1931,32 @@ def check_phase33_display_form_view_model_boundary(root: Path) -> list[str]:
         if token not in refresh_body:
             raise SmokeFailure(f"RefreshKeyHelp must consume the Phase33 row model/helper: {token}")
 
-    return ["PASS Phase33 private DisplayEngine form view-model boundary"]
+    row_surface_body = extract_c_function_body(
+        strip_c_comments(internal_c.read_text(encoding="utf-8")),
+        "ModernDisplayDrawStatementRow",
+    )
+    for token in (
+        "ModernDisplayClassifyStatementForForm",
+        "ModernDisplayFormRowGetVisualRole",
+        "MODERN_DISPLAY_FORM_ROW",
+    ):
+        if token not in row_surface_body:
+            raise SmokeFailure(f"DisplayEngine row surface must consume the Phase34 row model/helper: {token}")
+
+    form_display = root / "Universal" / "ModernDisplayEngineDxe" / "FormDisplay.c"
+    form_display_text = strip_c_comments(form_display.read_text(encoding="utf-8"))
+    display_one_menu = extract_c_function_body(form_display_text, "DisplayOneMenu")
+    if "ModernDisplayDrawStatementRow" not in display_one_menu:
+        raise SmokeFailure("DisplayOneMenu must keep the Modern row surface hook")
+    for stale_token in ("IsActionRow", "IsSubtitleRow"):
+        if stale_token in display_one_menu:
+            raise SmokeFailure(f"DisplayOneMenu must not reintroduce scattered row role state: {stale_token}")
+
+    for token in ("ConfigAccess", "RouteConfig", "ExtractConfig", "SetVariable", "HiiSetBrowserData"):
+        if token in row_surface_body:
+            raise SmokeFailure(f"Phase34 row rendering hook contains prohibited browser/storage token: {token}")
+
+    return ["PASS Phase33/34 private DisplayEngine form view-model boundary"]
 
 
 def check_modern_ui_builtin_glyph_subset(root: Path) -> list[str]: