Bug 47375

Summary: Blender crash on startup after upgrade to mesa 8.0.1
Product: Mesa Reporter: lukasas
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: hartmut
Version: 8.0   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:

Description lukasas 2012-03-15 14:02:26 UTC
Description: After upgrade to mesa 8.0.1 Blender don't start with this error:

blender: swrast/s_span.c:1327: _swrast_write_rgba_span: Assertion `colorType == 0x1401 || colorType == 0x1406' failed.

After downgrading libgl and ati-dri to 7.11.2-1 versions it works ok.

*direct rendering: Yes
OpenGL renderer string: Mesa DRI R200 (RV280 5961) x86/MMX/SSE2 TCL DRI2


Additional info:
* package version(s) 7.11.2-1 vs 8.0.1-2

Steps to reproduce: try to start blender on mesa 8.0.1 (and ati drivers)

Archlinux i686, up-to-date
Comment 1 hartmut 2012-06-10 12:59:49 UTC
same here.  

- Arch Linux
- Mesa 8.0.3
- Mesa DRI R200 (RV250 4C66) x86/MMX/SSE2 TCL DRI2


Stack trace:

Program received signal SIGABRT, Aborted.
0xb7fdd424 in __kernel_vsyscall ()
(gdb) back
#0  0xb7fdd424 in __kernel_vsyscall ()
#1  0xb5f6c24f in raise () from /lib/libc.so.6
#2  0xb5f6db53 in abort () from /lib/libc.so.6
#3  0xb5f65087 in __assert_fail_base () from /lib/libc.so.6
#4  0xb5f65137 in __assert_fail () from /lib/libc.so.6
#5  0xad60cd5f in _swrast_write_rgba_span () from /usr/lib/libdricore.so
#6  0xad600347 in ?? () from /usr/lib/libdricore.so
#7  0xad601162 in _swrast_DrawPixels () from /usr/lib/libdricore.so
#8  0xad64ca7f in _mesa_meta_DrawPixels () from /usr/lib/libdricore.so
#9  0xad4e4152 in ?? () from /usr/lib/libdricore.so
#10 0x087dad04 in icon_draw_rect.isra.1 ()
#11 0x087dafe5 in icon_draw_size.isra.3 ()
#12 0x087dd155 in UI_icon_draw_aspect ()
#13 0x087fe78c in widget_draw_icon ()
#14 0x087fefe3 in widget_draw_text_icon ()
#15 0x088025ee in ui_draw_but ()
#16 0x087c5a77 in uiDrawBlock ()
#17 0x0887c70e in ED_region_header ()
#18 0x08667723 in info_header_area_draw ()
#19 0x0887a518 in ED_region_do_draw ()
#20 0x0861786b in wm_method_draw_overlap_all ()
#21 0x08618a84 in wm_draw_update ()
#22 0x08617288 in WM_main ()
#23 0x085e658a in main ()
Comment 2 Barto 2012-07-13 10:10:37 UTC
same bug with a radeon 9000 card ( M9 ) :

OpenGL renderer string: Mesa DRI R200 (RV250 4C66) x86/MMX/SSE2 TCL DRI2

blender crashs with this message with mesa 8.0.4 :

blender: swrast/s_span.c:1327: _swrast_write_rgba_span: Assertion `colorType == 0x1401 || colorType == 0x1406' failed.

the bug doesn't occur with an old mesa release ( Mesa-7.11.2 )
Comment 3 Brian Paul 2012-07-19 19:31:37 UTC
(In reply to comment #2)
> same bug with a radeon 9000 card ( M9 ) :
> 
> OpenGL renderer string: Mesa DRI R200 (RV250 4C66) x86/MMX/SSE2 TCL DRI2
> 
> blender crashs with this message with mesa 8.0.4 :
> 
> blender: swrast/s_span.c:1327: _swrast_write_rgba_span: Assertion `colorType ==
> 0x1401 || colorType == 0x1406' failed.
> 
> the bug doesn't occur with an old mesa release ( Mesa-7.11.2 )

Can you get the value of colorType in gdb at the failed assertion?
Comment 4 Barto 2012-07-24 10:46:37 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > same bug with a radeon 9000 card ( M9 ) :
> > 
> > OpenGL renderer string: Mesa DRI R200 (RV250 4C66) x86/MMX/SSE2 TCL DRI2
> > 
> > blender crashs with this message with mesa 8.0.4 :
> > 
> > blender: swrast/s_span.c:1327: _swrast_write_rgba_span: Assertion `colorType ==
> > 0x1401 || colorType == 0x1406' failed.
> > 
> > the bug doesn't occur with an old mesa release ( Mesa-7.11.2 )
> 
> Can you get the value of colorType in gdb at the failed assertion?


Ok but I need to compile blender with debugging symbols in order to use breakpoints,

when I will do this I will tell the value of "colorType"
Comment 5 Brian Paul 2012-07-24 13:36:52 UTC
You don't need to recompile blender for debugging, only Mesa.
Comment 6 Barto 2012-07-24 15:46:01 UTC
(In reply to comment #5)
> You don't need to recompile blender for debugging, only Mesa.

I'm a newbie in gdb, can you tell me how I must proceed in order to debug mesa and blender ?

I have compiled mesa with the --enable-debug option, then I load manually the mesa lib with these commands :

export LD_LIBRARY_PATH=/home/barto/compilation/mesa/Mesa-8.0.4/lib/
export LIBGL_DRIVERS_PATH=/home/barto/compilation/mesa/Mesa-8.0.4/lib/

then I type:

gdb blender

and 

break s_span.c:1327

then:

run

but gdb can't reach the breakpoint, I think I don't use the correct way to debugging mesa, I need some help,

thanks
Comment 7 Brian Paul 2012-07-24 15:53:41 UTC
That looks about right.  How exactly did you configure/build Mesa?  You make need something like "CFLAGS=-g ./configure ..." to make sure you get -g and not -O2 when building Mesa.

Otherwise, you could try putting a printf near s_span.c:1327 to get the value.
Comment 8 Barto 2012-07-24 16:01:22 UTC
(In reply to comment #7)
> That looks about right.  How exactly did you configure/build Mesa?  You make
> need something like "CFLAGS=-g ./configure ..." to make sure you get -g and not
> -O2 when building Mesa.
> 
> Otherwise, you could try putting a printf near s_span.c:1327 to get the value.



finally I succeed to get the value of "colorType",

I set a breakpoint like this in gdb :

break s_span.c:1326

after that gdb manages to reach the breakpoint :

Breakpoint 1, _swrast_write_rgba_span (ctx=ctx@entry=0xbc60648,
span=span@entry=0xbfffe750)
    at swrast/s_span.c:1326
1326                assert(colorType == GL_UNSIGNED_BYTE ||


then I can see the value of colorType:


(gdb) print colorType
$1 = 0

colorType is set to zero here
Comment 9 Barto 2012-07-24 18:29:47 UTC
I found a fix for this bug, but it's a dirty fix :

the solution is to comment some assert() functions in 3 files :

1) in swrast/s_span.c

comment line 1078 :

assert(datatype == GL_FLOAT);

comment line 1326 :

assert(colorType == GL_UNSIGNED_BYTE ||
                   colorType == GL_FLOAT );
                   

2) in main/image.c

comment line 1760 : 

ASSERT(dstType == GL_UNSIGNED_SHORT);


3) in swrast/s_blend.c

comment line 217: 

ASSERT(chanType == GL_FLOAT);

after these modifications blender can be launched without errors, but I hope that someone ( Brian Paul ? ) will find a better fix for this bug who occurs with old radeon cards ( r200 driver, rv250 card for example )
Comment 10 Barto 2012-07-24 18:51:35 UTC
in fact my fix is not good, blender can be launched but everything is slow ( clicks on the interface, significant latency in the GUI ),

so we need to find a true fix for this bug

(In reply to comment #9)
> I found a fix for this bug, but it's a dirty fix :
> 
> the solution is to comment some assert() functions in 3 files :
> 
> 1) in swrast/s_span.c
> 
> comment line 1078 :
> 
> assert(datatype == GL_FLOAT);
> 
> comment line 1326 :
> 
> assert(colorType == GL_UNSIGNED_BYTE ||
>                    colorType == GL_FLOAT );
> 
> 
> 2) in main/image.c
> 
> comment line 1760 : 
> 
> ASSERT(dstType == GL_UNSIGNED_SHORT);
> 
> 
> 3) in swrast/s_blend.c
> 
> comment line 217: 
> 
> ASSERT(chanType == GL_FLOAT);
> 
> after these modifications blender can be launched without errors, but I hope
> that someone ( Brian Paul ? ) will find a better fix for this bug who occurs
> with old radeon cards ( r200 driver, rv250 card for example )
Comment 11 Brian Paul 2012-07-24 19:52:06 UTC
Adding new assertions shouldn't effect things like that since you're not actually changing any values.  There must be something else happening.

