From a2e9a7d48a0638dacc6ac25db799d07f9f06ea1c Mon Sep 17 00:00:00 2001 From: MarsDoge Date: Sat, 23 May 2026 14:38:37 +0800 Subject: [PATCH] phase34(display-engine): harden row rendering state model --- CHANGELOG.md | 5 + .../CustomizedDisplayLibInternal.c | 31 ++-- .../ModernDisplayFormModel.c | 162 +++++++++++++++++- .../ModernDisplayFormModel.h | 67 +++++++- Tests/Smoke/smoke_validate.py | 27 ++- .../ModernDisplayEngineDxe/FormDisplay.c | 16 +- .../ModernDisplayEngineDxe/FormDisplay.h | 19 +- 7 files changed, 280 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3b0a45..09a6506 100644 --- a/CHANGELOG.md +++ b/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, diff --git a/Library/ModernUiCustomizedDisplayLib/CustomizedDisplayLibInternal.c b/Library/ModernUiCustomizedDisplayLib/CustomizedDisplayLibInternal.c index 31bdb01..88fa582 100644 --- a/Library/ModernUiCustomizedDisplayLib/CustomizedDisplayLibInternal.c +++ b/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); diff --git a/Library/ModernUiCustomizedDisplayLib/ModernDisplayFormModel.c b/Library/ModernUiCustomizedDisplayLib/ModernDisplayFormModel.c index 8841e6a..3afca40 100644 --- a/Library/ModernUiCustomizedDisplayLib/ModernDisplayFormModel.c +++ b/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; } diff --git a/Library/ModernUiCustomizedDisplayLib/ModernDisplayFormModel.h b/Library/ModernUiCustomizedDisplayLib/ModernDisplayFormModel.h index 2b28b9e..0e799df 100644 --- a/Library/ModernUiCustomizedDisplayLib/ModernDisplayFormModel.h +++ b/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. diff --git a/Tests/Smoke/smoke_validate.py b/Tests/Smoke/smoke_validate.py index 77e27d3..79125fb 100755 --- a/Tests/Smoke/smoke_validate.py +++ b/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]: diff --git a/Universal/ModernDisplayEngineDxe/FormDisplay.c b/Universal/ModernDisplayEngineDxe/FormDisplay.c index a168aed..d589880 100644 --- a/Universal/ModernDisplayEngineDxe/FormDisplay.c +++ b/Universal/ModernDisplayEngineDxe/FormDisplay.c @@ -2423,8 +2423,7 @@ DisplayOneMenu ( CHAR16 AdjustValue; UINTN MaxRow; UINTN RowWidth; - BOOLEAN IsActionRow; - BOOLEAN IsSubtitleRow; + Statement = MenuOption->ThisTag; Temp = SkipLine; @@ -2435,26 +2434,19 @@ DisplayOneMenu ( OptionLineNum = 0; MaxRow = 0; IsProcessingFirstRow = TRUE; - IsActionRow = (BOOLEAN)( - (Statement->OpCode->OpCode == EFI_IFR_ACTION_OP) || - (Statement->OpCode->OpCode == EFI_IFR_REF_OP) || - (Statement->OpCode->OpCode == EFI_IFR_RESET_BUTTON_OP) - ); - IsSubtitleRow = (BOOLEAN)(Statement->OpCode->OpCode == EFI_IFR_SUBTITLE_OP); - // // Set default color. // SetDisplayAttribute (MenuOption, FALSE); RowWidth = (gPromptBlockWidth + gOptionBlockWidth + LEFT_SKIPPED_COLUMNS); ModernDisplayDrawStatementRow ( + gFormData, + Statement, BeginCol, MenuOption->Row, RowWidth, Highlight, - MenuOption->GrayOut, - IsActionRow, - IsSubtitleRow + (BOOLEAN)(gUserInput->SelectedStatement == Statement) ); // diff --git a/Universal/ModernDisplayEngineDxe/FormDisplay.h b/Universal/ModernDisplayEngineDxe/FormDisplay.h index d9c9468..4024025 100644 --- a/Universal/ModernDisplayEngineDxe/FormDisplay.h +++ b/Universal/ModernDisplayEngineDxe/FormDisplay.h @@ -77,27 +77,28 @@ extern BOOLEAN gMisMatch; /** Draw a ModernSetup row background for one FormBrowser statement. - The DisplayEngine owns statement semantics. The customized display library - owns the GOP painting details behind the text-grid output. + The DisplayEngine supplies already-materialized form/statement data. The + customized display library owns the private row model and GOP painting details + behind the text-grid output. + @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 ); //