[PATCH 19/19] expo: Tidy up insets and improve tests
Simon Glass
sjg at chromium.org
Mon May 5 17:42:57 CEST 2025
Insets are handled inconsistently at present, since menus use them to
offset the label text, whereas textlines don't. This is done because
menus need the margin to be visible when opened. However this causes an
alignment issue when menus and textlines appear in the same cedit.
Remove the offsets from menus and compensate by adjusting the bounding
boxes used for highlighting and the opened menu.
Line up menu items and textlines vertically and add a style option for
textlines to control how much padding is added.
Add a test to check the positions of objects in a cedit, since this is
more direct than the rendering tests. Add style information so that the
impact can be seen.
Signed-off-by: Simon Glass <sjg at chromium.org>
---
arch/sandbox/dts/sandbox.dtsi | 4 +-
arch/sandbox/dts/test.dts | 1 +
boot/cedit.c | 10 ++-
boot/expo.c | 2 +
boot/scene.c | 2 +-
boot/scene_menu.c | 9 +-
boot/scene_textline.c | 30 ++++---
doc/develop/expo.rst | 4 +
include/expo.h | 3 +
test/boot/cedit.c | 152 ++++++++++++++++++++++++++++++----
10 files changed, 181 insertions(+), 36 deletions(-)
diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index 8a115c503dc..dd18bd26352 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -46,7 +46,9 @@
cedit-theme {
font-size = <30>;
menu-inset = <3>;
- menuitem-gap-y = <1>;
+ menuitem-gap-y = <0>;
+ menu-title-margin-x = <10>;
+ textline-label-margin-x = <10>;
};
};
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 2087c91285c..3e8db5c09c6 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -137,6 +137,7 @@
font-size = <30>;
menu-inset = <3>;
menuitem-gap-y = <1>;
+ textline-label-margin-x = <10>;
};
/*
diff --git a/boot/cedit.c b/boot/cedit.c
index c560be4d9c4..cc60e757649 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -52,6 +52,7 @@ struct cedit_iter_priv {
int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id)
{
+ struct expo_theme *theme = &exp->theme;
struct expo_arrange_info arr;
struct scene_obj_txt *txt;
struct scene_obj *obj;
@@ -77,26 +78,31 @@ int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id)
y = 100;
list_for_each_entry(obj, &scn->obj_head, sibling) {
+ bool add_gap = true;
+
switch (obj->type) {
case SCENEOBJT_NONE:
case SCENEOBJT_IMAGE:
case SCENEOBJT_TEXT:
case SCENEOBJT_BOX:
case SCENEOBJT_TEXTEDIT:
+ add_gap = false;
break;
case SCENEOBJT_MENU:
scene_obj_set_pos(scn, obj->id, 50, y);
scene_menu_arrange(scn, &arr,
(struct scene_obj_menu *)obj);
- y += 50;
+ y += obj->dims.y;
break;
case SCENEOBJT_TEXTLINE:
scene_obj_set_pos(scn, obj->id, 50, y);
scene_textline_arrange(scn, &arr,
(struct scene_obj_textline *)obj);
- y += 50;
+ y += obj->dims.y;
break;
}
+ if (add_gap)
+ y += theme->menuitem_gap_y;
}
ret = scene_arrange(scn);
if (ret)
diff --git a/boot/expo.c b/boot/expo.c
index a218ea0e4e9..035c3de5a0a 100644
--- a/boot/expo.c
+++ b/boot/expo.c
@@ -313,6 +313,8 @@ int expo_setup_theme(struct expo *exp, ofnode node)
ofnode_read_u32(node, "menuitem-gap-y", &theme->menuitem_gap_y);
ofnode_read_u32(node, "menu-title-margin-x",
&theme->menu_title_margin_x);
+ ofnode_read_u32(node, "textline-label-margin-x",
+ &theme->textline_label_margin_x);
theme->white_on_black = ofnode_read_bool(node, "white-on-black");
ret = expo_apply_theme(exp);
diff --git a/boot/scene.c b/boot/scene.c
index 40ef3e0138c..61b943e2b52 100644
--- a/boot/scene.c
+++ b/boot/scene.c
@@ -563,7 +563,7 @@ static int scene_txt_render(struct expo *exp, struct udevice *dev,
inset = exp->popup ? menu_inset : 0;
vidconsole_push_colour(cons, fore, back, &old);
video_fill_part(dev, x - inset, y,
- obj->bbox.x1, obj->bbox.y1,
+ obj->bbox.x1 + inset, obj->bbox.y1,
vid_priv->colour_bg);
}
diff --git a/boot/scene_menu.c b/boot/scene_menu.c
index 54c20d3e137..fd16cad6521 100644
--- a/boot/scene_menu.c
+++ b/boot/scene_menu.c
@@ -297,8 +297,7 @@ int scene_menu_arrange(struct scene *scn, struct expo_arrange_info *arr,
* Put the label on the left, then leave a space for the
* pointer, then the key and the description
*/
- ret = scene_obj_set_pos(scn, item->label_id,
- x + theme->menu_inset, y);
+ ret = scene_obj_set_pos(scn, item->label_id, x, y);
if (ret < 0)
return log_msg_ret("nam", ret);
scene_obj_set_hide(scn, item->label_id,
@@ -348,11 +347,11 @@ int scene_menu_arrange(struct scene *scn, struct expo_arrange_info *arr,
list_for_each_entry(item, &menu->item_head, sibling) {
scene_obj_set_width(menu->obj.scene, item->label_id,
- dims[SCENEBB_label].x + theme->menu_inset);
+ dims[SCENEBB_label].x);
scene_obj_set_width(menu->obj.scene, item->key_id,
- dims[SCENEBB_key].x + theme->menu_inset);
+ dims[SCENEBB_key].x);
scene_obj_set_width(menu->obj.scene, item->desc_id,
- dims[SCENEBB_desc].x + theme->menu_inset);
+ dims[SCENEBB_desc].x);
}
if (sel_id)
diff --git a/boot/scene_textline.c b/boot/scene_textline.c
index e55db98ec30..acaf3d91bbc 100644
--- a/boot/scene_textline.c
+++ b/boot/scene_textline.c
@@ -49,13 +49,14 @@ void scene_textline_calc_bbox(struct scene_obj_textline *tline,
struct vidconsole_bbox *edit_bbox)
{
const struct expo_theme *theme = &tline->obj.scene->expo->theme;
+ int inset = theme->menu_inset;
bbox->valid = false;
- scene_bbox_union(tline->obj.scene, tline->label_id, 0, bbox);
- scene_bbox_union(tline->obj.scene, tline->edit_id, 0, bbox);
+ scene_bbox_union(tline->obj.scene, tline->label_id, inset, bbox);
+ scene_bbox_union(tline->obj.scene, tline->edit_id, inset, bbox);
edit_bbox->valid = false;
- scene_bbox_union(tline->obj.scene, tline->edit_id, theme->menu_inset,
+ scene_bbox_union(tline->obj.scene, tline->edit_id, inset,
edit_bbox);
}
@@ -89,6 +90,7 @@ int scene_textline_arrange(struct scene *scn, struct expo_arrange_info *arr,
struct scene_obj_textline *tline)
{
const bool open = tline->obj.flags & SCENEOF_OPEN;
+ const struct expo_theme *theme = &scn->expo->theme;
bool point;
int x, y;
int ret;
@@ -96,19 +98,22 @@ int scene_textline_arrange(struct scene *scn, struct expo_arrange_info *arr,
x = tline->obj.req_bbox.x0;
y = tline->obj.req_bbox.y0;
if (tline->label_id) {
- ret = scene_obj_set_pos(scn, tline->label_id, x, y);
- if (ret < 0)
- return log_msg_ret("tit", ret);
+ struct scene_obj *edit;
- ret = scene_obj_set_pos(scn, tline->edit_id, x + 200, y);
+ ret = scene_obj_set_pos(scn, tline->label_id, x, y);
if (ret < 0)
return log_msg_ret("tit", ret);
- ret = scene_obj_get_hw(scn, tline->label_id, NULL);
+ x += arr->label_width + theme->textline_label_margin_x;
+ ret = scene_obj_set_pos(scn, tline->edit_id, x, y);
if (ret < 0)
- return log_msg_ret("hei", ret);
+ return log_msg_ret("til", ret);
- y += ret * 2;
+ edit = scene_obj_find(scn, tline->edit_id, SCENEOBJT_NONE);
+ if (!edit)
+ return log_msg_ret("tie", -ENOENT);
+ x += edit->dims.x;
+ y += edit->dims.y;
}
point = scn->highlight_id == tline->obj.id;
@@ -116,6 +121,11 @@ int scene_textline_arrange(struct scene *scn, struct expo_arrange_info *arr,
scene_obj_flag_clrset(scn, tline->edit_id, SCENEOF_POINT,
point ? SCENEOF_POINT : 0);
+ tline->obj.dims.x = x - tline->obj.req_bbox.x0;
+ tline->obj.dims.y = y - tline->obj.req_bbox.y0;
+ scene_obj_set_size(scn, tline->obj.id, tline->obj.dims.x,
+ tline->obj.dims.y);
+
return 0;
}
diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst
index b94340e9a8d..5bdaffb14bb 100644
--- a/doc/develop/expo.rst
+++ b/doc/develop/expo.rst
@@ -218,6 +218,10 @@ menu-title-margin-x
Number of pixels between right side of menu title to the left size of the
menu labels
+textline-label-margin-x
+ Number of pixels between right side of textline label to the left size of
+ the editor
+
Pop-up mode
-----------
diff --git a/include/expo.h b/include/expo.h
index b67eb53b0c9..e383872b307 100644
--- a/include/expo.h
+++ b/include/expo.h
@@ -83,6 +83,8 @@ struct expo_action {
* @menuitem_gap_y: Gap between menu items in pixels
* @menu_title_margin_x: Gap between right side of menu title and left size of
* menu label
+ * @textline_label_margin_x: Gap between right side of textline prompt and left
+ * side of editable text
* @white_on_black: True to use white-on-black for the expo, false for
* black-on-white
*/
@@ -91,6 +93,7 @@ struct expo_theme {
u32 menu_inset;
u32 menuitem_gap_y;
u32 menu_title_margin_x;
+ u32 textline_label_margin_x;
bool white_on_black;
};
diff --git a/test/boot/cedit.c b/test/boot/cedit.c
index dbf781902fb..9d49fbdc306 100644
--- a/test/boot/cedit.c
+++ b/test/boot/cedit.c
@@ -243,6 +243,7 @@ static int cedit_render(struct unit_test_state *uts)
struct expo_action evt;
struct expo_action act;
struct udevice *dev, *con;
+ struct expo_theme *theme;
struct stdio_dev *sdev;
struct scene *scn;
struct expo *exp;
@@ -255,6 +256,12 @@ static int cedit_render(struct unit_test_state *uts)
ut_assertnonnull(sdev);
con = sdev->priv;
+ theme = &exp->theme;
+ theme->menuitem_gap_y = 2;
+ theme->menu_inset = 2;
+ /* theme->menu_title_margin_x is 0 so menu labels shouldn't line up */
+ theme->textline_label_margin_x = 10;
+
dev = dev_get_parent(con);
vid_priv = dev_get_uclass_priv(dev);
ut_asserteq(ID_SCENE1, cedit_prepare(exp, dev, &scn));
@@ -264,7 +271,7 @@ static int cedit_render(struct unit_test_state *uts)
ut_asserteq(ID_AC_OFF, menu->cur_item_id);
ut_assertok(expo_render(exp));
- ut_asserteq(4929, video_compress_fb(uts, dev, false));
+ ut_asserteq(4888, video_compress_fb(uts, dev, false));
ut_assertok(video_check_copy_fb(uts, dev));
/* move to the second menu */
@@ -272,54 +279,54 @@ static int cedit_render(struct unit_test_state *uts)
act.select.id = ID_POWER_LOSS;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(4986, video_compress_fb(uts, dev, false));
+ ut_asserteq(4967, video_compress_fb(uts, dev, false));
/* open the menu */
act.type = EXPOACT_OPEN;
act.select.id = ID_POWER_LOSS;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(5393, video_compress_fb(uts, dev, false));
+ ut_asserteq(5397, video_compress_fb(uts, dev, false));
/* close the menu */
act.type = EXPOACT_CLOSE;
act.select.id = ID_POWER_LOSS;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(4986, video_compress_fb(uts, dev, false));
+ ut_asserteq(4967, video_compress_fb(uts, dev, false));
/* open the menu again to check it looks the same */
act.type = EXPOACT_OPEN;
act.select.id = ID_POWER_LOSS;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(5393, video_compress_fb(uts, dev, false));
+ ut_asserteq(5397, video_compress_fb(uts, dev, false));
/* close the menu */
act.type = EXPOACT_CLOSE;
act.select.id = ID_POWER_LOSS;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(4986, video_compress_fb(uts, dev, false));
+ ut_asserteq(4967, video_compress_fb(uts, dev, false));
act.type = EXPOACT_OPEN;
act.select.id = ID_POWER_LOSS;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(5393, video_compress_fb(uts, dev, false));
+ ut_asserteq(5397, video_compress_fb(uts, dev, false));
act.type = EXPOACT_POINT_ITEM;
act.select.id = ID_AC_ON;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(5365, video_compress_fb(uts, dev, false));
+ ut_asserteq(5341, video_compress_fb(uts, dev, false));
/* select it */
act.type = EXPOACT_SELECT;
act.select.id = ID_AC_ON;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(4980, video_compress_fb(uts, dev, false));
+ ut_asserteq(4939, video_compress_fb(uts, dev, false));
ut_asserteq(ID_AC_ON, menu->cur_item_id);
@@ -328,14 +335,14 @@ static int cedit_render(struct unit_test_state *uts)
act.select.id = ID_MACHINE_NAME;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(4862, video_compress_fb(uts, dev, false));
+ ut_asserteq(4850, video_compress_fb(uts, dev, false));
/* open it */
act.type = EXPOACT_OPEN;
act.select.id = ID_MACHINE_NAME;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(4851, video_compress_fb(uts, dev, false));
+ ut_asserteq(4883, video_compress_fb(uts, dev, false));
/*
* Send some keypresses. Note that the console must be enabled so that
@@ -351,7 +358,7 @@ static int cedit_render(struct unit_test_state *uts)
ut_silence_console(uts);
ut_assertok(cedit_arange(exp, vid_priv, scn->id));
ut_assertok(expo_render(exp));
- ut_asserteq(4996, video_compress_fb(uts, dev, false));
+ ut_asserteq(5033, video_compress_fb(uts, dev, false));
expo_destroy(exp);
cur_exp = NULL;
@@ -394,7 +401,7 @@ static int cedit_render_lineedit(struct unit_test_state *uts)
ut_asserteq(20, tline->pos);
ut_assertok(expo_render(exp));
- ut_asserteq(5336, video_compress_fb(uts, dev, false));
+ ut_asserteq(5315, video_compress_fb(uts, dev, false));
ut_assertok(video_check_copy_fb(uts, dev));
/* move to the line-edit field */
@@ -402,15 +409,14 @@ static int cedit_render_lineedit(struct unit_test_state *uts)
act.select.id = ID_MACHINE_NAME;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
ut_assertok(expo_render(exp));
- ut_asserteq(5363, video_compress_fb(uts, dev, false));
+ ut_asserteq(5372, video_compress_fb(uts, dev, false));
/* open it */
act.type = EXPOACT_OPEN;
act.select.id = ID_MACHINE_NAME;
ut_assertok(cedit_do_action(exp, scn, vid_priv, &act));
- // ut_asserteq(0, tline->pos);
ut_assertok(expo_render(exp));
- ut_asserteq(5283, video_compress_fb(uts, dev, false));
+ ut_asserteq(5259, video_compress_fb(uts, dev, false));
/* delete some characters */
ut_unsilence_console(uts);
@@ -421,7 +427,7 @@ static int cedit_render_lineedit(struct unit_test_state *uts)
ut_assertok(cedit_arange(exp, vid_priv, scn->id));
ut_assertok(expo_render(exp));
- ut_asserteq(5170, video_compress_fb(uts, dev, false));
+ ut_asserteq(5126, video_compress_fb(uts, dev, false));
expo_destroy(exp);
cur_exp = NULL;
@@ -429,3 +435,115 @@ static int cedit_render_lineedit(struct unit_test_state *uts)
return 0;
}
BOOTSTD_TEST(cedit_render_lineedit, UTF_DM | UTF_SCAN_FDT);
+
+/* Check the cedit is arranged correctly */
+static int cedit_position(struct unit_test_state *uts)
+{
+ struct scene_obj_textline *tline;
+ struct scene_obj_menu *menu;
+ struct video_priv *vid_priv;
+ extern struct expo *cur_exp;
+ const uint label_width = 96;
+ struct scene_menitem *item;
+ struct udevice *dev, *con;
+ struct expo_theme *theme;
+ struct scene_obj *obj;
+ struct stdio_dev *sdev;
+ struct scene *scn;
+ struct expo *exp;
+ uint x_offset;
+
+ ut_assertok(run_command("cedit load hostfs - cedit.dtb", 0));
+ exp = cur_exp;
+
+ sdev = stdio_get_by_name("vidconsole");
+ ut_assertnonnull(sdev);
+ con = sdev->priv;
+
+ theme = &exp->theme;
+ theme->menuitem_gap_y = 2;
+ theme->menu_inset = 2;
+ theme->textline_label_margin_x = 10;
+ theme->menu_title_margin_x = 10;
+
+ dev = dev_get_parent(con);
+ vid_priv = dev_get_uclass_priv(dev);
+ ut_asserteq(ID_SCENE1, cedit_prepare(exp, dev, &scn));
+
+ // expo_dump(exp);
+ menu = scene_obj_find(scn, ID_CPU_SPEED, SCENEOBJT_MENU);
+ ut_assertnonnull(menu);
+ obj = &menu->obj;
+ ut_asserteq(50, obj->bbox.x0);
+ ut_asserteq(100, obj->bbox.y0);
+ ut_asserteq(50 + 160, obj->bbox.x1);
+ ut_asserteq(18 + 100, obj->bbox.y1);
+
+ obj = scene_obj_find(scn, menu->title_id, SCENEOBJT_TEXT);
+ ut_assertnonnull(obj);
+ ut_asserteq(50, obj->bbox.x0);
+ ut_asserteq(100, obj->bbox.y0);
+ ut_asserteq(50 + 75, obj->bbox.x1);
+ ut_asserteq(18 + 100, obj->bbox.y1);
+
+ item = scene_menuitem_find_seq(menu, 0);
+ ut_assertnonnull(item);
+
+ x_offset = label_width + theme->textline_label_margin_x;
+ obj = scene_obj_find(scn, item->label_id, SCENEOBJT_TEXT);
+ ut_assertnonnull(obj);
+ ut_asserteq(42, obj->dims.x);
+ ut_asserteq(18, obj->dims.y);
+
+ ut_asserteq(0, obj->ofs.xofs);
+ ut_asserteq(0, obj->ofs.yofs);
+
+ ut_asserteq(50 + x_offset, obj->bbox.x0);
+ ut_asserteq(100, obj->bbox.y0);
+ ut_asserteq(50 + x_offset + 54, obj->bbox.x1);
+ ut_asserteq(18 + 100, obj->bbox.y1);
+
+ /* all items should have the same, so check them */
+ item = scene_menuitem_find_seq(menu, 1);
+ ut_assertnonnull(item);
+ ut_asserteq(50 + x_offset + 54, obj->bbox.x1);
+
+ item = scene_menuitem_find_seq(menu, 2);
+ ut_assertnonnull(item);
+ ut_asserteq(50 + x_offset + 54, obj->bbox.x1);
+
+ menu = scene_obj_find(scn, ID_POWER_LOSS, SCENEOBJT_MENU);
+ ut_assertnonnull(menu);
+ ut_asserteq(ID_AC_OFF, menu->cur_item_id);
+
+ obj = &menu->obj;
+ ut_asserteq(50, obj->bbox.x0);
+ ut_asserteq(120, obj->bbox.y0);
+ ut_asserteq(50 + 160, obj->bbox.x1);
+ ut_asserteq(18 + 120, obj->bbox.y1);
+
+ tline = scene_obj_find(scn, ID_MACHINE_NAME, SCENEOBJT_TEXTLINE);
+ ut_assertnonnull(menu);
+
+ obj = &tline->obj;
+ ut_asserteq(381, obj->dims.x);
+ ut_asserteq(18, obj->dims.y);
+
+ ut_asserteq(50, obj->bbox.x0);
+ ut_asserteq(140, obj->bbox.y0);
+ ut_asserteq(50 + 381, obj->bbox.x1);
+ ut_asserteq(140 + 18, obj->bbox.y1);
+
+ obj = scene_obj_find(scn, ID_MACHINE_NAME_EDIT, SCENEOBJT_TEXT);
+ ut_assertnonnull(obj);
+ ut_asserteq(275, obj->dims.x);
+ ut_asserteq(18, obj->dims.y);
+
+ ut_asserteq(50 + x_offset, obj->bbox.x0);
+ ut_asserteq(140, obj->bbox.y0);
+ ut_asserteq(50 + x_offset + obj->dims.x, obj->bbox.x1);
+ ut_asserteq(140 + 18, obj->bbox.y1);
+
+ return 0;
+}
+BOOTSTD_TEST(cedit_position, UTF_DM | UTF_SCAN_FDT);
--
2.43.0
More information about the U-Boot
mailing list