As for the performance, perhaps it's the _swrast_DrawPixels() calls which are software fallbacks.  Maybe you can debug further and see why the fallback is getting called in the meta function.
Comment 12 Barto 2012-07-24 21:36:18 UTC
(In reply to comment #11)
> Adding new assertions shouldn't effect things like that since you're not
> actually changing any values.  There must be something else happening.
> 

I didn't add new assertions, in fact my "dirty" fix was to delete ( or comment ) some existing assertions , like this:

assert(datatype == GL_FLOAT);

but this causes serious lags in blender, which is not normal, 

when I run blender with the software mode ( export LIBGL_ALWAYS_SOFTWARE=1 ) and with mesa 8.0.4 there is no lags, all is ok about the speed, the reactivity of the GUI,

but when I apply my "dirty fix" it's very very slow in blender, this is not normal because the speed shouldn't be lower than in the "software mode" in blender

in fact the "good" fix is to use an old mesa release like the 7.11.2 version, the changes in source code made since the 8.0.x release bring this bug with older ati radeon card,

I have to do further investigations, as you said in your message, 

I notice some error messages in blender with my "dirty fix", but despite these errors blender doesn't crash ( but it's slow ) :

Mesa: User error: GL_INVALID_ENUM in glGetIntegerv(pname=GL_MAX_TEXTURE_IMAGE_UNITS)
Mesa 8.0.4 implementation error: unexpected type in get_row()
Comment 13 Brian Paul 2012-07-25 13:53:09 UTC
Sorry, I misread your update too quickly and thought that you had added some assertions.  I think we still need to get to the root cause of those before trying to diagnose performance problems.

The "unexpected type in get_row()" error is probably related to the failed assertion and will hopefully be fixed when the first problem is fixed.

Unfortunately, I can't reproduce this here.  I have blender 2.63a and tested it with swrast.  I forced Mesa to take the _mesa_meta_DrawPixels() path but it's not calling _swrast_DrawPixels().  The 'fallback' variable is false while it seems to be true for you.  Maybe you could find out why 'fallback' is getting set.

I also tried forcing fallback=1 so that _swrast_DrawPixels() gets called but I didn't see any failed assertions.  I'm using the Mesa 8.0.x branch.

