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. >> 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 <[email protected]> >>> Signed-off-by: Dongwon Kim <[email protected]> >>> --- >>> 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 >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel -- Jon A. Cruz - Senior Open Source Developer Samsung Open Source Group [email protected] _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
