Menu
▾
▴
mesa3d-dev
|
From: George S. <gsa...@gm...> - 2010-03-14 10:25:39
|
Hi, I put some patches at http://cgit.freedesktop.org/~gsap7/mesa/log/?h=gallium-drisw that do the plumbing in order to build the swrast_dri wrapper around gallium instead of classic mesa. For reference, swrast_dri is the fallback driver loaded by libGL (with LIBGL_ALWAYS_SOFTWARE) and xserver. It must not link with libdrm, nor use any drm headers during building because it is also used on platforms without drm. The branch is to the point where glxinfo runs and needs some glue with __DRIswrastLoaderExtension in order to show something other than black windows. The problem is that there seems to be two places where to put the glue, either in st/.../dri_drawable.c or in ws/.../swrast_drm_api.c . Can someone more familiar with gallium take a look at the branch and provide hints ? thanks, George. |
|
From: Ian R. <id...@fr...> - 2010-03-15 01:12:09
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 George Sapountzis wrote: > Hi, > > I put some patches at > http://cgit.freedesktop.org/~gsap7/mesa/log/?h=gallium-drisw that do > the plumbing in order to build the swrast_dri wrapper around gallium > instead of classic mesa. For reference, swrast_dri is the fallback > driver loaded by libGL (with LIBGL_ALWAYS_SOFTWARE) and xserver. It > must not link with libdrm, nor use any drm headers during building > because it is also used on platforms without drm. > > The branch is to the point where glxinfo runs and needs some glue with > __DRIswrastLoaderExtension in order to show something other than black > windows. The problem is that there seems to be two places where to put > the glue, either in st/.../dri_drawable.c or in > ws/.../swrast_drm_api.c . Can someone more familiar with gallium take > a look at the branch and provide hints ? If you're going to do this, please make a separate driver. Call it swrastg or something. I use swrast for testing new core Mesa features that I implement, and I don't want to be forced to muck about with Gallium to do that. Thanks. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkudiSoACgkQX1gOwKyEAw8lTgCeIivQ/MSPks11BOP6I/JRrCGj rwEAnRkZ0ZYPUFsD3Bs1+qOXjWiTbXaM =T1u6 -----END PGP SIGNATURE----- |
|
From: George S. <gsa...@gm...> - 2010-03-15 09:53:23
|
On Mon, Mar 15, 2010 at 3:11 AM, Ian Romanick <id...@fr...> wrote: > > If you're going to do this, please make a separate driver. Call it > swrastg or something. I use swrast for testing new core Mesa features > that I implement, and I don't want to be forced to muck about with > Gallium to do that. > sure, it will be called swrastg_dri.so and selected with LIBGL_GALLIUM_SOFTWARE or something with lowest priority. |
|
From: Luca B. <luc...@gm...> - 2010-03-15 14:27:12
|
Why not softpipe_dri.so? It would also be nice to have llvmpipe_dri.so (and once it is mature, make this the default one instead of swrast, since it is much faster). |
|
From: George S. <gsa...@gm...> - 2010-03-15 16:32:13
|
On Mon, Mar 15, 2010 at 4:26 PM, Luca Barbieri <luc...@gm...> wrote: > Why not softpipe_dri.so? > It would also be nice to have llvmpipe_dri.so (and once it is mature, > make this the default one instead of swrast, since it is much faster). > it will support both softpipe and llvmpipe, gallium allows interchanging software renderers trivially, swrast_dri is the glue with the DRI "window system". |
|
From: George S. <gsa...@gm...> - 2010-03-19 19:57:49
|
On Sun, Mar 14, 2010 at 12:25 PM, George Sapountzis <gsa...@gm...> wrote: > Hi, > > I put some patches at > http://cgit.freedesktop.org/~gsap7/mesa/log/?h=gallium-drisw that do > the plumbing in order to build the swrast_dri wrapper around gallium > instead of classic mesa. For reference, swrast_dri is the fallback > driver loaded by libGL (with LIBGL_ALWAYS_SOFTWARE) and xserver. It > must not link with libdrm, nor use any drm headers during building > because it is also used on platforms without drm. > > The branch is to the point where glxinfo runs and needs some glue with > __DRIswrastLoaderExtension in order to show something other than black > windows. The problem is that there seems to be two places where to put > the glue, either in st/.../dri_drawable.c or in > ws/.../swrast_drm_api.c . Can someone more familiar with gallium take > a look at the branch and provide hints ? > The timing of the first attempt was unfortunate because it was in the middle of a re-factoring I had not realized it was happening. The good thing is that after the changes by Chia-I and Keith, implementing swrastg_dri.so is much simpler. I update the patches at the above branch, gallium swrast_dri.so works now with the following caveats: * stride: the driver and the loader compute the stride independently. They usually agree, but when you start resizing and they finally don't, you get a regular oblique image. If you run with valgrind, you also get a regular message the size of the mismatch, at each PutImage. * fences: i did not use any, are they needed for cell/llvmpipe ? There are some patches (mostly one-liners) at the start of the branch on other code, please take a look and object. regards, George. |
|
From: Chia-I Wu <ol...@gm...> - 2010-03-21 03:16:14
|
On Fri, Mar 19, 2010 at 09:57:41PM +0200, George Sapountzis wrote: > The timing of the first attempt was unfortunate because it was in the > middle of a re-factoring I had not realized it was happening. The good > thing is that after the changes by Chia-I and Keith, implementing > swrastg_dri.so is much simpler. I update the patches at the above > branch, gallium swrast_dri.so works now with the following caveats: > * stride: the driver and the loader compute the stride independently. > They usually agree, but when you start resizing and they finally > don't, you get a regular oblique image. If you run with valgrind, you > also get a regular message the size of the mismatch, at each PutImage. > * fences: i did not use any, are they needed for cell/llvmpipe ? Nice work. I haven't tried this, but I think there should be DRM_CREATE_DRISW and drisw_api.h, similar to how DRI1 is supported, and let winsys/drm decide which version to support. swrast will be the only one to support DRM_CREATE_DRISW. This will also give st/dri a chance to define whatever needed to swrast to implement displaytarget_display in drisw_api.h, both at drm_api::create_screen time and pipe_screen::flush_frontbuffer time. drisw_st_framebuffer_flush_front will then only prepare a pipe_surface for the given attachment and call pipe_screen::flush_frontbuffer. Similar to what st/glx is doing. This may also solve the resize issue. If it is feasible, I think it is better to have st/dri support all three of DRI1/DRI2/DRISW at the same time when drm.h is available; And have st/dri support only DRISW when drm.h is not available. This will eliminate the need for st/drisw. > There are some patches (mostly one-liners) at the start of the branch > on other code, please take a look and object. -- ol...@Lu... |
|
From: George S. <gsa...@gm...> - 2010-03-21 16:20:03
|
On Sun, Mar 21, 2010 at 4:46 AM, Chia-I Wu <ol...@gm...> wrote: > On Fri, Mar 19, 2010 at 09:57:41PM +0200, George Sapountzis wrote: >> The timing of the first attempt was unfortunate because it was in the >> middle of a re-factoring I had not realized it was happening. The good >> thing is that after the changes by Chia-I and Keith, implementing >> swrastg_dri.so is much simpler. I update the patches at the above >> branch, gallium swrast_dri.so works now with the following caveats: >> * stride: the driver and the loader compute the stride independently. >> They usually agree, but when you start resizing and they finally >> don't, you get a regular oblique image. If you run with valgrind, you >> also get a regular message the size of the mismatch, at each PutImage. >> * fences: i did not use any, are they needed for cell/llvmpipe ? > Nice work. > > I haven't tried this, but I think there should be DRM_CREATE_DRISW and > drisw_api.h, similar to how DRI1 is supported, and let winsys/drm decide > which version to support. swrast will be the only one to support > DRM_CREATE_DRISW. > > This will also give st/dri a chance to define whatever needed to swrast > to implement displaytarget_display in drisw_api.h, both at > drm_api::create_screen time and pipe_screen::flush_frontbuffer time. > drisw_st_framebuffer_flush_front will then only prepare a pipe_surface > for the given attachment and call pipe_screen::flush_frontbuffer. > Similar to what st/glx is doing. This may also solve the resize issue. > Ah ok, I see this is the right way to do it in gallium. The problem is that DRISW, similar to DRI2, will need to call the loader from the winsys, and from a quick look, this is not done for DRI2 either. So this probably will have to wait for the infrastructure to grow for DRI2 and DRISW will be adapted accordingly afterwards. Wrt the resize issue, solving it properly requires adding the stride as a parameter to the putImage hook of the DRISW loader extension. I committed an ugly workaround at ~gsap7/gallium-drisw which works for softpipe but I don't know when it will explode platform-, rendering- or performance-wise. > If it is feasible, I think it is better to have st/dri support all three > of DRI1/DRI2/DRISW at the same time when drm.h is available; And have > st/dri support only DRISW when drm.h is not available. This will > eliminate the need for st/drisw. Yes, it is feasible but it requires mingling with the mesa build system, which I am not too enthusiastic about :-) Thanks for your comments! George. |
|
From: George S. <gsa...@gm...> - 2010-03-23 16:52:48
|
On Sun, Mar 14, 2010 at 12:25 PM, George Sapountzis <gsa...@gm...> wrote: > Hi, > > I put some patches at > http://cgit.freedesktop.org/~gsap7/mesa/log/?h=gallium-drisw that do > the plumbing in order to build the swrast_dri wrapper around gallium > instead of classic mesa. For reference, swrast_dri is the fallback > driver loaded by libGL (with LIBGL_ALWAYS_SOFTWARE) and xserver. It > must not link with libdrm, nor use any drm headers during building > because it is also used on platforms without drm. > I updated the patches with the feedback from the list and added TODO items at the top of drisw.c with the remaining issues. I think it's in good state, given the scope of swrastg_dri.so, and would like to merge this to master. So please review and test, esp. DRI2 and scons (since I cannot test DRI2 and have a tendency to break the build system ...) There is also the issue of when to choose swrastg_dri.so over swrast_dri.so that should be handled by the loaders, just rename swrastg_dri.so to swrast_dri.so for now. regards, George. |
|
From: George S. <gsa...@gm...> - 2010-03-25 15:12:42
|
I pushed this to master, swrastg_dri was added to the autoconf build system only. I also did a merge with gallium-resources for st/dri at ~gsap7/gallium-resources-merge-drisw, it basically reverts the old conversion, merges and redoes the conversion because there were a lot of conflicts, hope this is ok. regards, George. On Tue, Mar 23, 2010 at 6:52 PM, George Sapountzis <gsa...@gm...> wrote: > On Sun, Mar 14, 2010 at 12:25 PM, George Sapountzis > <gsa...@gm...> wrote: >> Hi, >> >> I put some patches at >> http://cgit.freedesktop.org/~gsap7/mesa/log/?h=gallium-drisw that do >> the plumbing in order to build the swrast_dri wrapper around gallium >> instead of classic mesa. For reference, swrast_dri is the fallback >> driver loaded by libGL (with LIBGL_ALWAYS_SOFTWARE) and xserver. It >> must not link with libdrm, nor use any drm headers during building >> because it is also used on platforms without drm. >> > > I updated the patches with the feedback from the list and added TODO > items at the top of drisw.c with the remaining issues. I think it's in > good state, given the scope of swrastg_dri.so, and would like to merge > this to master. So please review and test, esp. DRI2 and scons (since > I cannot test DRI2 and have a tendency to break the build system ...) > > There is also the issue of when to choose swrastg_dri.so over > swrast_dri.so that should be handled by the loaders, just rename > swrastg_dri.so to swrast_dri.so for now. > > regards, > George. > |
|
From: Jakob B. <wal...@gm...> - 2010-03-25 17:58:31
|
On Thu, Mar 25, 2010 at 3:12 PM, George Sapountzis <gsa...@gm...> wrote: > I pushed this to master, swrastg_dri was added to the autoconf build > system only. > > I also did a merge with gallium-resources for st/dri at > ~gsap7/gallium-resources-merge-drisw, it basically reverts the old > conversion, merges and redoes the conversion because there were a lot > of conflicts, hope this is ok. Hi George, Let me first of say that despite my negative tone in the following text I do really like what you have done, thanks for figuring it out and doing the work. I really don't like the way you reuse the dri state tracker file to produce two different o file form the same c file. Magic defines on the build line that make the c file take different paths make the code harder to read and there is a big chance that some paths just bit rot. However to does seems to be very limited. And I also realize that the is no way around it due to the fact that you have to work against two different dri_util.h files and as such two different dri interfaces. I have attached 4 patches that I like to run by you before pushing. 0001 is rather trivial and is just a code cleanup that adds dri2 prefix for functions that are in the dri2.c file. 0002 just removes the drisw.h from the drm build and dri[1|2].h from the sw build, just to be extra sure we don't use the wrong functions in the wrong place (you get a build time warning plus a link error, instead of just a link error). 0003 moves drisw into dri/sw and the drm dri files into dri/drm, yet again for clarity. 0004 moves the common files into a common directory. One thing that could be done to reduce the amount of #ifdefs would be to move the tables from dri_screen.c into drisw.c and dri2.c. And create a "virtual" function on the screen for flush_frontbuffer alloc_texture and so on. Comments please? Cheers Jakob. PS: I hope I didn't sound to negative... |
|
From: George S. <gsa...@gm...> - 2010-03-25 19:00:31
|
On Thu, Mar 25, 2010 at 7:58 PM, Jakob Bornecrantz <wal...@gm...> wrote: > On Thu, Mar 25, 2010 at 3:12 PM, George Sapountzis > <gsa...@gm...> wrote: >> I pushed this to master, swrastg_dri was added to the autoconf build >> system only. >> >> I also did a merge with gallium-resources for st/dri at >> ~gsap7/gallium-resources-merge-drisw, it basically reverts the old >> conversion, merges and redoes the conversion because there were a lot >> of conflicts, hope this is ok. > > Hi George, > > Let me first of say that despite my negative tone in the following > text I do really like what you have done, thanks for figuring it out > and doing the work. > > I really don't like the way you reuse the dri state tracker file to > produce two different o file form the same c file. Magic defines on > the build line that make the c file take different paths make the code > harder to read and there is a big chance that some paths just bit rot. > However to does seems to be very limited. And I also realize that the > is no way around it due to the fact that you have to work against two > different dri_util.h files and as such two different dri interfaces. > I agree it's not pretty either. It's basically like having different CFLAGS for multiple .la's in automake. It's "limited" to init_screen, flush_front/swap_buffers/alloc_buffers which is exactly where the DRI versions are pretty different. > I have attached 4 patches that I like to run by you before pushing. > 0001 is rather trivial and is just a code cleanup that adds dri2 > prefix for functions that are in the dri2.c file. 0002 just removes > the drisw.h from the drm build and dri[1|2].h from the sw build, just > to be extra sure we don't use the wrong functions in the wrong place > (you get a build time warning plus a link error, instead of just a > link error). > These look good. > 0003 moves drisw into dri/sw and the drm dri files into dri/drm, yet > again for clarity. > > 0004 moves the common files into a common directory. > I think that it may be better to just consolidate dri_screen.c / dri_context.c / dri_drawable.c / dri_extension.c in a single file named dri.c or dri_api.c or dri_driver_api.c similar to the glx/xlib state_tracker. There is not much left in the above files after splitting out version-specific code. Then you will have: dri_api.c or dri_driver_api.c for the DRI "window system" binding dri_st_api.c and dri1.c dri2.c drisw.c for the different versions the only outlier is dri1_helper which I agree may have a better name. But then I looked at this a lot lately and your suggestion may be better for someone looking at it after some time. > One thing that could be done to reduce the amount of #ifdefs would be > to move the tables from dri_screen.c into drisw.c and dri2.c. yes, this could be done unless the suggestion above is adopted, in which case I thinks it's probably better to keep all the dri_ functions static and the _DriverAPI tables in the common file. > And > create a "virtual" function on the screen for flush_frontbuffer > alloc_texture and so on. > This vtbl already exists, it's the st_framebuffer_iface. However you implement this, I think you would end up with either an ifdef or having the same symbol in the different DRI version files, I don't know which is better. Thanks for looking at the patches, George |
|
From: Jakob B. <wal...@gm...> - 2010-03-25 19:49:58
|
On Thu, Mar 25, 2010 at 7:00 PM, George Sapountzis <gsa...@gm...> wrote: > On Thu, Mar 25, 2010 at 7:58 PM, Jakob Bornecrantz <wal...@gm...> wrote: >> On Thu, Mar 25, 2010 at 3:12 PM, George Sapountzis >> <gsa...@gm...> wrote: >>> I pushed this to master, swrastg_dri was added to the autoconf build >>> system only. >>> >>> I also did a merge with gallium-resources for st/dri at >>> ~gsap7/gallium-resources-merge-drisw, it basically reverts the old >>> conversion, merges and redoes the conversion because there were a lot >>> of conflicts, hope this is ok. >> >> Hi George, >> >> Let me first of say that despite my negative tone in the following >> text I do really like what you have done, thanks for figuring it out >> and doing the work. >> >> I really don't like the way you reuse the dri state tracker file to >> produce two different o file form the same c file. Magic defines on >> the build line that make the c file take different paths make the code >> harder to read and there is a big chance that some paths just bit rot. >> However to does seems to be very limited. And I also realize that the >> is no way around it due to the fact that you have to work against two >> different dri_util.h files and as such two different dri interfaces. >> > > I agree it's not pretty either. It's basically like having different > CFLAGS for multiple .la's in automake. It's "limited" to init_screen, > flush_front/swap_buffers/alloc_buffers which is exactly where the DRI > versions are pretty different. > Yeah its not as bad as it could be, and there really isn't any way around it with regard to that drisw_util.h vs dri_util.h. > >> I have attached 4 patches that I like to run by you before pushing. >> 0001 is rather trivial and is just a code cleanup that adds dri2 >> prefix for functions that are in the dri2.c file. 0002 just removes >> the drisw.h from the drm build and dri[1|2].h from the sw build, just >> to be extra sure we don't use the wrong functions in the wrong place >> (you get a build time warning plus a link error, instead of just a >> link error). >> > > These look good. Cool I'll push those. > >> 0003 moves drisw into dri/sw and the drm dri files into dri/drm, yet >> again for clarity. >> >> 0004 moves the common files into a common directory. >> > > I think that it may be better to just consolidate dri_screen.c / > dri_context.c / dri_drawable.c / dri_extension.c in a single file > named dri.c or dri_api.c or dri_driver_api.c similar to the glx/xlib > state_tracker. There is not much left in the above files after > splitting out version-specific code. > > Then you will have: > dri_api.c or dri_driver_api.c for the DRI "window system" binding > dri_st_api.c > and > dri1.c dri2.c drisw.c for the different versions > > the only outlier is dri1_helper which I agree may have a better name. > > But then I looked at this a lot lately and your suggestion may be > better for someone looking at it after some time. I think you can do both, the moving of the files is more for making it blatantly clear what is going on. The common files live in the common directory and the rest are in their own directory. This mirrors what is done in egl. I'm ambivalent about grouping the files together. And I don't think that moving them it a common directory stops us from doing either one. Then again dri_extension.c is quite useless, and should probably be moved into dri_screen either way. Are you strongly against the layout that I suggested? I have no real problem with doing both? > >> One thing that could be done to reduce the amount of #ifdefs would be >> to move the tables from dri_screen.c into drisw.c and dri2.c. > > yes, this could be done unless the suggestion above is adopted, in > which case I thinks it's probably better to keep all the dri_ > functions static and the _DriverAPI tables in the common file. There is only one function that is static that is used in the tables, or are you meaning that we should make them static as we move all of them into a single file? > >> And >> create a "virtual" function on the screen for flush_frontbuffer >> alloc_texture and so on. >> > > This vtbl already exists, it's the st_framebuffer_iface. However you > implement this, I think you would end up with either an ifdef or > having the same symbol in the different DRI version files, I don't > know which is better. Ah yes double vtable, hmm, well anyways its not that invasive. > > Thanks for looking at the patches, No problems, thanks for looking at mine :) Cheers Jakob. |
|
From: George S. <gsa...@gm...> - 2010-03-25 23:22:50
|
On Thu, Mar 25, 2010 at 9:49 PM, Jakob Bornecrantz <wal...@gm...> wrote: > On Thu, Mar 25, 2010 at 7:00 PM, George Sapountzis > <gsa...@gm...> wrote: >> On Thu, Mar 25, 2010 at 7:58 PM, Jakob Bornecrantz <wal...@gm...> wrote: >>> 0003 moves drisw into dri/sw and the drm dri files into dri/drm, yet >>> again for clarity. >>> >>> 0004 moves the common files into a common directory. >>> >> >> I think that it may be better to just consolidate dri_screen.c / >> dri_context.c / dri_drawable.c / dri_extension.c in a single file >> named dri.c or dri_api.c or dri_driver_api.c similar to the glx/xlib >> state_tracker. There is not much left in the above files after >> splitting out version-specific code. >> >> Then you will have: >> dri_api.c or dri_driver_api.c for the DRI "window system" binding >> dri_st_api.c >> and >> dri1.c dri2.c drisw.c for the different versions >> >> the only outlier is dri1_helper which I agree may have a better name. >> >> But then I looked at this a lot lately and your suggestion may be >> better for someone looking at it after some time. > > I think you can do both, the moving of the files is more for making it > blatantly clear what is going on. The common files live in the common > directory and the rest are in their own directory. This mirrors what > is done in egl. > > I'm ambivalent about grouping the files together. And I don't think > that moving them it a common directory stops us from doing either one. > Then again dri_extension.c is quite useless, and should probably be > moved into dri_screen either way. > > Are you strongly against the layout that I suggested? I have no real > problem with doing both? > Ah ok, then the suggested layout serves well its purpose. Wrt to consilidating in dri_driver_api.c (dri_util.c is effectively dri_api.c), it depends on what your plans are for st/dri and if it involves adding more common code, I don't have other plans for st/dri :-) I'll add a comment about the ifdef's (srceen / swap_buffers in dri_screen.c, flush_front / alloc_buffers in dri_st_api.c) and to drisw_util.h about its relationship to dri_util.h . >> >>> One thing that could be done to reduce the amount of #ifdefs would be >>> to move the tables from dri_screen.c into drisw.c and dri2.c. >> >> yes, this could be done unless the suggestion above is adopted, in >> which case I thinks it's probably better to keep all the dri_ >> functions static and the _DriverAPI tables in the common file. > > There is only one function that is static that is used in the tables, > or are you meaning that we should make them static as we move all of > them into a single file? > yes, if we move them all to dri_driver_api.c then it makes more sense to make them static and keep _DriverAPI in dri_driver_api.c with an ifdef. |
|
From: Jakob B. <wal...@gm...> - 2010-03-26 00:12:58
|
On Thu, Mar 25, 2010 at 11:22 PM, George Sapountzis <gsa...@gm...> wrote: > On Thu, Mar 25, 2010 at 9:49 PM, Jakob Bornecrantz <wal...@gm...> wrote: >> On Thu, Mar 25, 2010 at 7:00 PM, George Sapountzis >> <gsa...@gm...> wrote: >>> On Thu, Mar 25, 2010 at 7:58 PM, Jakob Bornecrantz <wal...@gm...> wrote: >>>> 0003 moves drisw into dri/sw and the drm dri files into dri/drm, yet >>>> again for clarity. >>>> >>>> 0004 moves the common files into a common directory. >>>> >>> >>> I think that it may be better to just consolidate dri_screen.c / >>> dri_context.c / dri_drawable.c / dri_extension.c in a single file >>> named dri.c or dri_api.c or dri_driver_api.c similar to the glx/xlib >>> state_tracker. There is not much left in the above files after >>> splitting out version-specific code. >>> >>> Then you will have: >>> dri_api.c or dri_driver_api.c for the DRI "window system" binding >>> dri_st_api.c >>> and >>> dri1.c dri2.c drisw.c for the different versions >>> >>> the only outlier is dri1_helper which I agree may have a better name. >>> >>> But then I looked at this a lot lately and your suggestion may be >>> better for someone looking at it after some time. >> >> I think you can do both, the moving of the files is more for making it >> blatantly clear what is going on. The common files live in the common >> directory and the rest are in their own directory. This mirrors what >> is done in egl. >> >> I'm ambivalent about grouping the files together. And I don't think >> that moving them it a common directory stops us from doing either one. >> Then again dri_extension.c is quite useless, and should probably be >> moved into dri_screen either way. >> >> Are you strongly against the layout that I suggested? I have no real >> problem with doing both? >> > > Ah ok, then the suggested layout serves well its purpose. Wrt to > consilidating in dri_driver_api.c (dri_util.c is effectively > dri_api.c), it depends on what your plans are for st/dri and if it > involves adding more common code, I don't have other plans for st/dri > :-) Thanks, pushed. I don't know, I thought about trying to add EGL Image support to st/dri once it lands in st_api.h and st/egl. And thinking about it I'm leaning more towards keeping them separated. This is the traditionalist in me speaking, keeping code related to one object in each file (screen, context, drawable). This keeps things in line with the rest of the traditional dri drivers. We could merge in dri_extensions.c into dri_context.c and dri_st_api.c into dri_drawable.c and dri_screen.c, if it is the amount of files that you are worried about (in fact I prefer that over just keeping at as it is). Also I files above 1k loc board always feels a bit intimating :-). Then again this is not a strong feeling, so if you have strong opinions or it hampers your development please go ahead and change. Some improvement should be made tho. > > I'll add a comment about the ifdef's (srceen / swap_buffers in > dri_screen.c, flush_front / alloc_buffers in dri_st_api.c) and to > drisw_util.h about its relationship to dri_util.h . > >>> >>>> One thing that could be done to reduce the amount of #ifdefs would be >>>> to move the tables from dri_screen.c into drisw.c and dri2.c. >>> >>> yes, this could be done unless the suggestion above is adopted, in >>> which case I thinks it's probably better to keep all the dri_ >>> functions static and the _DriverAPI tables in the common file. >> >> There is only one function that is static that is used in the tables, >> or are you meaning that we should make them static as we move all of >> them into a single file? >> > > yes, if we move them all to dri_driver_api.c then it makes more sense > to make them static and keep _DriverAPI in dri_driver_api.c with an > ifdef. > With the -fvisibility=invisible flag given to gcc all those symbols are pretty much static. Again not strong opinions. Thanks again for the work you did. Cheers Jakob. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X