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 <mbruni(a)codeweavers.com> 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 <hverbeet(a)gmail.com>:
> On 30 September 2014 14:51, Matteo Bruni <mbruni(a)codeweavers.com> 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.