dri span patches...
here's a patch which mainly does 3 things: - convert sis, mach64, and radeon to spantmp2. The sis and mach64 drivers got a slight change, previously you could not read back alpha values (always 0xff) and I don't think there was a good reason for that? The 3 intel drivers and the s3v are not converted due to their support of a RGB555 format spantmp2 can't handle (actually, the i810 does not support the 555 format in its span functions, the driver elsewhere has support for it, this probably can't be right). tdfx and ffb are not converted since they support a 888 format. gamma is not converted since it performs some unusual always-round-up tricks when using RGB565 format. trident is not converted since it doesn't support span functions yet at all :-). - remove HW_CLIPLOOP, HW_ENDCLIPLOOP, CLIPPIXEL and CLIPSPAN macros out of the drivers whenever possible, since they are almost always identical. ffb uses very different macros, and some drivers do not use the driDrawable struct for HW_CLIPLOOP (which btw is interesting, I'm not quite sure why some drivers need to handle that differently, I didn't look too much into it but the difference is due to the handling of front and back buffers somehow), so they keep their own definitions hopefully. There are probably more macro definitions which could be factored out. - change CLIPSPAN to never return negative number of pixels (only change that once! not a dozen times, which is why I've done that macro definition removal in the first place...). Does this look sane? Driver source code should get quite a bit smaller overall. Roland Index: common/depthtmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/depthtmp.h,v retrieving revision 1.7 diff -u -r1.7 depthtmp.h --- common/depthtmp.h 14 Feb 2005 09:27:38 - 1.7 +++ common/depthtmp.h 3 Mar 2005 03:24:43 - @@ -1,24 +1,19 @@ /* $XFree86: xc/lib/GL/mesa/src/drv/common/depthtmp.h,v 1.5 2001/03/21 16:14:20 dawes Exp $ */ +#include "spantmp_common.h" + #ifndef DBG #define DBG 0 #endif - #ifndef HAVE_HW_DEPTH_SPANS #define HAVE_HW_DEPTH_SPANS 0 #endif + #ifndef HAVE_HW_DEPTH_PIXELS #define HAVE_HW_DEPTH_PIXELS 0 #endif -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - static void TAG(WriteDepthSpan)( GLcontext *ctx, GLuint n, GLint x, GLint y, const GLdepth *depth, Index: common/spantmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/spantmp.h,v retrieving revision 1.5 diff -u -r1.5 spantmp.h --- common/spantmp.h14 Feb 2005 09:27:38 - 1.5 +++ common/spantmp.h3 Mar 2005 03:24:43 - @@ -27,26 +27,12 @@ *Gareth Hughes <[EMAIL PROTECTED]> */ +#include "spantmp_common.h" + #ifndef DBG #define DBG 0 #endif -#ifndef HW_WRITE_LOCK -#define HW_WRITE_LOCK()HW_LOCK() -#endif - -#ifndef HW_WRITE_UNLOCK -#define HW_WRITE_UNLOCK() HW_UNLOCK() -#endif - -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif - -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - #ifndef HW_READ_CLIPLOOP #define HW_READ_CLIPLOOP() HW_CLIPLOOP() #endif Index: common/spantmp2.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/spantmp2.h,v retrieving revision 1.6 diff -u -r1.6 spantmp2.h --- common/spantmp2.h 15 Jan 2005 14:35:35 - 1.6 +++ common/spantmp2.h 3 Mar 2005 03:24:43 - @@ -34,27 +34,12 @@ */ #include "colormac.h" +#include "spantmp_common.h" #ifndef DBG #define DBG 0 #endif -#ifndef HW_WRITE_LOCK -#define HW_WRITE_LOCK()HW_LOCK() -#endif - -#ifndef HW_WRITE_UNLOCK -#define HW_WRITE_UNLOCK() HW_UNLOCK() -#endif - -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif - -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - #ifndef HW_READ_CLIPLOOP #define HW_READ_CLIPLOOP() HW_CLIPLOOP() #endif Index: common/stenciltmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/stenciltmp.h,v retrieving revision 1.4 diff -u -r1.4 stenciltmp.h --- common/stenciltmp.h 14 Feb 2005 09:27:38 - 1.4 +++ common/stenciltmp.h 3 Mar 2005 03:24:44 - @@ -1,23 +1,11 @@ /* $XFree86: xc/lib/GL/mesa/src/drv/common/stenciltmp.h,v 1.3 2001/03/21 16:14:20 dawes Exp $ */ +#include "spantmp_common.h" + #ifndef DBG #define DBG 0 #endif -#ifndef HW_WRITE_LOCK -#define HW_WRITE_LOCK()HW_LOCK() -#endif -#ifndef HW_WRITE_UNLOCK -#define HW_WRITE_UNLOCK() HW_UNLOCK() -#endif - -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif -
Re: dri span patches...
On Thu, Mar 03, 2005 at 04:48:58AM +0100, Roland Scheidegger wrote: > here's a patch which mainly does 3 things: > - convert sis, mach64, and radeon to spantmp2. > The sis and mach64 drivers got a slight change, previously you could not > read back alpha values (always 0xff) and I don't think there was a good > reason for that? AFAICS mach64 doesn't apply either the blend equation or the blend functions to the alpha values. You can choose to write either 0, 1, As, 1-As, Ad or 1-Ad to the destination :( -- Ville Syrjälä [EMAIL PROTECTED] http://www.sci.fi/~syrjala/ --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
On Thu, Mar 03, 2005 at 10:04:02AM +0200, Ville Syrjälä wrote: > On Thu, Mar 03, 2005 at 04:48:58AM +0100, Roland Scheidegger wrote: > > here's a patch which mainly does 3 things: > > - convert sis, mach64, and radeon to spantmp2. > > The sis and mach64 drivers got a slight change, previously you could not > > read back alpha values (always 0xff) and I don't think there was a good > > reason for that? > > AFAICS mach64 doesn't apply either the blend equation or the blend > functions to the alpha values. You can choose to write either 0, 1, As, > 1-As, Ad or 1-Ad to the destination :( Oh and a quick look at the mach64 driver indicats that it always chooses to write 0. -- Ville Syrjälä [EMAIL PROTECTED] http://www.sci.fi/~syrjala/ --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
On Thu, Mar 03, 2005 at 04:48:58AM +0100, Roland Scheidegger wrote: > here's a patch which mainly does 3 things: > - convert sis, mach64, and radeon to spantmp2. > The sis and mach64 drivers got a slight change, previously you could not > read back alpha values (always 0xff) and I don't think there was a good > reason for that? > The 3 intel drivers and the s3v are not converted due to their support > of a RGB555 format spantmp2 can't handle (actually, the i810 does not > support the 555 format in its span functions, the driver elsewhere has > support for it, this probably can't be right). tdfx and ffb are not > converted since they support a 888 format. gamma is not converted since > it performs some unusual always-round-up tricks when using RGB565 > format. trident is not converted since it doesn't support span functions > yet at all :-). > > - remove HW_CLIPLOOP, HW_ENDCLIPLOOP, CLIPPIXEL and CLIPSPAN macros out > of the drivers whenever possible, since they are almost always > identical. ffb uses very different macros, and some drivers do not use > the driDrawable struct for HW_CLIPLOOP (which btw is interesting, I'm > not quite sure why some drivers need to handle that differently, I > didn't look too much into it but the difference is due to the handling > of front and back buffers somehow), so they keep their own definitions > hopefully. > There are probably more macro definitions which could be factored out. > > - change CLIPSPAN to never return negative number of pixels (only change > that once! not a dozen times, which is why I've done that macro > definition removal in the first place...). > > Does this look sane? Driver source code should get quite a bit smaller > overall. It looks o.k. and if there's hardware that's more obscure than the common cases then they'd have their own span routines anyway. Just watch out on the macro expansion though. You've got this currently... #define GET_SRC_PTR(_x, _y) (read_buf + _x * 2 + _y * pitch) and it should be... #define GET_SRC_PTR(_x, _y) (read_buf + (_x) * 2 + (_y) * pitch) Alan. --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
Roland Scheidegger wrote: Brian Paul wrote: Roland Scheidegger wrote: here's a patch which mainly does 3 things: - convert sis, mach64, and radeon to spantmp2. The sis and mach64 drivers got a slight change, previously you could not read back alpha values (always 0xff) and I don't think there was a good reason for that? IIRC, the mach64 doesn't support destination alpha planes. OpenGL requires that reads of absent alpha planes returns 1.0. I don't know if the SiS chip is the same. Are you sure? At least the driver handles things like GL_DST_ALPHA blend factors, wouldn't you get awfully bogus results if it wouldn't support destination alpha planes in that case? It does not export BGRA visuals however for some reason. Note that r128 driver (which was converted to use spantmp2 by Ian and thus got that same change in behaviour reading back RGBA pixels already) does not export visuals with a alpha component neither (just BGR 888, if I read that right). SIS OTOH exports BGRA visuals. Like I said, "IIRC". If you run the Mesa 'reflect' demo and hit 'a' you should see the contents of the alpha channel. The demo may need to be hacked to request GLUT_ALPHA in glutInitDisplayMode. But that'll show if you've got a functioning alpha channel. Maybe someone with a mach64 or sis can try that. Does this look sane? Driver source code should get quite a bit smaller overall. As long as you're looking at the span functions I should tell you that at some point in the future, work on the GL_EXT_framebuffer_object extension will likely require some changes in this area. I'll elaborate on that in another email when I have things better sorted out. I'm doing initial implementation with the xlib renderer now. But one thing that seems to be wrong in the r200 driver (for example) (and probably all the other DRI drivers) is the fact that the r200SetBuffer() function ignores its parameter. As it is now, the function ignores that parameter and always assumes that the span functions are going to be reading/writing to the context's current drawing buffer. The glXMakeContextCurrent and glXMakeCurrentReadSGI functions specify separate 'read' and 'draw' drawables. This allows you to do a glCopyPixels from one window to another, for example. This won't work if the colorBuffer parameter of r200SetBuffer() is ignored. In general, the span functions are relying too much on the GLcontext state, and not enough on the GLframebuffer state. The pointer to the start of the color/depth/stencil buffer, its width, height, pitch, etc. should be obtained via the GLframebuffer passed to r200SetBuffer. To do that, I guess GLframebuffer would need to have a pointer back to the corresponding __DRIDrawablePrivate, since we have no way to contain one in the other. Anyway, I wouldn't change anything right now. Let's wait until the GL_EXT_framebuffer_objects stuff has firmed up. I do not think these changes should interfere with that however, since the code generated should not change at all, it's just unuglifying by moving macros around (except for those drivers which got converted to use spantmp2, but you'll need to make it work with that anyway if somehow that template won't be able to deal with these necessary changes). I meant to say that you should go ahead with your changes now (as far as I'm concerned), and don't worry about the GL_EXT_renderbuffer_object stuff or the ignored GLframebuffer parameter for now. Sorry for not being clear. -Brian --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
On Thu, 03 Mar 2005 17:45:15 +0100, Roland Scheidegger <[EMAIL PROTECTED]> wrote: > Brian Paul wrote: > > Roland Scheidegger wrote: > > > >> here's a patch which mainly does 3 things: - convert sis, mach64, > >> and radeon to spantmp2. The sis and mach64 drivers got a slight > >> change, previously you could not read back alpha values (always > >> 0xff) and I don't think there was a good reason for that? > > > > > > IIRC, the mach64 doesn't support destination alpha planes. OpenGL > > requires that reads of absent alpha planes returns 1.0. I don't know > > if the SiS chip is the same. > Are you sure? At least the driver handles things like GL_DST_ALPHA blend > factors, wouldn't you get awfully bogus results if it wouldn't support > destination alpha planes in that case? It does not export BGRA visuals > however for some reason. Note that r128 driver (which was converted to > use spantmp2 by Ian and thus got that same change in behaviour reading > back RGBA pixels already) does not export visuals with a alpha component > neither (just BGR 888, if I read that right). SIS OTOH exports BGRA visuals. > I think the mach64 had some limitations regarding alpha. I think it could only do fog or alpha, but not both at the same time. perhaps what you are seeing is a result of that. Alex --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
On Thu, Mar 03, 2005 at 05:45:15PM +0100, Roland Scheidegger wrote: > Brian Paul wrote: > >Roland Scheidegger wrote: > > > >>here's a patch which mainly does 3 things: - convert sis, mach64, > >>and radeon to spantmp2. The sis and mach64 drivers got a slight > >>change, previously you could not read back alpha values (always > >>0xff) and I don't think there was a good reason for that? > > > > > >IIRC, the mach64 doesn't support destination alpha planes. OpenGL > >requires that reads of absent alpha planes returns 1.0. I don't know > >if the SiS chip is the same. > Are you sure? At least the driver handles things like GL_DST_ALPHA blend > factors, wouldn't you get awfully bogus results if it wouldn't support > destination alpha planes in that case? Like I said before only the RGB components are blended. You can choose to write 0, 1, As, 1-As, Ad or 1-Ad to the destination alpha ([EMAIL PROTECTED] register). Currently the driver seems to write 0. It would probably be a better idea to write 1 instead. -- Ville Syrjälä [EMAIL PROTECTED] http://www.sci.fi/~syrjala/ --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
Am Donnerstag, 3. März 2005 4:48 schrieb Roland Scheidegger: > Roland Scheidegger wrote: > here's a patch which mainly does 3 things: - convert sis, mach64, > and radeon to spantmp2. The sis and mach64 drivers got a slight > change, previously you could not read back alpha values (always > 0xff) and I don't think there was a good reason for that? Roland, do you had a look on Felix's spantmp2.diff, too. http://marc.theaimsgroup.com/?l=dri-devel&m=110600974029291&w=2 I have it running with r200 (32 bpp) but do not see big changes (slowdown or speedup). BTW Can you please send me a private copy of dri-span3.diff, again. There are many ugly "=3D" characters in the archived version. -Dieter --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
Ville Syrjälä wrote: Like I said before only the RGB components are blended. You can choose to write 0, 1, As, 1-As, Ad or 1-Ad to the destination alpha ([EMAIL PROTECTED] register). Currently the driver seems to write 0. It would probably be a better idea to write 1 instead. Sorry, but I just can't see that in the driver. And there's no ALPHA_DST_SEL bit, at least not in the mach64 reg file I have... I was just looking at the specs :) They are named MACH64_ALPHA_DST_* in mach64_reg.h. The driver doesn't explicitly specify any value which means 0 gets written. I actually just stumbled on this issue a few days ago with the mach64 DirectFB driver. My plan for the DirectFB driver is simply to allow ZERO + ZERO/ONE/SRCALPHASAT blend functions for destinations with alpha. It's a rather serious limitation but I think it's better than incorrect rendering. The issue is even worse on older Rage chips since they can only write 0 to the destination alpha. But that is not an issue for the DRI driver since those chips aren't supported. I think now I understand. It has alpha channel and all, but it simply won't perform the blending equation on the alpha channel, instead simply writing zero, one, source alpha, 1 - minus source alpha, dst alpha, or 1 - dst alpha. Actually the driver does not write 0, it writes the source alpha value (MACH64_ALPHA_DST_SRCALPHA) as far as I can tell. Actually, this design means it would have some very limited support for blend_func_separate :-). Looks like a stupid design limitation to me (what would it cost to implement that additional blend adder to the 3 you need anyway?), but ah well. Maybe this wasn't required by DirectX 1.0 ;-). In practice though, this might just work quite often, the alpha-blended alpha values are probably not required a lot? Regardless if it can actually blend alpha values or not, there would be some half-way useful alpha values probably in the buffer. Either you get the correct results or the wrong results. Not sure if there are any really useful things you can do with incorrect values. I meant that you might just get the correct alpha values sometimes (depending on the blend func that should be true I guess). Roland --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
Dieter Nützel wrote: You'll even get a newer version, Alan pointed out some subtle issues with the macro expansion (one of the reasons I don't particularly like macros...). Instead of fixing all GET_SRC/DST_PTR macros, I got rid of them too, since they were identical again in all drivers which use spantmp2 (except unichrome which uses different pitches for reading and drawing, it keeps its macros). Sorry, you lost spantmp_common.h in dri_span4.diff. You're right. I really wanted to do a cvs diff to include new files, but I guess it's not possible, and hacking together the diffs manually is just asking for such errors :-). Roland Index: common/depthtmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/depthtmp.h,v retrieving revision 1.7 diff -u -r1.7 depthtmp.h --- common/depthtmp.h 14 Feb 2005 09:27:38 - 1.7 +++ common/depthtmp.h 3 Mar 2005 19:25:50 - @@ -1,24 +1,19 @@ /* $XFree86: xc/lib/GL/mesa/src/drv/common/depthtmp.h,v 1.5 2001/03/21 16:14:20 dawes Exp $ */ +#include "spantmp_common.h" + #ifndef DBG #define DBG 0 #endif - #ifndef HAVE_HW_DEPTH_SPANS #define HAVE_HW_DEPTH_SPANS 0 #endif + #ifndef HAVE_HW_DEPTH_PIXELS #define HAVE_HW_DEPTH_PIXELS 0 #endif -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - static void TAG(WriteDepthSpan)( GLcontext *ctx, GLuint n, GLint x, GLint y, const GLdepth *depth, Index: common/spantmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/spantmp.h,v retrieving revision 1.5 diff -u -r1.5 spantmp.h --- common/spantmp.h14 Feb 2005 09:27:38 - 1.5 +++ common/spantmp.h3 Mar 2005 19:25:50 - @@ -27,26 +27,12 @@ *Gareth Hughes <[EMAIL PROTECTED]> */ +#include "spantmp_common.h" + #ifndef DBG #define DBG 0 #endif -#ifndef HW_WRITE_LOCK -#define HW_WRITE_LOCK()HW_LOCK() -#endif - -#ifndef HW_WRITE_UNLOCK -#define HW_WRITE_UNLOCK() HW_UNLOCK() -#endif - -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif - -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - #ifndef HW_READ_CLIPLOOP #define HW_READ_CLIPLOOP() HW_CLIPLOOP() #endif Index: common/spantmp2.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/spantmp2.h,v retrieving revision 1.6 diff -u -r1.6 spantmp2.h --- common/spantmp2.h 15 Jan 2005 14:35:35 - 1.6 +++ common/spantmp2.h 3 Mar 2005 19:25:51 - @@ -34,27 +34,12 @@ */ #include "colormac.h" +#include "spantmp_common.h" #ifndef DBG #define DBG 0 #endif -#ifndef HW_WRITE_LOCK -#define HW_WRITE_LOCK()HW_LOCK() -#endif - -#ifndef HW_WRITE_UNLOCK -#define HW_WRITE_UNLOCK() HW_UNLOCK() -#endif - -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif - -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - #ifndef HW_READ_CLIPLOOP #define HW_READ_CLIPLOOP() HW_CLIPLOOP() #endif @@ -65,6 +50,14 @@ #if (SPANTMP_PIXEL_FMT == GL_RGB) && (SPANTMP_PIXEL_TYPE == GL_UNSIGNED_SHORT_5_6_5) +#ifndef GET_SRC_PTR +#define GET_SRC_PTR(_x, _y) (read_buf + (_x) * 2 + (_y) * pitch) +#endif + +#ifndef GET_DST_PTR +#define GET_DST_PTR(_x, _y) ( buf + (_x) * 2 + (_y) * pitch) +#endif + #define INIT_MONO_PIXEL(p, color) \ p = PACK_COLOR_565( color[0], color[1], color[2] ) @@ -92,6 +85,14 @@ #elif (SPANTMP_PIXEL_FMT == GL_BGRA) && (SPANTMP_PIXEL_TYPE == GL_UNSIGNED_INT_8_8_8_8_REV) +#ifndef GET_SRC_PTR +#define GET_SRC_PTR(_x, _y) (read_buf + (_x) * 4 + (_y) * pitch) +#endif + +#ifndef GET_DST_PTR +#define GET_DST_PTR(_x, _y) ( buf + (_x) * 4 + (_y) * pitch) +#endif + # define INIT_MONO_PIXEL(p, color) \ p = PACK_COLOR_(color[3], color[0], color[1], color[2]) Index: common/stenciltmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/stenciltmp.h,v retrieving revision 1.4 diff -u -r1.4 stenciltmp.h --- common/stenciltmp.h 14 Feb 2005 09:27:38 - 1.4 +++ common/stenciltmp.h 3 Mar 2005 19:25:51 - @@ -1,23 +1,11 @@ /* $XFree86: xc/lib/GL/mesa/src/drv/common/stenciltmp.h,v 1.3 2001/03/21 16:14:20 dawes Exp $ */ +#include "spantmp_common.h" + #ifndef DBG #define DBG 0 #endif -#ifndef HW_WRITE_LOCK -#define HW_WRITE_LOCK()HW_LOCK() -#endif -#ifndef HW_WRITE_UNLOCK -#define HW_WRITE_UNLOCK() HW_UNLOCK() -#endif - -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - static void TAG(WriteStencilSpan)( GLcontext *ctx,
Re: dri span patches...
Roland Scheidegger wrote: You're right. I really wanted to do a cvs diff to include new files, but I guess it's not possible, and hacking together the diffs manually is just asking for such errors :-). If you've done a 'cvs add' (or 'cvs rm') on the files, you can do 'cvs diff -Nud'. --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
Roland Scheidegger wrote: here's a patch which mainly does 3 things: - convert sis, mach64, and radeon to spantmp2. The sis and mach64 drivers got a slight change, previously you could not read back alpha values (always 0xff) and I don't think there was a good reason for that? IIRC, the mach64 doesn't support destination alpha planes. OpenGL requires that reads of absent alpha planes returns 1.0. I don't know if the SiS chip is the same. The 3 intel drivers and the s3v are not converted due to their support of a RGB555 format spantmp2 can't handle (actually, the i810 does not support the 555 format in its span functions, the driver elsewhere has support for it, this probably can't be right). tdfx and ffb are not converted since they support a 888 format. gamma is not converted since it performs some unusual always-round-up tricks when using RGB565 format. trident is not converted since it doesn't support span functions yet at all :-). - remove HW_CLIPLOOP, HW_ENDCLIPLOOP, CLIPPIXEL and CLIPSPAN macros out of the drivers whenever possible, since they are almost always identical. ffb uses very different macros, and some drivers do not use the driDrawable struct for HW_CLIPLOOP (which btw is interesting, I'm not quite sure why some drivers need to handle that differently, I didn't look too much into it but the difference is due to the handling of front and back buffers somehow), so they keep their own definitions hopefully. There are probably more macro definitions which could be factored out. - change CLIPSPAN to never return negative number of pixels (only change that once! not a dozen times, which is why I've done that macro definition removal in the first place...). Does this look sane? Driver source code should get quite a bit smaller overall. As long as you're looking at the span functions I should tell you that at some point in the future, work on the GL_EXT_framebuffer_object extension will likely require some changes in this area. I'll elaborate on that in another email when I have things better sorted out. I'm doing initial implementation with the xlib renderer now. But one thing that seems to be wrong in the r200 driver (for example) (and probably all the other DRI drivers) is the fact that the r200SetBuffer() function ignores its parameter. As it is now, the function ignores that parameter and always assumes that the span functions are going to be reading/writing to the context's current drawing buffer. The glXMakeContextCurrent and glXMakeCurrentReadSGI functions specify separate 'read' and 'draw' drawables. This allows you to do a glCopyPixels from one window to another, for example. This won't work if the colorBuffer parameter of r200SetBuffer() is ignored. In general, the span functions are relying too much on the GLcontext state, and not enough on the GLframebuffer state. The pointer to the start of the color/depth/stencil buffer, its width, height, pitch, etc. should be obtained via the GLframebuffer passed to r200SetBuffer. To do that, I guess GLframebuffer would need to have a pointer back to the corresponding __DRIDrawablePrivate, since we have no way to contain one in the other. Anyway, I wouldn't change anything right now. Let's wait until the GL_EXT_framebuffer_objects stuff has firmed up. -Brian --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
On Thu, Mar 03, 2005 at 06:49:26PM +0100, Roland Scheidegger wrote: > Ville Syrjälä wrote: > >On Thu, Mar 03, 2005 at 05:45:15PM +0100, Roland Scheidegger wrote: > > > >>Brian Paul wrote: > >> > >>>Roland Scheidegger wrote: > >>> > >>> > here's a patch which mainly does 3 things: - convert sis, mach64, > and radeon to spantmp2. The sis and mach64 drivers got a slight > change, previously you could not read back alpha values (always > 0xff) and I don't think there was a good reason for that? > >>> > >>> > >>>IIRC, the mach64 doesn't support destination alpha planes. OpenGL > >>>requires that reads of absent alpha planes returns 1.0. I don't know > >>>if the SiS chip is the same. > >> > >>Are you sure? At least the driver handles things like GL_DST_ALPHA blend > >>factors, wouldn't you get awfully bogus results if it wouldn't support > >>destination alpha planes in that case? > > > > > >Like I said before only the RGB components are blended. You can choose to > >write 0, 1, As, 1-As, Ad or 1-Ad to the destination alpha > >([EMAIL PROTECTED] register). Currently the driver seems to > >write 0. It would probably be a better idea to write 1 instead. > Sorry, but I just can't see that in the driver. And there's no > ALPHA_DST_SEL bit, at least not in the mach64 reg file I have... I was just looking at the specs :) They are named MACH64_ALPHA_DST_* in mach64_reg.h. The driver doesn't explicitly specify any value which means 0 gets written. I actually just stumbled on this issue a few days ago with the mach64 DirectFB driver. My plan for the DirectFB driver is simply to allow ZERO + ZERO/ONE/SRCALPHASAT blend functions for destinations with alpha. It's a rather serious limitation but I think it's better than incorrect rendering. The issue is even worse on older Rage chips since they can only write 0 to the destination alpha. But that is not an issue for the DRI driver since those chips aren't supported. > Regardless if it can actually blend alpha values or not, there would be > some half-way useful alpha values probably in the buffer. Either you get the correct results or the wrong results. Not sure if there are any really useful things you can do with incorrect values. -- Ville Syrjälä [EMAIL PROTECTED] http://www.sci.fi/~syrjala/ --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
Dieter Nützel wrote: Am Donnerstag, 3. März 2005 4:48 schrieb Roland Scheidegger: Roland Scheidegger wrote: here's a patch which mainly does 3 things: - convert sis, mach64, and radeon to spantmp2. The sis and mach64 drivers got a slight change, previously you could not read back alpha values (always 0xff) and I don't think there was a good reason for that? Roland, do you had a look on Felix's spantmp2.diff, too. http://marc.theaimsgroup.com/?l=dri-devel&m=110600974029291&w=2 I have missed that completely. Should be possible to work together with this patch just fine, though patch might disagree. BTW Can you please send me a private copy of dri-span3.diff, again. There are many ugly "=3D" characters in the archived version. You'll even get a newer version, Alan pointed out some subtle issues with the macro expansion (one of the reasons I don't particularly like macros...). Instead of fixing all GET_SRC/DST_PTR macros, I got rid of them too, since they were identical again in all drivers which use spantmp2 (except unichrome which uses different pitches for reading and drawing, it keeps its macros). Roland Index: common/depthtmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/depthtmp.h,v retrieving revision 1.7 diff -u -r1.7 depthtmp.h --- common/depthtmp.h 14 Feb 2005 09:27:38 - 1.7 +++ common/depthtmp.h 3 Mar 2005 19:25:50 - @@ -1,24 +1,19 @@ /* $XFree86: xc/lib/GL/mesa/src/drv/common/depthtmp.h,v 1.5 2001/03/21 16:14:20 dawes Exp $ */ +#include "spantmp_common.h" + #ifndef DBG #define DBG 0 #endif - #ifndef HAVE_HW_DEPTH_SPANS #define HAVE_HW_DEPTH_SPANS 0 #endif + #ifndef HAVE_HW_DEPTH_PIXELS #define HAVE_HW_DEPTH_PIXELS 0 #endif -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - static void TAG(WriteDepthSpan)( GLcontext *ctx, GLuint n, GLint x, GLint y, const GLdepth *depth, Index: common/spantmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/spantmp.h,v retrieving revision 1.5 diff -u -r1.5 spantmp.h --- common/spantmp.h14 Feb 2005 09:27:38 - 1.5 +++ common/spantmp.h3 Mar 2005 19:25:50 - @@ -27,26 +27,12 @@ *Gareth Hughes <[EMAIL PROTECTED]> */ +#include "spantmp_common.h" + #ifndef DBG #define DBG 0 #endif -#ifndef HW_WRITE_LOCK -#define HW_WRITE_LOCK()HW_LOCK() -#endif - -#ifndef HW_WRITE_UNLOCK -#define HW_WRITE_UNLOCK() HW_UNLOCK() -#endif - -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif - -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - #ifndef HW_READ_CLIPLOOP #define HW_READ_CLIPLOOP() HW_CLIPLOOP() #endif Index: common/spantmp2.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/spantmp2.h,v retrieving revision 1.6 diff -u -r1.6 spantmp2.h --- common/spantmp2.h 15 Jan 2005 14:35:35 - 1.6 +++ common/spantmp2.h 3 Mar 2005 19:25:51 - @@ -34,27 +34,12 @@ */ #include "colormac.h" +#include "spantmp_common.h" #ifndef DBG #define DBG 0 #endif -#ifndef HW_WRITE_LOCK -#define HW_WRITE_LOCK()HW_LOCK() -#endif - -#ifndef HW_WRITE_UNLOCK -#define HW_WRITE_UNLOCK() HW_UNLOCK() -#endif - -#ifndef HW_READ_LOCK -#define HW_READ_LOCK() HW_LOCK() -#endif - -#ifndef HW_READ_UNLOCK -#define HW_READ_UNLOCK() HW_UNLOCK() -#endif - #ifndef HW_READ_CLIPLOOP #define HW_READ_CLIPLOOP() HW_CLIPLOOP() #endif @@ -65,6 +50,14 @@ #if (SPANTMP_PIXEL_FMT == GL_RGB) && (SPANTMP_PIXEL_TYPE == GL_UNSIGNED_SHORT_5_6_5) +#ifndef GET_SRC_PTR +#define GET_SRC_PTR(_x, _y) (read_buf + (_x) * 2 + (_y) * pitch) +#endif + +#ifndef GET_DST_PTR +#define GET_DST_PTR(_x, _y) ( buf + (_x) * 2 + (_y) * pitch) +#endif + #define INIT_MONO_PIXEL(p, color) \ p = PACK_COLOR_565( color[0], color[1], color[2] ) @@ -92,6 +85,14 @@ #elif (SPANTMP_PIXEL_FMT == GL_BGRA) && (SPANTMP_PIXEL_TYPE == GL_UNSIGNED_INT_8_8_8_8_REV) +#ifndef GET_SRC_PTR +#define GET_SRC_PTR(_x, _y) (read_buf + (_x) * 4 + (_y) * pitch) +#endif + +#ifndef GET_DST_PTR +#define GET_DST_PTR(_x, _y) ( buf + (_x) * 4 + (_y) * pitch) +#endif + # define INIT_MONO_PIXEL(p, color) \ p = PACK_COLOR_(color[3], color[0], color[1], color[2]) Index: common/stenciltmp.h === RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/common/stenciltmp.h,v retrieving revision 1.4 diff -u -r1.4 stenciltmp.h --- common/stenciltmp.h 14 Feb 2005 09:27:38 - 1.4 +++ common/stenciltmp.h 3 Mar 2005 19:25:51 - @@ -1,23 +1,11 @@ /* $XFree86: xc/lib/GL/mesa/src/drv/common/s
Re: dri span patches...
Ville Syrjälä wrote: On Thu, Mar 03, 2005 at 05:45:15PM +0100, Roland Scheidegger wrote: Brian Paul wrote: Roland Scheidegger wrote: here's a patch which mainly does 3 things: - convert sis, mach64, and radeon to spantmp2. The sis and mach64 drivers got a slight change, previously you could not read back alpha values (always 0xff) and I don't think there was a good reason for that? IIRC, the mach64 doesn't support destination alpha planes. OpenGL requires that reads of absent alpha planes returns 1.0. I don't know if the SiS chip is the same. Are you sure? At least the driver handles things like GL_DST_ALPHA blend factors, wouldn't you get awfully bogus results if it wouldn't support destination alpha planes in that case? Like I said before only the RGB components are blended. You can choose to write 0, 1, As, 1-As, Ad or 1-Ad to the destination alpha ([EMAIL PROTECTED] register). Currently the driver seems to write 0. It would probably be a better idea to write 1 instead. Sorry, but I just can't see that in the driver. And there's no ALPHA_DST_SEL bit, at least not in the mach64 reg file I have... Regardless if it can actually blend alpha values or not, there would be some half-way useful alpha values probably in the buffer. Roland --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
On Thu, Mar 03, 2005 at 08:10:33PM +0100, Roland Scheidegger wrote: > Ville Syrjälä wrote: > >>>Like I said before only the RGB components are blended. You can > >>>choose to write 0, 1, As, 1-As, Ad or 1-Ad to the destination > >>>alpha ([EMAIL PROTECTED] register). Currently the > >>>driver seems to write 0. It would probably be a better idea to > >>>write 1 instead. > >> > >>Sorry, but I just can't see that in the driver. And there's no > >>ALPHA_DST_SEL bit, at least not in the mach64 reg file I have... > > > > > >I was just looking at the specs :) They are named MACH64_ALPHA_DST_* > >in mach64_reg.h. The driver doesn't explicitly specify any value > >which means 0 gets written. > > > >I actually just stumbled on this issue a few days ago with the mach64 > > DirectFB driver. My plan for the DirectFB driver is simply to allow > >ZERO + ZERO/ONE/SRCALPHASAT blend functions for destinations with > >alpha. It's a rather serious limitation but I think it's better than > >incorrect rendering. The issue is even worse on older Rage chips > >since they can only write 0 to the destination alpha. But that is not > >an issue for the DRI driver since those chips aren't supported. > I think now I understand. It has alpha channel and all, but it simply > won't perform the blending equation on the alpha channel, instead simply > writing zero, one, source alpha, 1 - minus source alpha, dst alpha, or 1 > - dst alpha. Exactly. > Actually the driver does not write 0, it writes the source > alpha value (MACH64_ALPHA_DST_SRCALPHA) as far as I can tell. Ok I missed that with my grepping. I was probably looking for ALPHA_DST_SEL myself too :) > Actually, this design means it would have some very limited support for > blend_func_separate :-). I had the same though :) > Looks like a stupid design limitation to me (what would it cost to > implement that additional blend adder to the 3 you need anyway?), but ah > well. Maybe this wasn't required by DirectX 1.0 ;-). Mach64 chips have quite a few stupid alpha limitations. Most likely the nastiest one being that texture alpha can't be modulated. And then there's the whole fog vs. alpha thing. > In practice though, this might just work quite often, the alpha-blended > alpha values are probably not required a lot? Not with X. With DirectFB they are needed every time you render to an ARGB window and then expect to display the window alpha blended on screen. > >>Regardless if it can actually blend alpha values or not, there > >>would be some half-way useful alpha values probably in the buffer. > > > > > >Either you get the correct results or the wrong results. Not sure if > > there are any really useful things you can do with incorrect values. > I meant that you might just get the correct alpha values sometimes > (depending on the blend func that should be true I guess). Right. Though there are only a few combinations that give correct results. -- Ville Syrjälä [EMAIL PROTECTED] http://www.sci.fi/~syrjala/ --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
Brian Paul wrote: Roland Scheidegger wrote: here's a patch which mainly does 3 things: - convert sis, mach64, and radeon to spantmp2. The sis and mach64 drivers got a slight change, previously you could not read back alpha values (always 0xff) and I don't think there was a good reason for that? IIRC, the mach64 doesn't support destination alpha planes. OpenGL requires that reads of absent alpha planes returns 1.0. I don't know if the SiS chip is the same. Are you sure? At least the driver handles things like GL_DST_ALPHA blend factors, wouldn't you get awfully bogus results if it wouldn't support destination alpha planes in that case? It does not export BGRA visuals however for some reason. Note that r128 driver (which was converted to use spantmp2 by Ian and thus got that same change in behaviour reading back RGBA pixels already) does not export visuals with a alpha component neither (just BGR 888, if I read that right). SIS OTOH exports BGRA visuals. Does this look sane? Driver source code should get quite a bit smaller overall. As long as you're looking at the span functions I should tell you that at some point in the future, work on the GL_EXT_framebuffer_object extension will likely require some changes in this area. I'll elaborate on that in another email when I have things better sorted out. I'm doing initial implementation with the xlib renderer now. But one thing that seems to be wrong in the r200 driver (for example) (and probably all the other DRI drivers) is the fact that the r200SetBuffer() function ignores its parameter. As it is now, the function ignores that parameter and always assumes that the span functions are going to be reading/writing to the context's current drawing buffer. The glXMakeContextCurrent and glXMakeCurrentReadSGI functions specify separate 'read' and 'draw' drawables. This allows you to do a glCopyPixels from one window to another, for example. This won't work if the colorBuffer parameter of r200SetBuffer() is ignored. In general, the span functions are relying too much on the GLcontext state, and not enough on the GLframebuffer state. The pointer to the start of the color/depth/stencil buffer, its width, height, pitch, etc. should be obtained via the GLframebuffer passed to r200SetBuffer. To do that, I guess GLframebuffer would need to have a pointer back to the corresponding __DRIDrawablePrivate, since we have no way to contain one in the other. Anyway, I wouldn't change anything right now. Let's wait until the GL_EXT_framebuffer_objects stuff has firmed up. I do not think these changes should interfere with that however, since the code generated should not change at all, it's just unuglifying by moving macros around (except for those drivers which got converted to use spantmp2, but you'll need to make it work with that anyway if somehow that template won't be able to deal with these necessary changes). Roland --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: dri span patches...
Am Donnerstag, 3. März 2005 20:36 schrieb Roland Scheidegger: > Dieter Nützel wrote: > > Am Donnerstag, 3. März 2005 4:48 schrieb Roland Scheidegger: > >> Roland Scheidegger wrote: here's a patch which mainly does 3 > >> things: - convert sis, mach64, and radeon to spantmp2. The sis and > >> mach64 drivers got a slight change, previously you could not read > >> back alpha values (always 0xff) and I don't think there was a good > >> reason for that? > > > > Roland, > > > > do you had a look on Felix's spantmp2.diff, too. > > > > http://marc.theaimsgroup.com/?l=dri-devel&m=110600974029291&w=2 > > I have missed that completely. Should be possible to work together with > this patch just fine, though patch might disagree. > > > BTW Can you please send me a private copy of dri-span3.diff, again. > > There are many ugly "=3D" characters in the archived version. > > You'll even get a newer version, Alan pointed out some subtle issues > with the macro expansion (one of the reasons I don't particularly like > macros...). Instead of fixing all GET_SRC/DST_PTR macros, I got rid of > them too, since they were identical again in all drivers which use > spantmp2 (except unichrome which uses different pitches for reading and > drawing, it keeps its macros). Sorry, you lost spantmp_common.h in dri_span4.diff. /opt/Mesa> grep -r spantmp_common.h * src/mesa/drivers/dri/common/stenciltmp.h:#include "spantmp_common.h" src/mesa/drivers/dri/common/depthtmp.h:#include "spantmp_common.h" src/mesa/drivers/dri/common/spantmp.h:#include "spantmp_common.h" src/mesa/drivers/dri/common/spantmp2.h:#include "spantmp_common.h" src/mesa/drivers/dri/common/spantmp2.h.orig:#include "spantmp_common.h" /opt/Mesa> find -name spantmp_common.h -Dieter --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_ide95&alloc_id396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
