[PATCH] desktop-shell: provide an option, background-bpp , for specifing the bit depth of the wallpaper

[Pekka Paalanen]

On Mon, 25 May 2015 12:24:42 -0700
Bill Spitzak <spitzak at gmail.com> wrote:

> On 05/25/2015 06:19 AM, nerdopolis wrote:
> 					 &background->bpp, 16);
> > +	/* TODO After Weston 1.8, change default to 32 bit bpp on the wallpaper*/
> > +	if (background->low_bpp != 32) {
> > +	  window_set_preferred_format(background->window,
> > +				      WINDOW_PREFERRED_FORMAT_RGB565);
> > +	} elseif (background->low_bpp != 16 && background->low_bpp != 32 ) {
> > +	  weston_log("invalid background bpp specified in ini file\n");
> > +	}
> 
> I think this would be a little clearer if it read this way:
> 
> 	if (background->bpp == 16) {
> 		... use RGB565 if possible ...
> 	} else if (background->bpp != 32) {
> 		... error ...
> 	}

Agreed. It could even be a switch.

> However it probably would make sense to say this chooses the lowest 
> available value greater or equal to the selected, ie:
> 
> 	if (background->bpp <= 16)
> 		... use RGB565 if possible ...
> 
> I think this is what everybody expects. If this starts supporting 8 bpp 
> or 64 bpp visuals then yes some numbers will change what they get, but 
> the numbers 16 and 32 won't change and I really doubt anybody is going 
> to put 8 in there when they want the 16 bit visual.

I'd rather we just fail hard it is an unsupported value. I'd deal with
this like it's an enumeration where the labels just happen to be also
integers.


Thanks,
pq


PS. People, please use reply-to-all, I'm growing tired of adding back
CC lists.