[Mesa-dev] [PATCH] mesa: make sure all lighting tables are updated before the computation

Brian Paul brianp at vmware.com
Wed Nov 16 06:39:21 PST 2011


On 11/15/2011 08:18 PM, Yuanhan Liu wrote:
> On Tue, Nov 15, 2011 at 07:26:52AM -0700, Brian Paul wrote:
>> On 11/15/2011 12:51 AM, Yuanhan Liu wrote:
>>> Make sure all lighting tables are updated before using the table to
>>> calculate something, say using _SpotExpTable to calculate
>>> _VP_inf_spot_attenuation.
>>>
>>> Signed-off-by: Yuanhan Liu<yuanhan.liu at linux.intel.com>
>>> ---
>>>   src/mesa/main/light.c |    3 +++
>>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/src/mesa/main/light.c b/src/mesa/main/light.c
>>> index c27cf1d..60daa89 100644
>>> --- a/src/mesa/main/light.c
>>> +++ b/src/mesa/main/light.c
>>> @@ -1138,6 +1138,9 @@ compute_light_positions( struct gl_context *ctx )
>>>         TRANSFORM_NORMAL( ctx->_EyeZDir, eye_z, ctx->ModelviewMatrixStack.Top->m );
>>>      }
>>>
>>> +   /* Make sure all the light tables are updated before the computation */
>>> +   _mesa_validate_all_lighting_tables(ctx);
>>> +
>>>      foreach (light,&ctx->Light.EnabledList) {
>>>
>>>         if (ctx->_NeedEyeCoords) {
>>
>> Does this fix a specific bug?
> Nope(well, I didn't search the bugzilla). But it did fix an issue. I
> wrote a piglit case for this issue.
>
> ---
>  From 47796b40c7f300e289e3a82361164b62e9e3411c Mon Sep 17 00:00:00 2001
> From: Yuanhan Liu<yuanhan.liu at linux.intel.com>
> Date: Wed, 16 Nov 2011 11:16:18 +0800
> Subject: [PATCH] Add an infinite spot light test case
>
> Signed-off-by: Yuanhan Liu<yuanhan.liu at linux.intel.com>
> ---
>   tests/all.tests                     |    1 +
>   tests/general/CMakeLists.gl.txt     |    1 +
>   tests/general/infinite-spot-light.c |   99 +++++++++++++++++++++++++++++++++++
>   3 files changed, 101 insertions(+), 0 deletions(-)
>   create mode 100644 tests/general/infinite-spot-light.c
>
> diff --git a/tests/all.tests b/tests/all.tests
> index 2a85b7a..664a9b4 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -254,6 +254,7 @@ add_plain_test(general, 'geterror-invalid-enum')
>   add_plain_test(general, 'geterror-inside-begin')
>   add_plain_test(general, 'gl30basic')
>   add_plain_test(general, 'hiz')
> +add_plain_test(general, 'infinite-spot-light')
>   add_plain_test(general, 'isbufferobj')
>   add_plain_test(general, 'line-aa-width')
>   add_plain_test(general, 'linestipple')
> diff --git a/tests/general/CMakeLists.gl.txt b/tests/general/CMakeLists.gl.txt
> index 8ff3771..2cfc7be 100644
> --- a/tests/general/CMakeLists.gl.txt
> +++ b/tests/general/CMakeLists.gl.txt
> @@ -62,6 +62,7 @@ if (UNIX)
>   	target_link_libraries (hiz m)
>   endif (UNIX)
>   add_executable (early-z early-z.c)
> +add_executable (infinite-spot-light infinite-spot-light.c)
>   add_executable (isbufferobj isbufferobj.c)
>   add_executable (linestipple linestipple.c)
>   add_executable (line-aa-width line-aa-width.c)
> diff --git a/tests/general/infinite-spot-light.c b/tests/general/infinite-spot-light.c
> new file mode 100644
> index 0000000..cb8616f
> --- /dev/null
> +++ b/tests/general/infinite-spot-light.c
> @@ -0,0 +1,99 @@
> +/*
> + * Copyright (C) 2011 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Yuanhan Liu<yuanhan.liu at linux.intel.com>
> + */
> +
> +/** @file infinite-spot-light.c
> + *
> + * This is a case that sounds doesn't make sense, but it is allowed by glSpec
> + * (see section 2.14.1 Lightin of glspec 2.1.pdf).  While writing this case,
> + * it servers as two purposes:
> + *
> + *   1. Test if swrast is OK with this case.
> + *      The old mesa code would always compute a zero attenuation, thus always
> + *      get a black lighting color.
> + *
> + *   2. Test if hardware rendering(only i965 tested) is OK with this patch.
> + *      The old mesa code would skip the attenuation and spot computation while
> + *      infinite light is met. This is somehow not permitted by glSpec.
> + */
> +
> +#include "piglit-util.h"
> +
> +int piglit_width = 100, piglit_height = 100;
> +int piglit_window_mode = GLUT_RGB | GLUT_DOUBLE;
> +
> +/* Already normalized, and 0.5 would be the expected color */
> +static GLfloat dir[3] = {0.866025404, 0.0, 0.5};
> +static GLfloat pos[4] = {0.0, 0.0, -1.0, 0.0};		/* infinite */
> +static GLfloat light_ambient[3] = {1.0, 0.0, 0.0};
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +	GLboolean pass = GL_TRUE;
> +	GLfloat expected[4] = {0.5, 0.0, 0.0, 1.0};
> +
> +	glClear(GL_COLOR_BUFFER_BIT);
> +
> +	glPointSize(10);
> +	glBegin(GL_POINTS);
> +		glVertex2f(0.5, 0.5);
> +	glEnd();
> +
> +	pass = piglit_probe_pixel_rgba(0, 0, expected);
> +
> +	glutSwapBuffers();
> +
> +	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	GLfloat buf[4] = {0.0, 0.0, 0.0, 1.0};
> +
> +	glEnable(GL_LIGHT0);
> +	glLightf(GL_LIGHT0, GL_SPOT_CUTOFF, 89.0);
> +	glLightf(GL_LIGHT0, GL_SPOT_EXPONENT, 1.0);
> +	glLightfv(GL_LIGHT0, GL_POSITION, pos);
> +	glLightfv(GL_LIGHT0, GL_SPOT_DIRECTION, dir);
> +	glLightfv(GL_LIGHT0, GL_AMBIENT, light_ambient);
> +
> +	glLightModelfv(GL_LIGHT_MODEL_AMBIENT, buf);
> +	glLightModeli(GL_LIGHT_MODEL_TWO_SIDE, 0);
> +
> +	glMaterialfv(GL_FRONT, GL_DIFFUSE, buf);
> +	glMaterialfv(GL_FRONT, GL_SPECULAR, buf);
> +
> +	buf[0] = 1.0;
> +	buf[1] = 1.0;
> +	buf[2] = 1.0;
> +	glMaterialfv(GL_FRONT, GL_AMBIENT, buf);
> +
> +	glEnable(GL_LIGHTING);
> +
> +	piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
> +}

I tested this with NVIDIA's driver too.  Looks good.

Reviewed-by: Brian Paul <brianp at vmware.com>


More information about the mesa-dev mailing list