I don't have a r200 gpu to try that driver.
Comment 14 Barto 2012-07-25 18:56:11 UTC
(In reply to comment #13)
 
> I also tried forcing fallback=1 so that _swrast_DrawPixels() gets called but I
> didn't see any failed assertions.  I'm using the Mesa 8.0.x branch.
> 
> I don't have a r200 gpu to try that driver.

Brian, can you tell me what are the differences in swrast component between the 7.11.2-1 mesa release version and the 8.0.x release ?

we know that swrast runs without problems in 7.11.2-1 mesa version with blender and an old ati radeon card ( rv250, rv280 cards ),

but something in the 8.0.x mesa release has changed in the "swrast component", if we can locate these new lines in the source code then we will find probably the culprit,

maybe a new onpenGL instruction who is not supported by old card like rv250, rv280, don't forget that radeon 9000 card ( rv250 ) only supports opengl 1.3 version,

I need some tips in order to debug this problem ( I'm a newbie in debugging opengl/multimedia software )

thanks
Comment 15 Brian Paul 2012-07-25 22:22:58 UTC
(In reply to comment #14)

> Brian, can you tell me what are the differences in swrast component between the
> 7.11.2-1 mesa release version and the 8.0.x release ?

There's a ton of changes in swrast since 7.11, unfortunately.

 
> we know that swrast runs without problems in 7.11.2-1 mesa version with blender
> and an old ati radeon card ( rv250, rv280 cards ),
> 
> but something in the 8.0.x mesa release has changed in the "swrast component",
> if we can locate these new lines in the source code then we will find probably
> the culprit,
> 
> maybe a new onpenGL instruction who is not supported by old card like rv250,
> rv280, don't forget that radeon 9000 card ( rv250 ) only supports opengl 1.3
> version,
> 
> I need some tips in order to debug this problem ( I'm a newbie in debugging
> opengl/multimedia software )

Let's first find out why the 'fallback' variable in _mesa_meta_DrawPixels() is getting set.  Basically, use gdb to set a breakpoint in _mesa_meta_DrawPixels() then go step by step to see where 'fallback = GL_TRUE' is getting hit.

To set a breakpoint: "break _mesa_meta_DrawPixels()"
To step one instruction: "s" or "step"

When you find the point where fallback = GL_TRUE is set, note the current line and try to print some of the variables in the conditionals.
Comment 16 Barto 2012-07-26 12:25:36 UTC
(In reply to comment #15)

> Let's first find out why the 'fallback' variable in _mesa_meta_DrawPixels() is
> getting set.  Basically, use gdb to set a breakpoint in _mesa_meta_DrawPixels()
> then go step by step to see where 'fallback = GL_TRUE' is getting hit.
> 
> To set a breakpoint: "break _mesa_meta_DrawPixels()"
> To step one instruction: "s" or "step"
> 
> When you find the point where fallback = GL_TRUE is set, note the current line
> and try to print some of the variables in the conditionals.

first I have done the debug with the original 8.0.4 mesa libs ( no modifications in the source code ), I have set a breakpoint like this :

break _mesa_meta_DrawPixels()

but gdb never reaches the _mesa_meta_DrawPixels() function, this function is never called, and Blender crashes at startup with this error :

blender: swrast/s_span.c:1327: _swrast_write_rgba_span: Assertion `colorType ==
0x1401 || colorType == 0x1406' failed.

when I check the value of colorType it's set to zero


then I decided to test with a modified 8.0.4 libs ( by applying my "dirty fix", I delete somme assertions functions in s_span.c, image.c and s_blend.c files ), and I set again the same breakpoint:

break _mesa_meta_DrawPixels()

but gdb again never reaches the _mesa_meta_DrawPixels() function, Blender doesn't crash, but it's very slow in the GUI,

this slowness is not normal because in "software mode" ( LIBGL_ALWAYS_SOFTWARE=1 ) for opengl blender is still fast with 8.0.4 original mesa libs, so I think it's my "dirty fix" who causes this slowness, deleting the assertions functions was not a good idea,

in conclusion I think we must find why the colorType variable is set to zero when I use the 8.0.4 original mesa libs, why the assertion fails in s_span.c at line 1327 ?
Comment 17 Brian Paul 2012-07-26 13:50:00 UTC
(In reply to comment #16)
> (In reply to comment #15)
> 
> > Let's first find out why the 'fallback' variable in _mesa_meta_DrawPixels() is
> > getting set.  Basically, use gdb to set a breakpoint in _mesa_meta_DrawPixels()
> > then go step by step to see where 'fallback = GL_TRUE' is getting hit.
> > 
> > To set a breakpoint: "break _mesa_meta_DrawPixels()"
> > To step one instruction: "s" or "step"
> > 
> > When you find the point where fallback = GL_TRUE is set, note the current line
> > and try to print some of the variables in the conditionals.
> 
> first I have done the debug with the original 8.0.4 mesa libs ( no
> modifications in the source code ), I have set a breakpoint like this :
> 
> break _mesa_meta_DrawPixels()
> 
> but gdb never reaches the _mesa_meta_DrawPixels() function, this function is
> never called, and Blender crashes at startup with this error :
> 
> blender: swrast/s_span.c:1327: _swrast_write_rgba_span: Assertion `colorType ==
> 0x1401 || colorType == 0x1406' failed.
> 
> when I check the value of colorType it's set to zero

According to hartmut's comment #2 earlier, the stack trace says _mesa_meta_DrawPixels() was called.  Can you verify that?  That is, after the assertion fails, type "back" or "where".


> then I decided to test with a modified 8.0.4 libs ( by applying my "dirty fix",
> I delete somme assertions functions in s_span.c, image.c and s_blend.c files ),
> and I set again the same breakpoint:
> 
> break _mesa_meta_DrawPixels()
> 
> but gdb again never reaches the _mesa_meta_DrawPixels() function, Blender
> doesn't crash, but it's very slow in the GUI,
> 
> this slowness is not normal because in "software mode" (
> LIBGL_ALWAYS_SOFTWARE=1 ) for opengl blender is still fast with 8.0.4 original
> mesa libs, so I think it's my "dirty fix" who causes this slowness, deleting
> the assertions functions was not a good idea,
> 
> in conclusion I think we must find why the colorType variable is set to zero
> when I use the 8.0.4 original mesa libs, why the assertion fails in s_span.c at
> line 1327 ?

Let's try to get a stack trace to see the path to the failed assertion.
Comment 18 Barto 2012-07-26 14:10:17 UTC
(In reply to comment #17)

> According to hartmut's comment #2 earlier, the stack trace says
> _mesa_meta_DrawPixels() was called.  Can you verify that?  That is, after the
> assertion fails, type "back" or "where".

so I type "back" when blender crashes, here is the result :

(gdb) back
#0  0xb7fdd424 in __kernel_vsyscall ()
#1  0xb5ecf5cf in raise () from /lib/libc.so.6
#2  0xb5ed0eb3 in abort () from /lib/libc.so.6
#3  0xb5ec8427 in __assert_fail_base () from /lib/libc.so.6
#4  0xb5ec84d7 in __assert_fail () from /lib/libc.so.6
#5  0xb1d6472d in _swrast_write_rgba_span (ctx=ctx@entry=0xaa5d5e0, span=span@entry=0xbfffe5f0)
    at swrast/s_span.c:1326
#6  0xb1e6db67 in draw_rgba_pixels (ctx=ctx@entry=0xaa5d5e0, x=x@entry=45, y=y@entry=445, 
    width=width@entry=16, height=height@entry=16, format=format@entry=6408, type=type@entry=5121, 
    unpack=unpack@entry=0xaa6c074, pixels=pixels@entry=0xb3b2720) at swrast/s_drawpix.c:481
#7  0xb1e6eae2 in _swrast_DrawPixels (ctx=ctx@entry=0xaa5d5e0, x=x@entry=45, y=y@entry=445, 
    width=width@entry=16, height=height@entry=16, format=format@entry=6408, type=type@entry=5121, 
    unpack=unpack@entry=0xaa6c074, pixels=pixels@entry=0xb3b2720) at swrast/s_drawpix.c:694
#8  0xb1da41f0 in _mesa_meta_DrawPixels (ctx=0xaa5d5e0, x=45, y=445, width=16, height=16, format=6408, 
    type=5121, unpack=0xaa6c074, pixels=0xb3b2720) at drivers/common/meta.c:2314
#9  0xb1e1dd6e in _mesa_DrawPixels (width=16, height=16, format=6408, type=5121, pixels=0xb3b2720)
    at main/drawpix.c:131
#10 0x088611c4 in icon_draw_rect.isra.1 ()
#11 0x088614a5 in icon_draw_size.isra.3 ()
#12 0x08863675 in UI_icon_draw_aspect ()
#13 0x0888581c in widget_draw_icon ()
#14 0x08886073 in widget_draw_text_icon ()
#15 0x0888962e in ui_draw_but ()
#16 0x0884b71e in uiDrawBlock ()
#17 0x0890811e in ED_region_header ()
#18 0x086dd5c3 in info_header_area_draw ()
#19 0x08905f28 in ED_region_do_draw ()
#20 0x0868cd1b in wm_method_draw_overlap_all ()
#21 0x0868df34 in wm_draw_update ()
#22 0x0868c738 in WM_main ()
Comment 19 Barto 2012-07-26 15:07:37 UTC
I use another way and now it works, gdb can reach _mesa_meta_DrawPixels,

for that I set 4 breakpoints in order to reach the line where "fallback = GL_TRUE" in meta.c file :

break meta.c:2258
break meta.c:2296
break meta.c:2306
break meta.c:2310

and gdb stops to the line 2258 :

Breakpoint 1, _mesa_meta_DrawPixels (ctx=0xaa5d5e0, x=12, y=446, width=16, height=16, format=6408, 
    type=5121, unpack=0xaa6c074, pixels=0xb3d1778) at drivers/common/meta.c:2258
2258          fallback = GL_TRUE;

the conditionnal :

 if (ctx->_ImageTransferState ||
       ctx->Fog.Enabled) {
      fallback = GL_TRUE;
   }

gdb found no value for ctx-> ImageTransferState :

(gdb) print ctx-> ImageTransferState
There is no member named ImageTransferState.

but for ctx->Fog.Enabled gdb found :

(gdb) print ctx->Fog.Enabled
$3 = 0 '\000'
Comment 20 Brian Paul 2012-07-26 15:09:14 UTC
(In reply to comment #19)
> I use another way and now it works, gdb can reach _mesa_meta_DrawPixels,
> 
> for that I set 4 breakpoints in order to reach the line where "fallback =
> GL_TRUE" in meta.c file :
> 
> break meta.c:2258
> break meta.c:2296
> break meta.c:2306
> break meta.c:2310
> 
> and gdb stops to the line 2258 :
> 
> Breakpoint 1, _mesa_meta_DrawPixels (ctx=0xaa5d5e0, x=12, y=446, width=16,
> height=16, format=6408, 
>     type=5121, unpack=0xaa6c074, pixels=0xb3d1778) at
> drivers/common/meta.c:2258
> 2258          fallback = GL_TRUE;
> 
> the conditionnal :
> 
>  if (ctx->_ImageTransferState ||
>        ctx->Fog.Enabled) {
>       fallback = GL_TRUE;
>    }
> 
> gdb found no value for ctx-> ImageTransferState :
> 
> (gdb) print ctx-> ImageTransferState
> There is no member named ImageTransferState.

There's a leading underscore on that variable.  Did you miss that?
Comment 21 Barto 2012-07-26 15:12:44 UTC
(In reply to comment #20)
> (In reply to comment #19)
> 
> There's a leading underscore on that variable.  Did you miss that?

I miss the underscore,

after correctin here is the result :


(gdb) print ctx->_ImageTransferState
$4 = 0
Comment 22 Brian Paul 2012-07-26 15:24:00 UTC
(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #19)
> > 
> > There's a leading underscore on that variable.  Did you miss that?
> 
> I miss the underscore,
> 
> after correctin here is the result :
> 
> 
> (gdb) print ctx->_ImageTransferState
> $4 = 0

Hmm, if _ImageTransferState==0 and ctx->Fog.Enabled==0 the condition is false and we shouldn't be setting fallback=true.

How did you build Mesa?  See comment 7.  If you don't build with CFLAGS=-g and --enable-debug you may be seeing bogus values.
Comment 23 Barto 2012-07-26 15:46:16 UTC
(In reply to comment #22)
> 
> Hmm, if _ImageTransferState==0 and ctx->Fog.Enabled==0 the condition is false
> and we shouldn't be setting fallback=true.
> 
> How did you build Mesa?  See comment 7.  If you don't build with CFLAGS=-g and
> --enable-debug you may be seeing bogus values.

I used this commande line :

 ./configure --enable-debug  --with-dri-drivers=r200,radeon --with-gallium-drivers=  

I will try with "CFLAGS=-g", like this :


./configure CFLAGS=-g --enable-debug  --with-dri-drivers=r200,radeon --with-gallium-drivers=  

then I will post again here the results for gdb
Comment 24 Barto 2012-07-26 16:11:31 UTC
I have finished the compilation of the debug mesa lib ( with CFLAG=-g )

now the result when gdb stops in breakpoint in meta.c, line 2258 :

(gdb) print ctx->_ImageTransferState
$4 = 1

(gdb) print ctx->Fog.Enabled
$5 = 0 '\000'

now ctx->_ImageTransferState is set to "1"

the conditionnal was :

 if (ctx->_ImageTransferState ||
        ctx->Fog.Enabled) {
       fallback = GL_TRUE;
    }

then I set another breakpoint:

break s_span.c:1326

the variable colorType is still set to "0" just before the crash :

(gdb) print colorType
$3 = 0

that's why this assertion fails in line 1326 :

GLchan rgbaSave[MAX_WIDTH][4];
struct swrast_renderbuffer *srb = swrast_renderbuffer(rb);
GLenum colorType = srb->ColorType;

assert(colorType == GL_UNSIGNED_BYTE || colorType == GL_FLOAT);
Comment 25 Roland Scheidegger 2012-07-26 16:30:37 UTC
A very quick look at blender source shows it can indeed use PixelTransfer ops, in particular scale/bias. Those should be possible to accelerate (both with and without fragment shader support without too much effort) if someone would bother enough, though of course this does not explain the crash with the fallback.
Comment 26 Brian Paul 2012-07-26 16:45:30 UTC
(In reply to comment #24)

> (gdb) print colorType
> $3 = 0
> 
> that's why this assertion fails in line 1326 :
> 
> GLchan rgbaSave[MAX_WIDTH][4];
> struct swrast_renderbuffer *srb = swrast_renderbuffer(rb);
> GLenum colorType = srb->ColorType;
> 
> assert(colorType == GL_UNSIGNED_BYTE || colorType == GL_FLOAT);

Can you do "print *srb" at this point?
Comment 27 Barto 2012-07-26 16:56:50 UTC
(In reply to comment #26)
> (In reply to comment #24)
> 
> > (gdb) print colorType
> > $3 = 0
> > 
> > that's why this assertion fails in line 1326 :
> > 
> > GLchan rgbaSave[MAX_WIDTH][4];
> > struct swrast_renderbuffer *srb = swrast_renderbuffer(rb);
> > GLenum colorType = srb->ColorType;
> > 
> > assert(colorType == GL_UNSIGNED_BYTE || colorType == GL_FLOAT);
> 
> Can you do "print *srb" at this point?

here is the result :

(gdb) print *srb
$1 = {Base = {Mutex = {__data = {__lock = 0, __count = 0, __owner = 0, __kind = 0, __nusers = 0, {
          __spins = 0, __list = {__next = 0x0}}}, __size = '\000' <repeats 23 times>, __align = 0}, 
    ClassID = 3735928559, Name = 0, RefCount = 2, Width = 676, Height = 467, Purgeable = 0 '\000', 
    AttachedAnytime = 0 '\000', NumSamples = 0 '\000', InternalFormat = 6407, _BaseFormat = 6407, 
    Format = MESA_FORMAT_XRGB8888, Delete = 0xb1bc282c <radeon_delete_renderbuffer>, 
    AllocStorage = 0xb1bc3e13 <radeon_alloc_window_storage>}, Buffer = 0x0, 
  Map = 0xb0b13600 <Address 0xb0b13600 out of bounds>, RowStride = -2816, ColorType = 0}
Comment 28 Brian Paul 2012-07-26 18:45:48 UTC
Can you try making this change to radeon_span.c:

diff --git a/src/mesa/drivers/dri/radeon/radeon_span.c b/src/mesa/drivers/dri/ra
index 1f2ba49..fd91f88 100644
--- a/src/mesa/drivers/dri/radeon/radeon_span.c
+++ b/src/mesa/drivers/dri/radeon/radeon_span.c
@@ -66,6 +66,7 @@ radeon_renderbuffer_map(struct gl_context *ctx, struct gl_rend
 
        rrb->base.Map = map;
        rrb->base.RowStride = stride;
+       rrb->Base.ColorType = GL_UNSIGNED_BYTE;
 }
 
 static void
Comment 29 Barto 2012-07-26 19:19:49 UTC
(In reply to comment #28)
> Can you try making this change to radeon_span.c:
> 
> diff --git a/src/mesa/drivers/dri/radeon/radeon_span.c
> b/src/mesa/drivers/dri/ra
> index 1f2ba49..fd91f88 100644
> --- a/src/mesa/drivers/dri/radeon/radeon_span.c
> +++ b/src/mesa/drivers/dri/radeon/radeon_span.c
> @@ -66,6 +66,7 @@ radeon_renderbuffer_map(struct gl_context *ctx, struct
> gl_rend
> 
>         rrb->base.Map = map;
>         rrb->base.RowStride = stride;
> +       rrb->Base.ColorType = GL_UNSIGNED_BYTE;
>  }
> 
>  static void

so I add this line in radeon_span.c, after the line 68 :

rrb->base.ColorType = GL_UNSIGNED_BYTE;

I compiled mesa lib, Blender runs without crash, but it still very slow in the GUI ( too much lag when I click in the gui, menus ), it's not normal because with 7.11 mesa lib Blender was much more fast,

I notice also some errors message in the console :

Mesa: User error: GL_INVALID_ENUM in glGetIntegerv(pname=GL_MAX_TEXTURE_IMAGE_UNITS)
Error: Not freed memory blocks: 2
IDProperty group len: 108 0xb91d808
uiAfterFunc len: 616 0xb9e5f88
Comment 30 Brian Paul 2012-07-26 19:56:28 UTC
(In reply to comment #29)
> (In reply to comment #28)
> > Can you try making this change to radeon_span.c:
> > 
> > diff --git a/src/mesa/drivers/dri/radeon/radeon_span.c
> > b/src/mesa/drivers/dri/ra
> > index 1f2ba49..fd91f88 100644
> > --- a/src/mesa/drivers/dri/radeon/radeon_span.c
> > +++ b/src/mesa/drivers/dri/radeon/radeon_span.c
> > @@ -66,6 +66,7 @@ radeon_renderbuffer_map(struct gl_context *ctx, struct
> > gl_rend
> > 
> >         rrb->base.Map = map;
> >         rrb->base.RowStride = stride;
> > +       rrb->Base.ColorType = GL_UNSIGNED_BYTE;
> >  }
> > 
> >  static void
> 
> so I add this line in radeon_span.c, after the line 68 :
> 
> rrb->base.ColorType = GL_UNSIGNED_BYTE;
> 
> I compiled mesa lib, Blender runs without crash,

OK, great I'll push that fix.


> but it still very slow in the
> GUI ( too much lag when I click in the gui, menus ), it's not normal because
> with 7.11 mesa lib Blender was much more fast,

That'll be a bit more work to diagnose.  Have you ever used any profiling tools?

 
> I notice also some errors message in the console :
> 
> Mesa: User error: GL_INVALID_ENUM in
> glGetIntegerv(pname=GL_MAX_TEXTURE_IMAGE_UNITS)

I think you can safely ignore that.  I suspect that blender is querying GL_MAX_TEXTURE_IMAGE_UNITS without doing some extension checking.

> Error: Not freed memory blocks: 2
> IDProperty group len: 108 0xb91d808
> uiAfterFunc len: 616 0xb9e5f88

No idea about that one.
Comment 31 Roland Scheidegger 2012-07-26 20:10:28 UTC
(In reply to comment #29)

> I compiled mesa lib, Blender runs without crash, but it still very slow in the
> GUI ( too much lag when I click in the gui, menus ), it's not normal because
> with 7.11 mesa lib Blender was much more fast,
Unfortunately I think this is expected. Previously swrast didn't map whole buffers, only parts of them. For much simpler and more robust code though swrast fallbacks now maps the full buffers, an operation which is hideously expensive at least with radeons (as you've got to blit back the buffers from the card memory to system memory). I believe blender draws some little things on a large buffer, so the performance of the swrast fallback is fully dominated by copying the buffers around, the actual cost of the drawing operation in the swrast fallback is completely irrelevant. This is also why it's much faster to just use swrast to begin with as then you don't need to copy the buffers around 
(you should see this if you some some profiling tool like sysprof, oprofile, perf).
I'm not sure how this could be solved really, I think the best option would be to modify blender so it doesn't rely on these legacy features leading to fallbacks. Or similarly try to accelerate these in metaops so it doesn't need to fallback.
A third option would be to try to figure out the area accessed and only map those parts (to swrast it would still look like the whole buffer would be mapped to keep things simple, but the driver would only blit the needed parts). The infrastructure for that isn't there and I'm not sure it's desired in any case, and it would probably only work for the draw buffers and not for textures neither.


> 
> I notice also some errors message in the console :
> 
> Mesa: User error: GL_INVALID_ENUM in
> glGetIntegerv(pname=GL_MAX_TEXTURE_IMAGE_UNITS)
> Error: Not freed memory blocks: 2
> IDProperty group len: 108 0xb91d808
> uiAfterFunc len: 616 0xb9e5f88
I don't think this is related to this issue.
Comment 32 Barto 2012-07-26 20:13:04 UTC
(In reply to comment #30)

> That'll be a bit more work to diagnose.  Have you ever used any profiling
> tools?
> 

no, but I can try to learn how to use these tools,

I notice that the fallback variable is set again to GL_TRUE when I set a
breakpoint in meta.c, line 2258 

Breakpoint 1, _mesa_meta_DrawPixels (ctx=0xaa5d5e0, x=45, y=445, width=16,
height=16, format=6408, 
    type=5121, unpack=0xaa6c074, pixels=0xb2b2820) at
drivers/common/meta.c:2258
2258          fallback = GL_TRUE;


but I don't know what to do now, do you think we are close to the solution ?

maybe it's a problem with some for/next loops, unnecessary loops who occur only
with a r200 driver ( bad values or missing values ? ), that can explain the
lags in blender for the gui ?
Comment 33 Alex Deucher 2012-07-26 20:22:21 UTC
Also, IIRC, blender uses GL_SELECT for which is currently always software.  There is a patch to hw accelerate it, but it never made it into master:
http://cgit.freedesktop.org/mesa/mesa/log/?h=hw_gl_select
Comment 34 Roland Scheidegger 2012-07-26 20:24:37 UTC
(In reply to comment #31)
> Unfortunately I think this is expected. Previously swrast didn't map whole
> buffers, only parts of them. For much simpler and more robust code though
> swrast fallbacks now maps the full buffers, an operation which is hideously
> expensive at least with radeons (as you've got to blit back the buffers from
> the card memory to system memory). I believe blender draws some little things
> on a large buffer, so the performance of the swrast fallback is fully dominated
> by copying the buffers around, the actual cost of the drawing operation in the
> swrast fallback is completely irrelevant. This is also why it's much faster to
> just use swrast to begin with as then you don't need to copy the buffers around 
> (you should see this if you some some profiling tool like sysprof, oprofile,
> perf).
> I'm not sure how this could be solved really, I think the best option would be
> to modify blender so it doesn't rely on these legacy features leading to
> fallbacks. Or similarly try to accelerate these in metaops so it doesn't need
> to fallback.
> A third option would be to try to figure out the area accessed and only map
> those parts (to swrast it would still look like the whole buffer would be
> mapped to keep things simple, but the driver would only blit the needed parts).
> The infrastructure for that isn't there and I'm not sure it's desired in any
> case, and it would probably only work for the draw buffers and not for textures
> neither.
Forgot to mention, there's also another possible problem which could be avoided in the driver: it currently will do all the copying of the whole buffer always. Suppose the app does several glDrawPixels calls each just drawing a couple of pixels and each resulting in a fallback (I don't know if that is really happening with blender but it would be quite possible). The driver will do the whole-buffer copy for each of these calls - it would be possible to defer that (until the buffer is used for accelerated drawing again, so if the app does several DrawPixels some resulting in fallbacks some not it wouldn't help). But noone is really working much on these drivers nowadays.
Comment 35 Barto 2012-07-26 20:33:26 UTC
(In reply to comment #31)
> (In reply to comment #29)
> Unfortunately I think this is expected. Previously swrast didn't map whole
> buffers, only parts of them. For much simpler and more robust code though
> swrast fallbacks now maps the full buffers, an operation which is hideously
> expensive at least with radeons (as you've got to blit back the buffers from
> the card memory to system memory). I believe blender draws some little things
> on a large buffer, so the performance of the swrast fallback is fully dominated
> by copying the buffers around, the actual cost of the drawing operation in the
> swrast fallback is completely irrelevant. This is also why it's much faster to
> just use swrast to begin with as then you don't need to copy the buffers around 
> (you should see this if you some some profiling tool like sysprof, oprofile,
> perf).
> I'm not sure how this could be solved really, I think the best option would be
> to modify blender so it doesn't rely on these legacy features leading to
> fallbacks. Or similarly try to accelerate these in metaops so it doesn't need
> to fallback.
> A third option would be to try to figure out the area accessed and only map
> those parts (to swrast it would still look like the whole buffer would be
> mapped to keep things simple, but the driver would only blit the needed parts).
> The infrastructure for that isn't there and I'm not sure it's desired in any
> case, and it would probably only work for the draw buffers and not for textures
> neither.


very interesting, thanks,

I'm not sure to understand all the facts ( I'm a newbie in opengl/multimedia programming ), maybe, as you said the best solution would be to modify Blender,

but there is one thing I don't understand: why blender is very fast if I use the software mode for opengl with 8.0.4 mesa lib ( by using the command "export LIBGL_ALWAYS_SOFTWARE=1" before running blender ) ?

is it because only the CPU power works here for opengl instructions and not the graphic card ? ( in this case swrast works differently ? )

it's weird to see the graphic performances decreasing when I use the normal mode ( graphic card acceleration ) with Blender and see the exact opposite when I use the software mode for opengl operations in mesa 8.0.4
Comment 36 Roland Scheidegger 2012-07-26 21:42:26 UTC
(In reply to comment #33)
> Also, IIRC, blender uses GL_SELECT for which is currently always software. 
> There is a patch to hw accelerate it, but it never made it into master:
> http://cgit.freedesktop.org/mesa/mesa/log/?h=hw_gl_select
Oh yes that will also cause swrast (I don't know how frequent its use of GL_SELECT vs. some random operation like DrawPixels causing fallbacks is).
That patch wouldn't help though for the classic drivers.

(In reply to comment #35)
> I'm not sure to understand all the facts ( I'm a newbie in opengl/multimedia
> programming ), maybe, as you said the best solution would be to modify Blender,
> 
> but there is one thing I don't understand: why blender is very fast if I use
> the software mode for opengl with 8.0.4 mesa lib ( by using the command "export
> LIBGL_ALWAYS_SOFTWARE=1" before running blender ) ?
> 
> is it because only the CPU power works here for opengl instructions and not the
> graphic card ? ( in this case swrast works differently ? )
> 
> it's weird to see the graphic performances decreasing when I use the normal
> mode ( graphic card acceleration ) with Blender and see the exact opposite when
> I use the software mode for opengl operations in mesa 8.0.4
Yes typically you'd expect hardware to be (much) faster.
The problem here though is however that (likely) all the drawing operations are fairly cheap, but the graphic chip is doing nothing but constantly copying the buffers from graphic card memory to system memory and back due to these fallbacks. Something you don't have to do if the buffers just stay in system memory because you only use the cpu to access them.
And the cost of that is going to kill you - now I don't know what exactly blender is doing but when it crashed the function was called icon_draw_rect(), so it's probably fair to assume this was some simple, small rectangle to draw. Most likely really just a couple of pixels, but the gpu will most likely blit an area corresponding the the whole window from card memory to system memory (and back) for being able to draw these couple of pixels.
And if it was drawing say 20 small icons it will copy that whole large buffer 20 times back and forth...
Now as said the code in earlier mesa versions didn't have to copy the whole buffers but could copy them partially, so the cost of the fallbacks was generally smaller for cases like this, but on the flipside it didn't quite work right some of the time neither. And the new code makes things easier for swrast itself - the effort has gone to avoid fallbacks whenever possible instead.
Comment 37 Brian Paul 2012-07-26 22:13:28 UTC
Try setting the RADEON_DEBUG env var to "fall".  Ex: export RADEON_DEBUG=fall
then run blender.  It should print a message to stdout/err when a software fallback is hit.  That should give some clue.

If it's just glDrawPixels we could try to just map the region that'll be touched by the glDrawPixels to try to speed things up.
Comment 38 Barto 2012-07-26 22:33:11 UTC
(In reply to comment #37)
> Try setting the RADEON_DEBUG env var to "fall".  Ex: export RADEON_DEBUG=fall
> then run blender.  It should print a message to stdout/err when a software
> fallback is hit.  That should give some clue.
> 
> If it's just glDrawPixels we could try to just map the region that'll be
> touched by the glDrawPixels to try to speed things up.


I did the "export RADEON_DEBUG=fall" but I see no messages related to software fallback when I run blender,

there is only this error message already seen before :

Mesa: User error: GL_INVALID_ENUM in glGetIntegerv(pname=GL_MAX_TEXTURE_IMAGE_UNITS)
Comment 39 Brian Paul 2012-07-26 23:39:26 UTC
Try setting a breakpoint on radeonSpanRenderStart() and get some stack traces when gdb stops there.  Maybe set the breakpoint after the splash screen/window goes away because it's sure to stop from the glDrawPixels commands used there.
Comment 40 Roland Scheidegger 2012-07-27 00:24:19 UTC
I was actually wondering, do we really need the fallback when there's PixelTransfer state? Clearly this applies to DrawPixels, but doesn't this also apply to TexImage() calls hence mesa would just do that when we create the texture out of the pixel data to draw a quad?
(Of course the fastest way to handle scale/bias transfer state would probably be to just use MODULATE/ADD/MODULATE_ADD tex combiner so mesa wouldn't have to do that in software - one of scale/bias could use tex env color whereas if we have both could just set up a source color too).
FWIW the _ImageTransferState fallback decision looks quite bogus in any case - this state includes for instance rgb_scale but not depth_scale, hence when drawing to a depth texture the fallback decision is probably not quite right (though I guess just skipping fallback would break drawing to stencil buffer, since this is set up as a alpha texture mesa would probably incorrectly apply rgb transfer ops there - but that's certainly solvable without having to resort to fallback).
Also fog fallback shouldn't apply to depth/stencil buffer drawing neither (but OTOH enabled texturing should cause a fallback for color buffer drawing, even though I'd guess if some app really does a drawpixels with enabled fog or texturing it is probably by mistake not intentionally...).

--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -2280,8 +2280,7 @@ _mesa_meta_DrawPixels(struct gl_context *ctx,
     * Determine if we can do the glDrawPixels with texture mapping.
     */
    fallback = GL_FALSE;
-   if (ctx->_ImageTransferState ||
-       ctx->Fog.Enabled) {
+   if (ctx->Fog.Enabled) {
       fallback = GL_TRUE;
    }
Comment 41 Barto 2012-07-27 01:21:23 UTC
(In reply to comment #39)
> Try setting a breakpoint on radeonSpanRenderStart() and get some stack traces
> when gdb stops there.  Maybe set the breakpoint after the splash screen/window
> goes away because it's sure to stop from the glDrawPixels commands used there.

I set a breakpoint after the splash screen, here is the backtrace :

reakpoint 5, radeonSpanRenderStart (ctx=0xaa5d5e0) at radeon_span.c:119
119             radeonContextPtr rmesa = RADEON_CONTEXT(ctx);
(gdb) bt
#0  radeonSpanRenderStart (ctx=0xaa5d5e0) at radeon_span.c:119
#1  0xb1e4e8c4 in swrast_render_start (ctx=0xaa5d5e0) at swrast/s_context.h:343
#2  0xb1e4fa48 in draw_rgba_pixels (ctx=0xaa5d5e0, x=642, y=367, width=16, height=16, format=6408, 
    type=5121, unpack=0xaa6c074, pixels=0xb3e0488) at swrast/s_drawpix.c:425
#3  0xb1e50475 in _swrast_DrawPixels (ctx=0xaa5d5e0, x=642, y=367, width=16, height=16, format=6408, 
    type=5121, unpack=0xaa6c074, pixels=0xb3e0488) at swrast/s_drawpix.c:694
#4  0xb1d63366 in _mesa_meta_DrawPixels (ctx=0xaa5d5e0, x=642, y=367, width=16, height=16, format=6408, 
    type=5121, unpack=0xaa6c074, pixels=0xb3e0488) at drivers/common/meta.c:2314
#5  0xb1de7c2e in _mesa_DrawPixels (width=16, height=16, format=6408, type=5121, pixels=0xb3e0488)
    at main/drawpix.c:131
#6  0x088611c4 in icon_draw_rect.isra.1 ()
#7  0x088614a5 in icon_draw_size.isra.3 ()
#8  0x08863675 in UI_icon_draw_aspect ()
#9  0x0888581c in widget_draw_icon ()
#10 0x08886073 in widget_draw_text_icon ()
#11 0x0888962e in ui_draw_but ()
#12 0x0884b71e in uiDrawBlock ()
#13 0x086b182e in buttons_header_buttons ()
#14 0x086aed4f in buttons_header_area_draw ()
#15 0x08905f28 in ED_region_do_draw ()
#16 0x0868cd1b in wm_method_draw_overlap_all ()
#17 0x0868df34 in wm_draw_update ()
#18 0x0868c738 in WM_main ()
#19 0x0865ae01 in main ()

gdb reaches immediatly the breakpoint if the mouse cursor is in the GUI of blender
Comment 42 Barto 2012-07-27 01:40:32 UTC
(In reply to comment #40)

> 
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -2280,8 +2280,7 @@ _mesa_meta_DrawPixels(struct gl_context *ctx,
>      * Determine if we can do the glDrawPixels with texture mapping.
>      */
>     fallback = GL_FALSE;
> -   if (ctx->_ImageTransferState ||
> -       ctx->Fog.Enabled) {
> +   if (ctx->Fog.Enabled) {
>        fallback = GL_TRUE;
>     }

congratulations !

I used your patch and now all is ok with blender and mesa 8.0.4,

blender is now very fast, no lag in GUI, it's great !

a simple change in src/mesa/drivers/common/meta.c seems to speed up blender :

the good conditionnal in line 2256 seems to fix the lag in blender :

 if (ctx->Fog.Enabled) { 
      fallback = GL_TRUE;
   }

I think the bug is solved now with these 2 changes :

1) the fix from Brian solves the crash at startup ( adding this line in radeon_span.c, after the line 68 :

rrb->base.ColorType = GL_UNSIGNED_BYTE; ) 

2) and this optimization from Roland solves the slowness :

 if (ctx->Fog.Enabled) { 
      fallback = GL_TRUE;
   }

in meta.c at line 2256

thank you both
Comment 43 Brian Paul 2012-07-27 14:16:09 UTC
(In reply to comment #40)
> I was actually wondering, do we really need the fallback when there's
> PixelTransfer state? Clearly this applies to DrawPixels, but doesn't this also
> apply to TexImage() calls hence mesa would just do that when we create the
> texture out of the pixel data to draw a quad?

Yes, that's correct.  Removing the _ImageTransferState test is correct.  I've tested it for color and depth scale/bias.


> FWIW the _ImageTransferState fallback decision looks quite bogus in any case -
> this state includes for instance rgb_scale but not depth_scale, hence when
> drawing to a depth texture the fallback decision is probably not quite right
> (though I guess just skipping fallback would break drawing to stencil buffer,
> since this is set up as a alpha texture mesa would probably incorrectly apply
> rgb transfer ops there - but that's certainly solvable without having to resort
> to fallback).

If we'd save/restore the MESA_META_PIXEL_TRANSFER state for stencil draws, that would fix that.  Nice catch.  I'll post a patch with these changes.


> Also fog fallback shouldn't apply to depth/stencil buffer drawing neither (but
> OTOH enabled texturing should cause a fallback for color buffer drawing, even
> though I'd guess if some app really does a drawpixels with enabled fog or
> texturing it is probably by mistake not intentionally...).

Actually, I believe fog would apply to a glDrawPixels(GL_DEPTH) since we generate fragments with color + Z in that case and the colors would be subjected to fog, just like glDrawPixels(GL_RGB).  But it's a pretty obscure case and I'm not going to worry about it for now.
Comment 44 Roland Scheidegger 2012-07-27 14:54:22 UTC
(In reply to comment #43)
> > Also fog fallback shouldn't apply to depth/stencil buffer drawing neither (but
> > OTOH enabled texturing should cause a fallback for color buffer drawing, even
> > though I'd guess if some app really does a drawpixels with enabled fog or
> > texturing it is probably by mistake not intentionally...).
> 
> Actually, I believe fog would apply to a glDrawPixels(GL_DEPTH) since we
> generate fragments with color + Z in that case and the colors would be
> subjected to fog, just like glDrawPixels(GL_RGB).  But it's a pretty obscure
> case and I'm not going to worry about it for now.
Yes but why would the generated color (modified by fog) matter in this case?
Also there's WAY more obscure cases which look handled wrong (I guess that we don't see bugs filed about them is a testament that the whole "DrawPixel/Bitmap is like ordinary rasterization" is a serious misdesign):
- enabled ColorSum (secondary color comes from current raster pos) for color buffer drawing
- enabled AlphaTest (when drawing to stencil/depth buffer so color comes from raster pos and not the pixel data)
- enabled texturing
- enabled fragment program / shader
- probably way more (crazy things like depth/stencil drawpixels with multisampled buffer and alpha_to_coverage?)
At least I don't see how any of these could work with metaops.
Comment 45 Brian Paul 2012-07-27 15:12:02 UTC
(In reply to comment #44)
> (In reply to comment #43)
> > > Also fog fallback shouldn't apply to depth/stencil buffer drawing neither (but
> > > OTOH enabled texturing should cause a fallback for color buffer drawing, even
> > > though I'd guess if some app really does a drawpixels with enabled fog or
> > > texturing it is probably by mistake not intentionally...).
> > 
> > Actually, I believe fog would apply to a glDrawPixels(GL_DEPTH) since we
> > generate fragments with color + Z in that case and the colors would be
> > subjected to fog, just like glDrawPixels(GL_RGB).  But it's a pretty obscure
> > case and I'm not going to worry about it for now.

> Yes but why would the generated color (modified by fog) matter in this case?

I'm only saying that fog (surprisingly?) applies to glDrawPixels(GL_DEPTH) according to the OpenGL spec.  I'm not saying anything about the meta implementation.


> Also there's WAY more obscure cases which look handled wrong (I guess that we
> don't see bugs filed about them is a testament that the whole "DrawPixel/Bitmap
> is like ordinary rasterization" is a serious misdesign):
> - enabled ColorSum (secondary color comes from current raster pos) for color
> buffer drawing
> - enabled AlphaTest (when drawing to stencil/depth buffer so color comes from
> raster pos and not the pixel data)
> - enabled texturing
> - enabled fragment program / shader
> - probably way more (crazy things like depth/stencil drawpixels with
> multisampled buffer and alpha_to_coverage?)
> At least I don't see how any of these could work with metaops.

I completely agree that there's probably quite a few corner cases that are mishandled in the meta DrawPixels/Bitmap/etc code.

Anyway, I think we can close out this particular bug if the above-mentioned patches look OK to everyone.
Comment 46 Barto 2012-07-27 20:24:42 UTC
(In reply to comment #45)
> (In reply to comment #44)

> Anyway, I think we can close out this particular bug if the above-mentioned
> patches look OK to everyone.

for me it's Ok with your fix ( "rrb->base.ColorType = GL_UNSIGNED_BYTE;" radeon_span.c line 68  ) and the optimization patch from Roland for meta.c at line 2256,

I have a radeon 9000 ( M9, rv250 ) in a laptop ( pentium 4 2.4 Ghz ), archlinux 32 bits, I see no regression with these patches
Comment 47 Brian Paul 2012-07-27 21:59:45 UTC
OK, I'm closing this bug.  These 3 commits fix the assertion and performance issue:

906febaf8bd37fffa4fc14c56eda1dd6e4b4dcda
38184dcd54e77c8f9adc89d337a97afd630b2c07
0e893b42610a2e8f2797bd7da68669dac38d9538

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.