[PATCH] egl, wayland: RGB565 format support on Back-buffer

[Bryce Harrington]

On Fri, Feb 06, 2015 at 04:01:51PM -0800, Jon A. Cruz wrote:
> On 02/06/2015 03:50 PM, Bryce Harrington wrote:
> > On Fri, Feb 06, 2015 at 12:29:02PM -0800, Jon A. Cruz wrote:
> >>
> >> My initial concern with this is that it changes from one hardcoding to
> >> two hardcodings. Is 565 the only other format that will need to be
> >> supported? Could XRGB888 or others also be needed?
> >>
> >> If this needs changing it might be good to be sure it is comprehensive.
> > 
> > Yes, that'd be nice, but couldn't those other formats be added as their
> > need crops up?
> 
> If it were started as a solution using case, then yes. But by using the
> conditional operator the later changes would be more significant and
> allow for bugs to creep in.
> 
> Also I do see a list of three formats set for some bitmask elsewhere.
> This then hints that the code cares about at least those three formats.

Yeah I can see the case statement as more future proof.

In any case, this is mesa code.  Dongwon Kim, you'll need to submit this
to the mesa mailing list for inclusion.  You should probably also give
consideration to Jon's review feedback.

Bryce
 
> >> On 02/03/2015 02:52 PM, Dongwon Kim wrote:
> >>> In current code, color format is always hardcoded to __DRI_IMAGE_FORMAT_ARGB8888
> >>> when buffer or DRI image is allocated in function calls, get_back_bo and dri2_get_buffers,
> >>> regardless of current target's color format. This problem may leads to incorrect render
> >>> pitch calculation, which eventually ends up with wrong offset of pixels in the frame buffer
> >>> when the image is in different color format from dri surf's, especially with different bpp.
> >>> (e.g. 16bpp)
> >>>
> >>> Attached code patch simply adds RGB565 case to two functions noted above to resolve
> >>> the issue.
> >>>
> >>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> >>> Signed-off-by: Dongwon Kim <dongwon.kim at intel.com>
> >>> ---
> >>>  src/egl/drivers/dri2/platform_wayland.c |   10 ++++++++--
> >>>  1 file changed, 8 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
> >>> index 3c34e07..78761e2 100644
> >>> --- a/src/egl/drivers/dri2/platform_wayland.c
> >>> +++ b/src/egl/drivers/dri2/platform_wayland.c
> >>> @@ -293,6 +293,11 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
> >>>        dri2_egl_display(dri2_surf->base.Resource.Display);
> >>>     int i;
> >>>  
> >>> +   /* currently supports two DRI image formats, __DRI_IMAGE_FORMAT_RGB565 or __DRI_IMAGE_FORMAT_ARGB8888,
> >>> +    * one of which is selected depending on dri surface format */
> >>> +   const unsigned int dri_image_format =
> >>> +        (dri2_surf->format == WL_DRM_FORMAT_RGB565) ? __DRI_IMAGE_FORMAT_RGB565 : __DRI_IMAGE_FORMAT_ARGB8888;
> >>> +
> >>>     /* We always want to throttle to some event (either a frame callback or
> >>>      * a sync request) after the commit so that we can be sure the
> >>>      * compositor has had a chance to handle it and send us a release event
> >>> @@ -322,7 +327,7 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
> >>>           dri2_dpy->image->createImage(dri2_dpy->dri_screen,
> >>>                                        dri2_surf->base.Width,
> >>>                                        dri2_surf->base.Height,
> >>> -                                      __DRI_IMAGE_FORMAT_ARGB8888,
> >>> +                                      dri_image_format,
> >>>                                        __DRI_IMAGE_USE_SHARE,
> >>>                                        NULL);
> >>>        dri2_surf->back->age = 0;
> >>> @@ -462,9 +467,10 @@ dri2_wl_get_buffers(__DRIdrawable * driDrawable,
> >>>                      unsigned int *attachments, int count,
> >>>                      int *out_count, void *loaderPrivate)
> >>>  {
> >>> +   struct dri2_egl_surface *dri2_surf = loaderPrivate;
> >>>     unsigned int *attachments_with_format;
> >>>     __DRIbuffer *buffer;
> >>> -   const unsigned int format = 32;
> >>> +   const unsigned int format = (dri2_surf->format == WL_DRM_FORMAT_RGB565) ? 16 : 32;
> >>>     int i;
> >>>  
> >>>     attachments_with_format = calloc(count, 2 * sizeof(unsigned int));
> >>>
> >> _______________________________________________
> >> wayland-devel mailing list
> >> wayland-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 
> -- 
> Jon A. Cruz - Senior Open Source Developer
> Samsung Open Source Group
> jonc at osg.samsung.com
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel