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.

7 files changed, 115 insertions, 72 deletions

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