[Mesa-dev] [PATCH] llvmpipe: don't assert/crash on invalid clear color

[Roland Scheidegger]

Am 25.11.2013 09:51, schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
> 
> So GL4.3 spec pretty much says glClear on integer buffers is undefined,
> then we have a piglit multisample test ext_framebuffer_multisample-int-draw-buffers-alpha-to-coverage
> that actually does undefined things but doesn't rely on the results except
> not crashing.
> 
> (The tests binds 3 colorbuffers, one int, 2 float to an fbo and then calls glClear).
> 
> This stops llvmpipe from crashing with is worse than undefined imho, though I'm not
>  sure if the test should be fixed, and also a new test written to show this undefined
> behaviour outside multisamples.
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  src/gallium/drivers/llvmpipe/lp_rast.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c b/src/gallium/drivers/llvmpipe/lp_rast.c
> index af661e9..75e1593 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast.c
> +++ b/src/gallium/drivers/llvmpipe/lp_rast.c
> @@ -142,9 +142,11 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
>              if (util_format_is_pure_sint(format)) {
>                 util_format_write_4i(format, arg.clear_color.i, 0, &uc, 0, 0, 0, 1, 1);
>              }
> -            else {
> -               assert(util_format_is_pure_uint(format));
> +            else if (util_format_is_pure_uint(format)) {
>                 util_format_write_4ui(format, arg.clear_color.ui, 0, &uc, 0, 0, 0, 1, 1);
> +            } else {
> +               util_pack_color(arg.clear_color.f,
> +                               scene->fb.cbufs[i]->format, &uc);
>              }
>  
>              util_fill_box(scene->cbufs[i].map,
> 

Oh I probably thought such framebuffers were illegal when I wrote it
that way. But doesn't look like that's the case at all.
This patch looks ok, but I wonder if it wouldn't make sense to
restructure this whole function so simply the decision if int or
"ordinary" float path be taken would be per attachment, rather than do
it for the complete framebuffer and then still have to handle the float
case inside the int case anyway...

Roland