Re: [OSS-Tools] [PATCH platsch V5 5/5] Add spinner executable for boot animation and text show

[LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>]

> -----Original Message-----
> From: Philipp Zabel <p.zabel@pengutronix.de>
> Sent: Thursday, June 20, 2024 8:47 PM
> To: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>;
> oss-tools@pengutronix.de; m.felsch@pengutronix.de
> Cc: GEO-CHHER-bsp-development
> <bsp-development.geo@leica-geosystems.com>
> Subject: Re: [OSS-Tools] [PATCH platsch V5 5/5] Add spinner executable for boot
> animation and text show
> 
> [你通常不会收到来自 p.zabel@pengutronix.de 的电子邮件。请访问
> https://aka.ms/LearnAboutSenderIdentification，以了解这一点为什么很重要]
> 
> This email is not from Hexagon’s Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> 
> 
> On Mi, 2024-06-19 at 12:22 +0200, LI Qingwu wrote:
> > This commit introduces a new executable, spinner,
> >     which supports two types of animations for boot sequences:
> >     1 static PNG and text support.
> >     2 rotates square PNG images per frame
> >     3 shows a sequence of square images from a strip of PNG images.
> 
> I've tried this and the CPU load was a bit unexpected.
> 
> I suggest clipping the background_surface->cr blits in
> on_draw_..._animation() and and the drawing_surface->disp_cr blit in the
> drawing loop to the minimum rectangle necessary, commented below.

I just create a drawing_surface same size as the animation symbol instead of display,
And 20fps with 800*600 display, CPU load of rotate animation goes from 16.6% to 4.3%
And sequence animation goes from 12.3% to 0.7%

> 
> Also setting spinner = "" should skip the drawing loop somehow. There should be
> minimal CPU load after disabling the animation as documented.
> 
> Are you stopping the spinner process at some point after the system has booted?
> That would be nice to suggest in the documentation, otherwise it keeps causing
> unnecessary CPU load.

Yes, I stop the spinner process after booted and I update the document in V6.

> 
> > Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> > ---
> >  README.rst        |  37 ++++++
> >  meson.build       |  21 ++++
> >  meson_options.txt |   1 +
> >  spinner.c         | 299
> ++++++++++++++++++++++++++++++++++++++++++++++
> >  spinner_conf.c    |  67 +++++++++++
> >  spinner_conf.h    |  33 +++++
> >  6 files changed, 458 insertions(+)
> >  create mode 100644 meson_options.txt
> >  create mode 100644 spinner.c
> >  create mode 100644 spinner_conf.c
> >  create mode 100644 spinner_conf.h
> >
> > diff --git a/README.rst b/README.rst
> > index f1c0812..a0f7f7e 100644
> > --- a/README.rst
> > +++ b/README.rst
> > @@ -149,3 +149,40 @@ Compiling Instructions
> >
> >      meson setup build
> >      meson compile -C build
> > +
> > +
> > +Spinner - Splash Screen with Animation
> > +======================================
> > +
> > +The `spinner` executable is designed to provide boot animations. It supports
> two types of animations:
> > +
> > +1. **static PNG and text support**: Show a static png image and text.
> > +1. **Square PNG Rotation Animation**: Rotates a square PNG image.
> > +2. **Sequence Move Rectangle Animation**: Displays a sequence of square
> images from a strip of PNG images.
> > +
> > +spinner Configuration
> > +---------------------
> > +
> > +The configuration for the `spinner` executable is read from a
> > +configuration file, with a default path of `/usr/share/platsch/spinner.conf`.
> > +The directory of the configuration file can be set via the `platsch_directory`
> environment variable.
> > +
> > +
> > +spinner Configuration
> > +---------------------
> > +
> > +Here is a sample  of a configuration file (`spinner.conf`):
> > +
> > +.. code-block:: ini
> > +
> > +    symbol="/usr/share/platsch/Spinner.png"
> > +    fps=20
> > +    text=""
> > +    text_x=350
> > +    text_y=400
> > +    text_font="Sans"
> > +    text_size=30
> > +
> > +Set text to empty if you don't want to display any text.
> > +Set the symbol to empty if you don't want to display any animation.
> > +The background image is indicated by ``--directory`` and ``--basename``.
> > diff --git a/meson.build b/meson.build index ccc7a66..936e7ca 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -22,3 +22,24 @@ executable('platsch',
> >      install: true,
> >      install_dir : 'sbin'
> >  )
> > +
> > +
> > +if get_option('SPINNER')
> > +    spinner_dep = [
> > +        dependency('cairo', version: '>= 1.0', static: true),
> 
> I think it would be best to just drop the "static: true" for now. The spinner
> executable isn't linked statically anyway.
> 
> > +        dep_libdrm,
> > +    ]
> > +
> > +    spinner_src = [
> > +        'spinner.c',
> > +        'spinner_conf.c'
> > +    ]
> > +
> > +    executable('spinner',
> > +        spinner_src,
> > +        dependencies: spinner_dep,
> > +        link_with: libplatsch,
> > +        install: true,
> > +        install_dir : 'sbin'
> > +    )
> > +endif
> > diff --git a/meson_options.txt b/meson_options.txt new file mode
> > 100644 index 0000000..d1149fe
> > --- /dev/null
> > +++ b/meson_options.txt
> > @@ -0,0 +1 @@
> > +option('SPINNER', type: 'boolean', value: false, description: 'Enable
> > +spinner build')
> 
> Please make this lowercase, as is common practice for Meson options.
> 
> > diff --git a/spinner.c b/spinner.c
> > new file mode 100644
> > index 0000000..dbf8a4b
> > --- /dev/null
> > +++ b/spinner.c
> > @@ -0,0 +1,299 @@
> > +
> > +#include <cairo.h>
> > +#include <math.h>
> > +#include <sys/time.h>
> > +
> > +#include "libplatsch.h"
> > +#include "spinner_conf.h"
> > +
> > +typedef struct spinner {
> > +     cairo_format_t fmt;
> > +     cairo_surface_t *background_surface;
> > +     cairo_surface_t *symbol_surface;
> > +     cairo_surface_t *drawing_surface;
> > +     cairo_t *cr_background;
> > +     cairo_t *cr_drawing;
> > +     cairo_t *disp_cr;
> > +     int background_height;
> > +     int background_width;
> > +     int display_height;
> > +     int display_width;
> > +     int symbol_height;
> > +     int symbol_width;
> > +     struct modeset_dev *dev;
> > +     struct spinner *next;
> > +} spinner_t;
> > +
> > +void on_draw_Sequence_animation(cairo_t *cr, spinner_t *data) {
> > +     static int current_frame;
> > +     int num_frames = data->symbol_width / data->symbol_height;
> > +     int frame_width = data->symbol_height;
> > +
> > +     cairo_set_source_surface(cr, data->background_surface, 0, 0);
> > +     cairo_paint(cr);
> 
> This paint is likely wasteful and should be clipped to the rectangle that is
> overwritten by the animation frame.
> 
> > +
> > +     cairo_save(cr);
> > +
> > +     cairo_translate(cr, data->display_width / 2,
> > + data->display_height / 2);
> > +
> > +     cairo_set_source_surface(cr, data->symbol_surface,
> > +                              -frame_width / 2 - current_frame *
> > + frame_width, -frame_width / 2);
> > +
> > +     cairo_rectangle(cr, -frame_width / 2, -frame_width / 2, frame_width,
> frame_width);
> > +     cairo_clip(cr);
> > +     cairo_paint(cr);
> > +
> > +     cairo_restore(cr);
> > +
> > +     current_frame = (current_frame + 1) % num_frames; }
> > +
> > +void on_draw_rotation_animation(cairo_t *cr, spinner_t *data) {
> > +     static float angle = 0.0;
> > +
> > +     cairo_set_source_surface(cr, data->background_surface, 0, 0);
> > +     cairo_paint(cr);
> 
> This paint likely is wasteful and should be clipped to the rectangle that was
> overwritten by the previous animation frame, or at least to a square the size of
> the diagonal of the symbol surface.
> 
> > +     cairo_save(cr);
> > +     cairo_translate(cr, data->background_width / 2,
> data->background_height / 2);
> > +     cairo_rotate(cr, angle);
> > +     cairo_translate(cr, -data->symbol_width / 2, -data->symbol_height /
> 2);
> > +     cairo_set_source_surface(cr, data->symbol_surface, 0, 0);
> > +     cairo_paint(cr);
> > +     cairo_restore(cr);
> > +     angle += 0.1;
> > +     if (angle > 2 * M_PI)
> > +             angle = 0.0;
> > +}
> > +
> > +static uint32_t convert_to_cairo_format(uint32_t format) {
> > +     switch (format) {
> > +     case DRM_FORMAT_RGB565:
> > +             return CAIRO_FORMAT_RGB16_565;
> > +     case DRM_FORMAT_XRGB8888:
> > +             return CAIRO_FORMAT_ARGB32;
> > +     }
> > +     return CAIRO_FORMAT_INVALID;
> > +}
> > +
> > +static void cairo_draw_text(cairo_t *cr, Config config) {
> > +     if (strlen(config.text) > 0) {
> > +             cairo_select_font_face(cr, config.text_font,
> CAIRO_FONT_SLANT_NORMAL,
> > +
> CAIRO_FONT_WEIGHT_NORMAL);
> > +             cairo_set_font_size(cr, (double)config.text_size);
> > +             cairo_move_to(cr, config.text_x, config.text_y);
> > +             cairo_set_source_rgb(cr, 0, 0, 0);
> > +             cairo_show_text(cr, config.text);
> > +     }
> > +}
> > +
> > +static int cairo_create_display_surface(struct modeset_dev *dev,
> > +spinner_t *spinner_node) {
> > +     cairo_surface_t *disp_surface =
> cairo_image_surface_create_for_data(
> > +         dev->map, convert_to_cairo_format(dev->format->format),
> dev->width, dev->height,
> > +         dev->stride);
> > +     if (cairo_surface_status(disp_surface)) {
> > +             platsch_error("Failed to create cairo surface\n");
> > +             return EXIT_FAILURE;
> > +     }
> > +     spinner_node->display_width =
> cairo_image_surface_get_width(disp_surface);
> > +     spinner_node->display_height =
> cairo_image_surface_get_height(disp_surface);
> > +     spinner_node->fmt = cairo_image_surface_get_format(disp_surface);
> > +     spinner_node->disp_cr = cairo_create(disp_surface);
> > +     return EXIT_SUCCESS;
> > +}
> > +
> > +// create a surface and paint background image static int
> > +cairo_create_background_surface(spinner_t *spinner_node, Config config,
> const char *dir,
> > +                                        const char *base) {
> > +     char filename[128];
> > +     int ret;
> > +
> > +     ret = snprintf(filename, sizeof(filename), "%s/%s.png", dir, base);
> > +     if (ret >= sizeof(filename)) {
> > +             platsch_error("Failed to fit filename into buffer\n");
> > +             return EXIT_FAILURE;
> > +     }
> > +
> > +     spinner_node->background_surface = cairo_image_surface_create(
> > +         spinner_node->fmt, spinner_node->display_width,
> spinner_node->display_height);
> > +     if (cairo_surface_status(spinner_node->background_surface)) {
> > +             platsch_error("Failed to create background surface\n");
> > +             return EXIT_FAILURE;
> > +     }
> > +
> > +     cairo_surface_t *bk_img_surface =
> > + cairo_image_surface_create_from_png(filename);
> > +
> > +     if (cairo_surface_status(bk_img_surface)) {
> > +             platsch_error("Failed to create cairo surface from %s\n",
> filename);
> > +             return EXIT_FAILURE;
> > +     }
> > +
> > +     int image_width = cairo_image_surface_get_width(bk_img_surface);
> > +     int image_height = cairo_image_surface_get_height(bk_img_surface);
> > +     double scale_x = (double)spinner_node->display_width / image_width;
> > +     double scale_y = (double)spinner_node->display_height /
> > + image_height;
> > +
> > +     spinner_node->cr_background =
> cairo_create(spinner_node->background_surface);
> > +     cairo_scale(spinner_node->cr_background, scale_x, scale_y);
> > +     cairo_set_source_surface(spinner_node->cr_background,
> > + bk_img_surface, 0, 0);
> > +
> > +     spinner_node->background_width =
> > +
> cairo_image_surface_get_width(spinner_node->background_surface);
> > +     spinner_node->background_height =
> > +
> > + cairo_image_surface_get_height(spinner_node->background_surface);
> > +
> > +     cairo_paint(spinner_node->cr_background);
> > +     cairo_draw_text(spinner_node->cr_background, config);
> > +     return EXIT_SUCCESS;
> > +}
> > +
> > +static int cairo_create_symbol_surface(spinner_t *spinner_node,
> > +Config config) {
> > +     if (strlen(config.symbol) == 0)
> > +             return EXIT_FAILURE;
> > +
> > +     spinner_node->symbol_surface =
> cairo_image_surface_create_from_png(config.symbol);
> > +     if (cairo_surface_status(spinner_node->symbol_surface)) {
> > +             platsch_error("Failed loading %s\n", config.symbol);
> > +             spinner_node->symbol_surface = NULL;
> > +             return EXIT_FAILURE;
> > +     }
> > +     spinner_node->symbol_width =
> cairo_image_surface_get_width(spinner_node->symbol_surface);
> > +     spinner_node->symbol_height =
> cairo_image_surface_get_height(spinner_node->symbol_surface);
> > +     return EXIT_SUCCESS;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +     bool pid1 = getpid() == 1;
> > +     char **initsargv;
> > +     char filename[128];
> > +     Config config = DEFAULT_CONFIG;
> > +     const char *base = "splash";
> > +     const char *dir = "/usr/share/platsch";
> > +     const char *env;
> > +     int ret;
> > +     long elapsed_time;
> > +
> > +     spinner_t *spinner_list = NULL, *spinner_node = NULL, *spinner_iter =
> NULL;
> > +     struct modeset_dev *iter;
> > +     struct timeval start, end;
> > +
> > +     env = getenv("platsch_directory");
> > +     if (env)
> > +             dir = env;
> > +
> > +     env = getenv("platsch_basename");
> > +     if (env)
> > +             base = env;
> > +
> > +     ret = snprintf(filename, sizeof(filename), "%s/spinner.conf", dir);
> > +     if (ret >= sizeof(filename)) {
> > +             platsch_error("Failed to fit filename\n");
> > +             return EXIT_FAILURE;
> > +     }
> > +
> > +     parseConfig(filename, &config);
> > +
> > +     struct modeset_dev *modeset_list = platsch_init();
> > +
> > +     if (!modeset_list) {
> > +             platsch_error("Failed to initialize modeset\n");
> > +             return EXIT_FAILURE;
> > +     }
> > +
> > +     for (iter = modeset_list; iter; iter = iter->next) {
> > +             spinner_node = (spinner_t *)malloc(sizeof(spinner_t));
> > +             if (!spinner_node) {
> > +                     platsch_error("Failed to allocate memory for
> spinner_node\n");
> > +                     return EXIT_FAILURE;
> > +             }
> > +             memset(spinner_node, 0, sizeof(*spinner_node));
> > +
> > +             if (cairo_create_display_surface(iter, spinner_node))
> > +                     return EXIT_FAILURE;
> > +
> > +             if (cairo_create_background_surface(spinner_node, config,
> dir, base))
> > +                     return EXIT_FAILURE;
> > +
> > +             cairo_create_symbol_surface(spinner_node, config);
> > +
> > +             spinner_node->drawing_surface =
> cairo_image_surface_create(
> > +                 spinner_node->fmt, spinner_node->display_width,
> spinner_node->display_height);
> > +             if (cairo_surface_status(spinner_node->drawing_surface)) {
> > +                     platsch_error("Failed to create drawing
> surface\n");
> > +                     return EXIT_FAILURE;
> > +             }
> > +             // create cairo context for drawing surface to avoid
> frlickering
> > +             spinner_node->cr_drawing =
> cairo_create(spinner_node->drawing_surface);
> > +             // draw static image with text as first frame
> > +             cairo_set_source_surface(spinner_node->disp_cr,
> spinner_node->background_surface, 0,
> > +                                      0);
> > +             cairo_paint(spinner_node->disp_cr);
> > +             platsch_setup_display(iter);
> > +
> > +             spinner_node->next = spinner_list;
> > +             spinner_list = spinner_node;
> > +     }
> > +
> > +     platsch_drmDropMaster();
> > +
> > +     ret = fork();
> > +     if (ret < 0)
> > +             platsch_error("failed to fork for init: %m\n");
> > +     else if (ret == 0)
> > +             /*
> > +              * Always fork to make sure we got the required
> > +              * resources for drawing the animation in the child.
> > +              */
> > +             goto drawing;
> > +
> > +     if (pid1) {
> > +             initsargv = calloc(sizeof(argv[0]), argc + 1);
> > +
> > +             if (!initsargv) {
> > +                     platsch_error("failed to allocate argv for init\n");
> > +                     return EXIT_FAILURE;
> > +             }
> > +             memcpy(initsargv, argv, argc * sizeof(argv[0]));
> > +             initsargv[0] = "/sbin/init";
> > +             initsargv[argc] = NULL;
> > +
> > +             execv("/sbin/init", initsargv);
> > +
> > +             platsch_error("failed to exec init: %m\n");
> > +
> > +             return EXIT_FAILURE;
> > +     }
> > +     return EXIT_SUCCESS;
> > +
> > +drawing:
> > +     while (true) {
> > +             gettimeofday(&start, NULL);
> > +             for (spinner_iter = spinner_list; spinner_iter; spinner_iter =
> spinner_iter->next) {
> > +                     if (spinner_node->symbol_surface == NULL)
> > +                             usleep(1000000 / config.fps);
> 
> What is the purpose of this delay?
> 
> > +                     if (spinner_node->symbol_width /
> spinner_node->symbol_height > 2)
> > +
> on_draw_Sequence_animation(spinner_iter->cr_drawing, spinner_iter);
> > +                     else
> > +
> > + on_draw_rotation_animation(spinner_iter->cr_drawing, spinner_iter);
> > +
> > +                     cairo_set_source_surface(spinner_iter->disp_cr,
> > +
> spinner_iter->drawing_surface, 0, 0);
> > +                     cairo_paint(spinner_iter->disp_cr);
> 
> This paint is likely wasteful and should be clipped to the rectangle that was
> overwritten in on_draw_..._animation().
> 
> > +             }
> > +             gettimeofday(&end, NULL);
> > +             elapsed_time =
> > +                 (end.tv_sec - start.tv_sec) * 1000000 + (end.tv_usec
> > + - start.tv_usec);
> > +
> > +             long sleep_time = (1000000 / config.fps) - elapsed_time;
> > +
> > +             if (sleep_time > 0)
> > +                     usleep(sleep_time);
> 
> I'd use clock_gettime and clock_nanosleep with CLOCK_MONOTONIC instead of
> gettimeofday and usleep. That should allow advancing the target time without
> having to measure the elapsed time.
> 
> 
> regards
> Philipp