On Thu, Sep 8, 2011 at 3:11 PM, Benjamin Franzke
<benjaminfran...@googlemail.com> wrote:
> First thanks for taking this on.
>
> There are some things I'd like to have addtionally/differently:
>
> Supported shm formats are exposed via a "format" event as well
> (like the supported drm formats), so the config creation logic is the
> same for drm and shm, and I think it can remain in native_wayland.c
>
> We need roundtrips to check that we get at least one supported format.
>
> I've attached two patches (heavily based on your last two) that do this,
> are you ok with using them?
That is great. Sure.
I've noted two minor issues or typos, fixed by the attached patch.
One is that param_premultiplied_alpha can be set in the common code
and only when the display has both HAS_ARGB32 and HAS_PREMUL_ARGB32.
The other is that we should not claim PIPE_FORMAT_B8G8R8A8_UNORM
without HAS_ARGB32. If it looks good to you, I will commit an updated
version of your patches.
> 2011/9/8 Chia-I Wu <olva...@gmail.com>:
>> From: Chia-I Wu <o...@lunarg.com>
>>
>> When wl_drm is avaiable and enabled, handle "format" events and return
>> configs for the supported formats. Otherwise, assume all formats of
>> wl_shm are supported.
>> ---
>> .../state_trackers/egl/wayland/native_drm.c | 70
>> +++++++++++++++++++-
>> .../state_trackers/egl/wayland/native_shm.c | 41 +++++++++++-
>> .../state_trackers/egl/wayland/native_wayland.c | 28 +-------
>> .../state_trackers/egl/wayland/native_wayland.h | 4 +-
>> 4 files changed, 113 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c
>> b/src/gallium/state_trackers/egl/wayland/native_drm.c
>> index 05c32f4..facab32 100644
>> --- a/src/gallium/state_trackers/egl/wayland/native_drm.c
>> +++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
>> @@ -58,6 +58,11 @@ struct wayland_drm_display {
>> int fd;
>> char *device_name;
>> boolean authenticated;
>> +
>> + /* supported formats */
>> + boolean argb32;
>> + boolean argb32_pre;
>> + boolean xrgb32;
>> };
>>
>> static INLINE struct wayland_drm_display *
>> @@ -77,8 +82,8 @@ wayland_drm_display_destroy(struct native_display *ndpy)
>> wl_drm_destroy(drmdpy->wl_drm);
>> if (drmdpy->device_name)
>> FREE(drmdpy->device_name);
>> - if (drmdpy->base.config)
>> - FREE(drmdpy->base.config);
>> + if (drmdpy->base.configs)
>> + FREE(drmdpy->base.configs);
>> if (drmdpy->base.own_dpy)
>> wl_display_destroy(drmdpy->base.dpy);
>>
>> @@ -124,6 +129,50 @@ wayland_create_drm_buffer(struct wayland_display
>> *display,
>> width, height, wsh.stride, format);
>> }
>>
>> +static boolean
>> +wayland_drm_display_add_configs(struct wayland_drm_display *drmdpy)
>> +{
>> + struct wayland_config *configs;
>> + enum pipe_format formats[2];
>> + int i, num_formats = 0;
>> +
>> + /*
>> + * Only argb32 counts here. If we make (!argbb32 && argb32_pre) count,
>> we
>> + * will not be able to support the case where
>> + * native_present_control::premultiplied_alpha is FALSE.
>> + */
>> + if (drmdpy->argb32)
>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8A8_UNORM;
>> +
>> + if (drmdpy->xrgb32)
>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8X8_UNORM;
>> +
>> + if (!num_formats)
>> + return FALSE;
>> +
>> + configs = CALLOC(num_formats, sizeof(*configs));
>> + if (!configs)
>> + return FALSE;
>> +
>> + for (i = 0; i < num_formats; i++) {
>> + struct native_config *nconf = &configs[i].base;
>> +
>> + nconf->buffer_mask =
>> + (1 << NATIVE_ATTACHMENT_FRONT_LEFT) |
>> + (1 << NATIVE_ATTACHMENT_BACK_LEFT);
>> +
>> + nconf->color_format = formats[i];
>> +
>> + nconf->window_bit = TRUE;
>> + nconf->pixmap_bit = TRUE;
>> + }
>> +
>> + drmdpy->base.configs = configs;
>> + drmdpy->base.num_configs = num_formats;
>> +
>> + return TRUE;
>> +}
>> +
>> static void
>> drm_handle_device(void *data, struct wl_drm *drm, const char *device)
>> {
>> @@ -148,7 +197,19 @@ drm_handle_device(void *data, struct wl_drm *drm, const
>> char *device)
>> static void
>> drm_handle_format(void *data, struct wl_drm *drm, uint32_t format)
>> {
>> - /* TODO */
>> + struct wayland_drm_display *drmdpy = data;
>> +
>> + switch (format) {
>> + case WL_DRM_FORMAT_ARGB32:
>> + drmdpy->argb32 = TRUE;
>> + break;
>> + case WL_DRM_FORMAT_PREMULTIPLIED_ARGB32:
>> + drmdpy->argb32_pre = TRUE;
>> + break;
>> + case WL_DRM_FORMAT_XRGB32:
>> + drmdpy->xrgb32 = TRUE;
>> + break;
>> + }
>> }
>>
>> static void
>> @@ -191,6 +252,9 @@ wayland_drm_display_init_screen(struct native_display
>> *ndpy)
>> if (!drmdpy->authenticated)
>> return FALSE;
>>
>> + if (!wayland_drm_display_add_configs(drmdpy))
>> + return FALSE;
>> +
>> drmdpy->base.base.screen =
>> drmdpy->event_handler->new_drm_screen(&drmdpy->base.base,
>> NULL, drmdpy->fd);
>> diff --git a/src/gallium/state_trackers/egl/wayland/native_shm.c
>> b/src/gallium/state_trackers/egl/wayland/native_shm.c
>> index 598df9f..5882e74 100644
>> --- a/src/gallium/state_trackers/egl/wayland/native_shm.c
>> +++ b/src/gallium/state_trackers/egl/wayland/native_shm.c
>> @@ -63,8 +63,8 @@ wayland_shm_display_destroy(struct native_display *ndpy)
>> {
>> struct wayland_shm_display *shmdpy = wayland_shm_display(ndpy);
>>
>> - if (shmdpy->base.config)
>> - FREE(shmdpy->base.config);
>> + if (shmdpy->base.configs)
>> + FREE(shmdpy->base.configs);
>> if (shmdpy->base.own_dpy)
>> wl_display_destroy(shmdpy->base.dpy);
>>
>> @@ -111,6 +111,40 @@ wayland_create_shm_buffer(struct wayland_display
>> *display,
>> }
>>
>> static boolean
>> +wayland_shm_display_add_configs(struct wayland_shm_display *shmdpy)
>> +{
>> + struct wayland_config *configs;
>> + enum pipe_format formats[2];
>> + int i, num_formats = 0;
>> +
>> + /* assume all formats are supported */
>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8A8_UNORM;
>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8X8_UNORM;
>> +
>> + configs = CALLOC(num_formats, sizeof(*configs));
>> + if (!configs)
>> + return FALSE;
>> +
>> + for (i = 0; i < num_formats; i++) {
>> + struct native_config *nconf = &configs[i].base;
>> +
>> + nconf->buffer_mask =
>> + (1 << NATIVE_ATTACHMENT_FRONT_LEFT) |
>> + (1 << NATIVE_ATTACHMENT_BACK_LEFT);
>> +
>> + nconf->color_format = formats[i];
>> +
>> + nconf->window_bit = TRUE;
>> + nconf->pixmap_bit = TRUE;
>> + }
>> +
>> + shmdpy->base.configs = configs;
>> + shmdpy->base.num_configs = num_formats;
>> +
>> + return TRUE;
>> +}
>> +
>> +static boolean
>> wayland_shm_display_init_screen(struct native_display *ndpy)
>> {
>> struct wayland_shm_display *shmdpy = wayland_shm_display(ndpy);
>> @@ -128,6 +162,9 @@ wayland_shm_display_init_screen(struct native_display
>> *ndpy)
>> if (!shmdpy->wl_shm)
>> return FALSE;
>>
>> + if (!wayland_shm_display_add_configs(shmdpy))
>> + return FALSE;
>> +
>> winsys = wayland_create_sw_winsys(shmdpy->base.dpy);
>> if (!winsys)
>> return FALSE;
>> diff --git a/src/gallium/state_trackers/egl/wayland/native_wayland.c
>> b/src/gallium/state_trackers/egl/wayland/native_wayland.c
>> index 29c9b46..14cc908 100644
>> --- a/src/gallium/state_trackers/egl/wayland/native_wayland.c
>> +++ b/src/gallium/state_trackers/egl/wayland/native_wayland.c
>> @@ -44,31 +44,11 @@ wayland_display_get_configs (struct native_display
>> *ndpy, int *num_configs)
>> const struct native_config **configs;
>> int i;
>>
>> - if (!display->config) {
>> - struct native_config *nconf;
>> - display->config = CALLOC(2, sizeof(*display->config));
>> - if (!display->config)
>> - return NULL;
>> -
>> - for (i = 0; i < 2; ++i) {
>> - nconf = &display->config[i].base;
>> -
>> - nconf->buffer_mask =
>> - (1 << NATIVE_ATTACHMENT_FRONT_LEFT) |
>> - (1 << NATIVE_ATTACHMENT_BACK_LEFT);
>> -
>> - nconf->window_bit = TRUE;
>> - nconf->pixmap_bit = TRUE;
>> - }
>> -
>> - display->config[0].base.color_format = PIPE_FORMAT_B8G8R8A8_UNORM;
>> - display->config[1].base.color_format = PIPE_FORMAT_B8G8R8X8_UNORM;
>> - }
>> -
>> - configs = MALLOC(2 * sizeof(*configs));
>> + configs = MALLOC(display->num_configs * sizeof(*configs));
>> if (configs) {
>> - configs[0] = &display->config[0].base;
>> - configs[1] = &display->config[1].base;
>> + for (i = 0; i < display->num_configs; i++) {
>> + configs[i] = &display->configs[i].base;
>> + }
>> if (num_configs)
>> *num_configs = 2;
>> }
>> diff --git a/src/gallium/state_trackers/egl/wayland/native_wayland.h
>> b/src/gallium/state_trackers/egl/wayland/native_wayland.h
>> index 5390f2f..93e670b 100644
>> --- a/src/gallium/state_trackers/egl/wayland/native_wayland.h
>> +++ b/src/gallium/state_trackers/egl/wayland/native_wayland.h
>> @@ -39,10 +39,12 @@ struct wayland_surface;
>> struct wayland_display {
>> struct native_display base;
>>
>> - struct wayland_config *config;
>> struct wl_display *dpy;
>> boolean own_dpy;
>>
>> + struct wayland_config *configs;
>> + int num_configs;
>> +
>> struct wl_buffer *(*create_buffer)(struct wayland_display *display,
>> struct wayland_surface *surface,
>> enum native_attachment attachment);
>> --
>> 1.7.5.4
>>
>>
>
--
o...@lunarg.com
From 49970cff79bc42f1a957389ffb4a8b2d0aee93d5 Mon Sep 17 00:00:00 2001
From: Chia-I Wu <o...@lunarg.com>
Date: Thu, 8 Sep 2011 15:32:52 +0800
Subject: [PATCH] st/egl: fix wayland pre-multiplied alpha support
---
.../state_trackers/egl/wayland/native_drm.c | 3 ---
.../state_trackers/egl/wayland/native_shm.c | 3 ---
.../state_trackers/egl/wayland/native_wayland.c | 11 +++++++++--
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c b/src/gallium/state_trackers/egl/wayland/native_drm.c
index 6d19596..5618f3e 100644
--- a/src/gallium/state_trackers/egl/wayland/native_drm.c
+++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
@@ -208,9 +208,6 @@ wayland_drm_display_init_screen(struct native_display *ndpy)
if (drmdpy->base.formats == 0)
return FALSE;
- if (drmdpy->base.formats & HAS_ARGB32)
- drmdpy->base.param_premultiplied_alpha = TRUE;
-
drmdpy->base.base.screen =
drmdpy->event_handler->new_drm_screen(&drmdpy->base.base,
NULL, drmdpy->fd);
diff --git a/src/gallium/state_trackers/egl/wayland/native_shm.c b/src/gallium/state_trackers/egl/wayland/native_shm.c
index 80a4c9b..f9a7d81 100644
--- a/src/gallium/state_trackers/egl/wayland/native_shm.c
+++ b/src/gallium/state_trackers/egl/wayland/native_shm.c
@@ -157,9 +157,6 @@ wayland_shm_display_init_screen(struct native_display *ndpy)
if (shmdpy->base.formats == 0)
return FALSE;
- if (shmdpy->base.formats & HAS_ARGB32)
- shmdpy->base.param_premultiplied_alpha = TRUE;
-
winsys = wayland_create_sw_winsys(shmdpy->base.dpy);
if (!winsys)
return FALSE;
diff --git a/src/gallium/state_trackers/egl/wayland/native_wayland.c b/src/gallium/state_trackers/egl/wayland/native_wayland.c
index db4ccd1..c694293 100644
--- a/src/gallium/state_trackers/egl/wayland/native_wayland.c
+++ b/src/gallium/state_trackers/egl/wayland/native_wayland.c
@@ -41,7 +41,13 @@ const static struct {
enum pipe_format format;
enum wayland_format_flag flag;
} wayland_formats[] = {
- { PIPE_FORMAT_B8G8R8A8_UNORM, HAS_ARGB32 | HAS_PREMUL_ARGB32 },
+ /*
+ * HAS_PREMUL_ARGB32 is ignored here. For the case that HAS_PREMUL_ARGB32
+ * is set but HAS_ARGB32 isn't, we should not claim
+ * PIPE_FORMAT_B8G8R8A8_UNORM support because we will not be able to present
+ * a surface with non-premultiplied alpha.
+ */
+ { PIPE_FORMAT_B8G8R8A8_UNORM, HAS_ARGB32 },
{ PIPE_FORMAT_B8G8R8X8_UNORM, HAS_XRGB32 },
};
@@ -98,7 +104,8 @@ wayland_display_get_param(struct native_display *ndpy,
switch (param) {
case NATIVE_PARAM_PREMULTIPLIED_ALPHA:
- val = display->param_premultiplied_alpha;
+ val = ((display->formats & HAS_ARGB32) &&
+ (display->formats & HAS_PREMUL_ARGB32));
break;
case NATIVE_PARAM_USE_NATIVE_BUFFER:
case NATIVE_PARAM_PRESERVE_BUFFER:
--
1.7.5.4
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
