expo: Tidy up insets and improve tests

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@chromium.org>
This commit is contained in:
Simon Glass
2025-05-05 17:42:57 +02:00
parent 502aa8bc31
commit e27b36c015
10 changed files with 181 additions and 36 deletions

View File

@@ -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>;
};
};

View File

@@ -137,6 +137,7 @@
font-size = <30>;
menu-inset = <3>;
menuitem-gap-y = <1>;
textline-label-margin-x = <10>;
};
/*

View File

@@ -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)

View File

@@ -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);

View File

@@ -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);
}

View File

@@ -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)

View File

@@ -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) {
struct scene_obj *edit;
ret = scene_obj_set_pos(scn, tline->label_id, x, y);
if (ret < 0)
return log_msg_ret("tit", ret);
ret = scene_obj_set_pos(scn, tline->edit_id, x + 200, y);
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("tit", ret);
return log_msg_ret("til", ret);
ret = scene_obj_get_hw(scn, tline->label_id, NULL);
if (ret < 0)
return log_msg_ret("hei", 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;
}

View File

@@ -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
-----------

View File

@@ -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;
};

View File

@@ -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);