From 8bd50ffd4ba81cb9c5197bcae926e177cc7a8987 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Fri, 15 Jul 2022 15:09:01 -0700 Subject: til_fb: switch til_fb_ops_t.init() to use til_setup_t Until now the fb init has been receiving a til_settings_t to access its setup. Now that there's a til_setup_t for representing the fully baked setup, let's bring the fb stuff up to speed so their init() behaves more like til_module_t.create_context() WRT settings/setup. This involves some reworking of how settings are handled in {drm,sdl}_fb.c but nothing majorly different. The only real funcitonal change that happened in the course of this work is I made it possible now to actually instruct SDL to do a more legacy SDL_WINDOW_FULLSCREEN vs. SDL_WINDOW_FULLSCREEN_DESKTOP where SDL will attempt to switch the video mode. This is triggered by specifying both a size=WxH and fullscreen=on for video=sdl. Be careful though, I've observed some broken display states when specifying goofy sizes, which look like Xorg bugs. --- src/drm_fb.c | 62 +++++++++++++++++++++++--------------------- src/main.c | 30 +++++++++++----------- src/sdl_fb.c | 75 +++++++++++++++++++++++++++++++++++++++--------------- src/til_fb.c | 4 +-- src/til_fb.h | 4 +-- src/til_settings.c | 7 +++-- src/til_settings.h | 5 ++-- 7 files changed, 115 insertions(+), 72 deletions(-) (limited to 'src') diff --git a/src/drm_fb.c b/src/drm_fb.c index a60f1e7..e7f0376 100644 --- a/src/drm_fb.c +++ b/src/drm_fb.c @@ -35,6 +35,7 @@ struct drm_fb_page_t { }; typedef struct drm_fb_setup_t { + til_setup_t til_setup; const char *dev; const char *connector; const char *mode; @@ -68,7 +69,7 @@ static const char * connector_type_name(uint32_t type) { } -static int dev_desc_generator(void *setup_context, const til_setting_desc_t **res_desc) +static int dev_desc_generator(til_setup_t *setup_context, const til_setting_desc_t **res_desc) { return til_setting_desc_clone(&(til_setting_desc_t){ .name = "DRM device path", @@ -145,9 +146,9 @@ static void free_strv(const char **strv) } -static int connector_desc_generator(void *setup_context, const til_setting_desc_t **res_desc) +static int connector_desc_generator(til_setup_t *setup_context, const til_setting_desc_t **res_desc) { - drm_fb_setup_t *s = setup_context; + drm_fb_setup_t *s = (drm_fb_setup_t *)setup_context; const char **connectors; int r; @@ -254,9 +255,9 @@ _out: } -static int mode_desc_generator(void *setup_context, const til_setting_desc_t **res_desc) +static int mode_desc_generator(til_setup_t *setup_context, const til_setting_desc_t **res_desc) { - drm_fb_setup_t *s = setup_context; + drm_fb_setup_t *s = (drm_fb_setup_t *)setup_context; const char **modes; int r; @@ -280,24 +281,36 @@ static int mode_desc_generator(void *setup_context, const til_setting_desc_t **r } +static void drm_fb_setup_free(til_setup_t *setup) +{ + drm_fb_setup_t *s = (drm_fb_setup_t *)setup; + + assert(s); + + free((void *)s->dev); + free((void *)s->connector); + free((void *)s->mode); +} + + /* setup is called repeatedly as settings is constructed, until 0 is returned. */ /* a negative value is returned on error */ /* positive value indicates another setting is needed, described in next_setting */ static int drm_fb_setup(const til_settings_t *settings, til_setting_t **res_setting, const til_setting_desc_t **res_desc, til_setup_t **res_setup) { - drm_fb_setup_t context = {}; + drm_fb_setup_t *setup = til_setup_new(sizeof(*setup), drm_fb_setup_free); til_setting_desc_generator_t generators[] = { { .key = "dev", - .value_ptr = &context.dev, + .value_ptr = &setup->dev, .func = dev_desc_generator }, { .key = "connector", - .value_ptr = &context.connector, + .value_ptr = &setup->connector, .func = connector_desc_generator }, { .key = "mode", - .value_ptr = &context.mode, + .value_ptr = &setup->mode, .func = mode_desc_generator }, }; @@ -305,7 +318,10 @@ static int drm_fb_setup(const til_settings_t *settings, til_setting_t **res_sett if (!drmAvailable()) return -ENOSYS; - return til_settings_apply_desc_generators(settings, generators, nelems(generators), &context, res_setting, res_desc); + if (!setup) + return -ENOMEM; + + return til_settings_apply_desc_generators(settings, generators, nelems(generators), &setup->til_setup, res_setting, res_desc, res_setup); } @@ -335,8 +351,9 @@ static drmModeModeInfo * lookup_mode(drmModeConnector *connector, const char *mo /* prepare the drm context for use with the supplied settings */ -static int drm_fb_init(const til_settings_t *settings, void **res_context) +static int drm_fb_init(const til_setup_t *setup, void **res_context) { + drm_fb_setup_t *s = (drm_fb_setup_t *)setup; drm_fb_t *c; const char *dev; const char *connector; @@ -344,27 +361,14 @@ static int drm_fb_init(const til_settings_t *settings, void **res_context) drmModeEncoder *enc; int r; - assert(settings); + assert(setup); if (!drmAvailable()) { r = -errno; goto _err; } - dev = til_settings_get_value(settings, "dev", NULL); - if (!dev) { - r = -EINVAL; - goto _err; - } - - connector = til_settings_get_value(settings, "connector", NULL); - if (!connector) { - r = -EINVAL; - goto _err; - } - - mode = til_settings_get_value(settings, "mode", NULL); - if (!mode) { + if (!s->dev || !s->connector || !s->mode) { r = -EINVAL; goto _err; } @@ -375,17 +379,17 @@ static int drm_fb_init(const til_settings_t *settings, void **res_context) goto _err; } - c->drm_fd = open(dev, O_RDWR); + c->drm_fd = open(s->dev, O_RDWR); if (c->drm_fd < 0) { r = -errno; goto _err_ctxt; } - r = lookup_connector(c->drm_fd, connector, &c->connector); + r = lookup_connector(c->drm_fd, s->connector, &c->connector); if (r < 0) goto _err_fd; - c->mode = lookup_mode(c->connector, mode); + c->mode = lookup_mode(c->connector, s->mode); if (!c->mode) { r = -EINVAL; goto _err_con; diff --git a/src/main.c b/src/main.c index 6381bc5..68ff1da 100644 --- a/src/main.c +++ b/src/main.c @@ -50,9 +50,9 @@ static rototiller_t rototiller; typedef struct setup_t { - til_settings_t *module; + til_settings_t *module_settings; til_setup_t *module_setup; - til_settings_t *video; + til_settings_t *video_settings; til_setup_t *video_setup; } setup_t; @@ -126,21 +126,21 @@ static int setup_from_args(til_args_t *args, setup_t *res_setup, const til_setti int r = -ENOMEM, changes = 0; setup_t setup = {}; - setup.module = til_settings_new(args->module); - if (!setup.module) + setup.module_settings = til_settings_new(args->module); + if (!setup.module_settings) goto _err; - setup.video = til_settings_new(args->video); - if (!setup.video) + setup.video_settings = til_settings_new(args->video); + if (!setup.video_settings) goto _err; - r = setup_interactively(setup.module, til_module_setup, args->use_defaults, &setup.module_setup, res_failed_desc); + r = setup_interactively(setup.module_settings, til_module_setup, args->use_defaults, &setup.module_setup, res_failed_desc); if (r < 0) goto _err; if (r) changes = 1; - r = setup_interactively(setup.video, setup_video, args->use_defaults, &setup.video_setup, res_failed_desc); + r = setup_interactively(setup.video_settings, setup_video, args->use_defaults, &setup.video_setup, res_failed_desc); if (r < 0) goto _err; if (r) @@ -151,8 +151,8 @@ static int setup_from_args(til_args_t *args, setup_t *res_setup, const til_setti return changes; _err: - til_settings_free(setup.module); - til_settings_free(setup.video); + til_settings_free(setup.module_settings); + til_settings_free(setup.video_settings); return r; } @@ -164,14 +164,14 @@ static int print_setup_as_args(setup_t *setup) char buf[64]; int r; - module_args = til_settings_as_arg(setup->module); + module_args = til_settings_as_arg(setup->module_settings); if (!module_args) { r = -ENOMEM; goto _out; } - video_args = til_settings_as_arg(setup->video); + video_args = til_settings_as_arg(setup->video_settings); if (!video_args) { r = -ENOMEM; @@ -267,10 +267,10 @@ int main(int argc, const char *argv[]) exit_if(!args.gogogo && r && print_setup_as_args(&setup) < 0, "unable to print setup"); - exit_if(!(rototiller.module = til_lookup_module(til_settings_get_key(setup.module, 0, NULL))), - "unable to lookup module from settings \"%s\"", til_settings_get_key(setup.module, 0, NULL)); + exit_if(!(rototiller.module = til_lookup_module(til_settings_get_key(setup.module_settings, 0, NULL))), + "unable to lookup module from settings \"%s\"", til_settings_get_key(setup.module_settings, 0, NULL)); - exit_if((r = til_fb_new(fb_ops, setup.video, NUM_FB_PAGES, &rototiller.fb)) < 0, + exit_if((r = til_fb_new(fb_ops, setup.video_setup, NUM_FB_PAGES, &rototiller.fb)) < 0, "unable to create fb: %s", strerror(-r)); exit_if(!fps_setup(), diff --git a/src/sdl_fb.c b/src/sdl_fb.c index b260091..8ce133d 100644 --- a/src/sdl_fb.c +++ b/src/sdl_fb.c @@ -10,6 +10,12 @@ /* sdl fb backend, everything sdl-specific in rototiller resides here. */ +typedef struct sdl_fb_setup_t { + til_setup_t til_setup; + int fullscreen; + unsigned width, height; +} sdl_fb_setup_t; + typedef struct sdl_fb_t { unsigned width, height; Uint32 flags; @@ -34,6 +40,7 @@ static int sdl_fb_setup(const til_settings_t *settings, til_setting_t **res_sett NULL }; const char *fullscreen; + const char *size; int r; r = til_settings_get_and_describe_value(settings, @@ -52,8 +59,6 @@ static int sdl_fb_setup(const til_settings_t *settings, til_setting_t **res_sett return r; if (!strcasecmp(fullscreen, "off")) { - const char *size; - r = til_settings_get_and_describe_value(settings, &(til_setting_desc_t){ .name = "SDL window size", @@ -68,6 +73,45 @@ static int sdl_fb_setup(const til_settings_t *settings, til_setting_t **res_sett res_desc); if (r) return r; + } else if ((size = til_settings_get_value(settings, "size", res_setting)) && !(*res_setting)->desc) { + /* if fullscreen=on AND size=WxH is specified, we'll do a more legacy style SDL fullscreen + * where it tries to change the video mode. But if size is unspecified, it'll be a desktop + * style fullscreen where it just uses a fullscreen window in the existing video mode, and + * we won't forcibly require a size= be specified. + */ + /* FIXME TODO: this is all copy-n-pasta grossness that wouldn't need to exist if + * til_settings_get_and_describe_value() just supported optional settings we only + * describe when they're already present. It just needs something like an optional flag, + * to be added in a future commit which will remove this hack. + */ + r = til_setting_desc_clone( &(til_setting_desc_t){ + .name = "SDL window size", + .key = "size", + .regex = "[1-9][0-9]*[xX][1-9][0-9]*", + .preferred = "640x480", + .values = NULL, + .annotations = NULL + }, res_desc); + if (r < 0) + return r; + + return 1; + } + + if (res_setup) { + sdl_fb_setup_t *setup; + + setup = til_setup_new(sizeof(*setup), (void(*)(til_setup_t *))free); + if (!setup) + return -ENOMEM; + + if (!strcasecmp(fullscreen, "on")) + setup->fullscreen = 1; + + if (size) + sscanf(size, "%u%*[xX]%u", &setup->width, &setup->height); + + *res_setup = &setup->til_setup; } return 0; @@ -89,37 +133,28 @@ static int sdl_err_to_errno(int err) } } -static int sdl_fb_init(const til_settings_t *settings, void **res_context) +static int sdl_fb_init(const til_setup_t *setup, void **res_context) { - const char *fullscreen; - const char *size; + sdl_fb_setup_t *s = (sdl_fb_setup_t *)setup; sdl_fb_t *c; int r; - assert(settings); + assert(setup); assert(res_context); - fullscreen = til_settings_get_value(settings, "fullscreen", NULL); - if (!fullscreen) - return -EINVAL; - - size = til_settings_get_value(settings, "size", NULL); - if (!size && !strcasecmp(fullscreen, "off")) - return -EINVAL; - c = calloc(1, sizeof(sdl_fb_t)); if (!c) return -ENOMEM; - if (!strcasecmp(fullscreen, "on")) { - if (!size) - c->flags = SDL_WINDOW_FULLSCREEN_DESKTOP; - else + if (s->fullscreen) { + if (s->width && s->height) c->flags = SDL_WINDOW_FULLSCREEN; + else + c->flags = SDL_WINDOW_FULLSCREEN_DESKTOP; } - if (size) /* TODO: errors */ - sscanf(size, "%u%*[xX]%u", &c->width, &c->height); + c->width = s->width; + c->height = s->height; SDL_SetMainReady(); r = SDL_Init(SDL_INIT_VIDEO); diff --git a/src/til_fb.c b/src/til_fb.c index f6a71a8..503af6a 100644 --- a/src/til_fb.c +++ b/src/til_fb.c @@ -305,7 +305,7 @@ til_fb_t * til_fb_free(til_fb_t *fb) /* create a new fb instance */ -int til_fb_new(const til_fb_ops_t *ops, til_settings_t *settings, int n_pages, til_fb_t **res_fb) +int til_fb_new(const til_fb_ops_t *ops, const til_setup_t *setup, int n_pages, til_fb_t **res_fb) { _til_fb_page_t *page; til_fb_t *fb; @@ -328,7 +328,7 @@ int til_fb_new(const til_fb_ops_t *ops, til_settings_t *settings, int n_pages, t fb->ops = ops; if (ops->init) { - r = ops->init(settings, &fb->ops_context); + r = ops->init(setup, &fb->ops_context); if (r < 0) goto fail; } diff --git a/src/til_fb.h b/src/til_fb.h index a9636e4..c5bf1fe 100644 --- a/src/til_fb.h +++ b/src/til_fb.h @@ -44,7 +44,7 @@ typedef struct til_fb_t til_fb_t; /* Supply this struct to fb_new() with the appropriate context */ typedef struct til_fb_ops_t { int (*setup)(const til_settings_t *settings, til_setting_t **res_setting, const til_setting_desc_t **res_desc, til_setup_t **res_setup); - int (*init)(const til_settings_t *settings, void **res_context); + int (*init)(const til_setup_t *setup, void **res_context); void (*shutdown)(til_fb_t *fb, void *context); int (*acquire)(til_fb_t *fb, void *context, void *page); void (*release)(til_fb_t *fb, void *context); @@ -57,7 +57,7 @@ til_fb_page_t * til_fb_page_get(til_fb_t *fb); void til_fb_page_put(til_fb_t *fb, til_fb_page_t *page); til_fb_t * til_fb_free(til_fb_t *fb); void til_fb_get_put_pages_count(til_fb_t *fb, unsigned *count); -int til_fb_new(const til_fb_ops_t *ops, til_settings_t *settings, int n_pages, til_fb_t **res_fb); +int til_fb_new(const til_fb_ops_t *ops, const til_setup_t *setup, int n_pages, til_fb_t **res_fb); void til_fb_rebuild(til_fb_t *fb); void * til_fb_context(til_fb_t *fb); int til_fb_flip(til_fb_t *fb); diff --git a/src/til_settings.c b/src/til_settings.c index 1d01d8e..01b27b7 100644 --- a/src/til_settings.c +++ b/src/til_settings.c @@ -243,7 +243,7 @@ void til_settings_reset_descs(til_settings_t *settings) /* returns 0 when input settings are complete */ /* returns 1 when input settings are incomplete, storing the next setting's description needed in *next_setting */ /* returns -errno on error */ -int til_settings_apply_desc_generators(const til_settings_t *settings, const til_setting_desc_generator_t generators[], unsigned n_generators, void *setup_context, til_setting_t **res_setting, const til_setting_desc_t **res_desc) +int til_settings_apply_desc_generators(const til_settings_t *settings, const til_setting_desc_generator_t generators[], unsigned n_generators, til_setup_t *setup, til_setting_t **res_setting, const til_setting_desc_t **res_desc, til_setup_t **res_setup) { assert(settings); assert(generators); @@ -256,7 +256,7 @@ int til_settings_apply_desc_generators(const til_settings_t *settings, const til const til_setting_desc_t *desc; int r; - r = g->func(setup_context, &desc); + r = g->func(setup, &desc); if (r < 0) return r; @@ -266,6 +266,9 @@ int til_settings_apply_desc_generators(const til_settings_t *settings, const til return r; } + if (res_setup) + *res_setup = setup; + return 0; } diff --git a/src/til_settings.h b/src/til_settings.h index cbff2a7..4586f11 100644 --- a/src/til_settings.h +++ b/src/til_settings.h @@ -5,6 +5,7 @@ typedef struct til_setting_t til_setting_t; typedef struct til_settings_t til_settings_t; +typedef struct til_setup_t til_setup_t; /* Individual setting description */ typedef struct til_setting_desc_t { @@ -21,7 +22,7 @@ typedef struct til_setting_desc_t { typedef struct til_setting_desc_generator_t { const char *key; /* key this generator applies to */ const char **value_ptr; /* where to put the value */ - int (*func)(void *setup_context, const til_setting_desc_t **res_desc); + int (*func)(til_setup_t *setup_context, const til_setting_desc_t **res_desc); } til_setting_desc_generator_t; /* Encapsulates a single til_settings_t entry */ @@ -40,7 +41,7 @@ int til_settings_add_value(til_settings_t *settings, const char *key, const char void til_settings_reset_descs(til_settings_t *settings); int til_settings_get_and_describe_value(const til_settings_t *settings, const til_setting_desc_t *desc, const char **res_value, til_setting_t **res_setting, const til_setting_desc_t **res_desc); char * til_settings_as_arg(const til_settings_t *settings); -int til_settings_apply_desc_generators(const til_settings_t *settings, const til_setting_desc_generator_t generators[], unsigned n_generators, void *setup_context, til_setting_t **res_setting, const til_setting_desc_t **res_desc); +int til_settings_apply_desc_generators(const til_settings_t *settings, const til_setting_desc_generator_t generators[], unsigned n_generators, til_setup_t *setup, til_setting_t **res_setting, const til_setting_desc_t **res_desc, til_setup_t **res_setup); int til_setting_desc_clone(const til_setting_desc_t *desc, const til_setting_desc_t **res_desc); til_setting_desc_t * til_setting_desc_free(const til_setting_desc_t *desc); -- cgit v1.2.1