Re: [PATCH 3/5] wined3d: Fail to create shaders using more than the supported number of float constants.

[Henri Verbeet]

On 30 September 2014 14:51, Matteo Bruni  wrote:
> -
>    case WINED3D_SHADER_VERSION(1, 0):
> -
>    case WINED3D_SHADER_VERSION(1, 1):
> -
>        shader->limits.constant_bool = 0;
> -
>        shader->limits.constant_int = 0;
> -
>        shader->limits.packed_output = 12;
> -
>        shader->limits.sampler = 0;
> -
>        /* TODO: vs_1_1 has a minimum of 96 constants. What happens when
> -
>         * a vs_1_1 shader is used on a vs_3_0 capable card that has 256
> -
>         * constants? */
> -
>        shader->limits.constant_float = min(256, vs_uniform_count);

Since you touched it... Perhaps at least as interesting as what
happens when trying to create SM3 shaders that use more that 256
constants is what happens when trying to create e.g. SM1 shaders that
use more than 96 constants.

Also note that aside from the somewhat questionable clamping of
"constant_float" to the uniform count, this is all just static const
data, and there's really no reason that every single shader needs its
own copy of it. I don't think there are a lot of users of
"constant_float" that don't already do that kind of clamping
themselves, so we should probably just fix the remaining ones, if any.
> - switch (reg_maps->shader_version.type)
> - {
> -
>    case WINED3D_SHADER_TYPE_VERTEX:
> -
>        vertexshader_set_limits(shader);
> -
>        break;
> -
>    case WINED3D_SHADER_TYPE_GEOMETRY:
> -
>        geometryshader_set_limits(shader);
> -
>        break;
> -
>    case WINED3D_SHADER_TYPE_PIXEL:
> -
>        pixelshader_set_limits(shader);
> -
>        break;
> -
>    default:
> -
>        FIXME("Unexpected shader type %u found.\n", reg_maps->shader_version.type);
> - }
> - reg_maps->constf = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
>            sizeof(*reg_maps->constf) * ((constf_size + 31) / 32));

But now you can use "shader->limits.constant_float" instead of "constf_size".

[PATCH 3/5] wined3d: Fail to create shaders using more than the supported number of float constants.

[Matteo Bruni]

2014-09-30 16:55 GMT+02:00 Henri Verbeet :
> On 30 September 2014 14:51, Matteo Bruni  wrote:
> > -
> >    case WINED3D_SHADER_VERSION(1, 0):
> > -
> >    case WINED3D_SHADER_VERSION(1, 1):
> > -
> >        shader->limits.constant_bool = 0;
> > -
> >        shader->limits.constant_int = 0;
> > -
> >        shader->limits.packed_output = 12;
> > -
> >        shader->limits.sampler = 0;
> > -
> >        /* TODO: vs_1_1 has a minimum of 96 constants. What happens when
> > -
> >         * a vs_1_1 shader is used on a vs_3_0 capable card that has 256
> > -
> >         * constants? */
> > -
> >        shader->limits.constant_float = min(256, vs_uniform_count);
>
> Since you touched it...

Fair enough ;)
> Perhaps at least as interesting as what
> happens when trying to create SM3 shaders that use more that 256
> constants is what happens when trying to create e.g. SM1 shaders that
> use more than 96 constants.

Interesting that I had that comment under my nose and somehow managed
to ignore it. Sure, I'm going to add one or more tests for that.
> Also note that aside from the somewhat questionable clamping of
> "constant_float" to the uniform count, this is all just static const
> data, and there's really no reason that every single shader needs its
> own copy of it. I don't think there are a lot of users of
> "constant_float" that don't already do that kind of clamping
> themselves, so we should probably just fix the remaining ones, if any.

Good idea.
> > - switch (reg_maps->shader_version.type)
> > - {
> > -
> >    case WINED3D_SHADER_TYPE_VERTEX:
> > -
> >        vertexshader_set_limits(shader);
> > -
> >        break;
> > -
> >    case WINED3D_SHADER_TYPE_GEOMETRY:
> > -
> >        geometryshader_set_limits(shader);
> > -
> >        break;
> > -
> >    case WINED3D_SHADER_TYPE_PIXEL:
> > -
> >        pixelshader_set_limits(shader);
> > -
> >        break;
> > -
> >    default:
> > -
> >        FIXME("Unexpected shader type %u found.\n", reg_maps->shader_version.type);
> > - }
> > - reg_maps->constf = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
> >            sizeof(*reg_maps->constf) * ((constf_size + 31) / 32));
>
> But now you can use "shader->limits.constant_float" instead of "constf_size".

Good point as well.