Hi Kristian, Thanks for your comments. I will follow the mesa development process to add swrast into egl_dri2.
I have used the OO approach to refactor your egl_dri2 driver. Now drm and x11_dri2 shared the dri2_dri2 part, and x11_dri2 and x11_swrast shared the X11 related part. So I have made x11_dri2 inherit both dri2_dri2 and X11. There is a diamond after refactoring and 9 files in the dri2 folder. If we do not want to split this file to that much, and only need 4 files: egl_dri2.h - header file for everything egl_dri2.c - shared code platform_x11.c code related to the x11 platform platform_drm.c code related to the drm platform It is doable. I will move all the dri2 related to egl_dri2.c including dri2_dri2 and dri2_swrast. I will do this when I am back to office after the Chinese New Year Holiday vacation :) Thanks -Haitao -----Original Message----- From: hoegsb...@gmail.com [mailto:hoegsb...@gmail.com] On Behalf Of Kristian Høgsberg Sent: 2011年2月4日 0:55 To: Feng, Haitao Cc: mesa-dev@lists.freedesktop.org Subject: Re: [Mesa-dev] [PATCH 2/2] egl_dri2: Adding swrast and refactoring On Sun, Jan 30, 2011 at 3:05 AM, Feng, Haitao <haitao.f...@intel.com> wrote: > From f723112e2d10429e9691a547baeab588d45511d6 Mon Sep 17 00:00:00 2001 > From: Haitao Feng <haitao.f...@intel.com> > Date: Sat, 29 Jan 2011 23:50:01 -0800 > Subject: [PATCH 2/2] egl_dri2: Adding swrast and refactoring > > This enables the egl_dri2 driver to load swrast driver > for software rendering. It could be used when hardware > dri2 driver is not available, such as in VM. After the > swrast enabling, I refactor the code as egl image functions > are not supported in swrast and egl surface functions > are not supported in drm. After refactoring, egl_dri2.c > contains functions shared between drm, x11_dri2 and > x11_swrast. dri2_shared.c contains functions shared between > drm and x11_dri2. x11_shared.c contains functions shared > between x11_dri2 and x11_swrast. > > Note: this is a candidate for the 7.9 branch. Hi Haitao, Thanks for adding swrast support to egl_dri2.c. The overall approach looks good, though I'm not sure we need to split it up quite that much. However for something like this we really need to break it down into a series of patches. The patch here splits out the different platforms and then adds the swrast functionality, but it would be better if it was a patch to split out shared code, a patch split out the x11 platform, and patch to split out the drm platform, and then the patch to add swrast. It makes it easer to review the patches since all the code motion is in separate patches and the meat of the series, the swrast addition, will have each own patch. It also makes bisecting possible and you're less likely to hit the mailing size limit. Also, the patch looks like it's against mesa 7.9. I know that's what we ship in meego and where we need the swrast functionality, but for submitting upstream, we need a patch against mesa master. We can ship the patch in the meego rpm, but we can't add a feature like this in a stable mesa branch. The patch doesn't apply against mesa master, and since a lot changed in egl_dri2.c since 7.9, I think the best approach is to just redo the split and then add the swrast functionality. I've split up egl_dri2.c in mesa master like this: egl_dri2.h - header file for everything egl_dri2.c - shared code platform_x11.c code related to the x11 platform platform_drm.c code related to the drm platform and I imagine we can just add swrast in platform_x11.c. Is that doable you think? Kristian _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
