Re: [PATCH 2/5] wined3d: Limit the number of multisample levels reported by wined3d_check_device_multisample_type().

[Henri Verbeet]

On 14 December 2015 at 23:37, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
> @@ -4426,7 +4428,7 @@ HRESULT CDECL wined3d_check_device_multisample_type(const struct wined3d *wined3
>      if (quality_levels)
>      {
>          if (multisample_type == WINED3D_MULTISAMPLE_NON_MASKABLE)
> -            *quality_levels = gl_info->limits.samples;
> +            *quality_levels = wined3d_log2i(gl_info->limits.samples);
>          else
>              *quality_levels = 1;
>      }
> I'm not sure this really makes sense. It may make sense to limit the
> number of non-maskable quality levels to 8, similar to how we limit
> e.g. shader constants and render targets for d3d9, although that would
> need a test and a comment. I don't see how you can justify the
> wined3d_log2i().

As an aside, note that we don't actually support maskable
multi-sampling and would need ARB_texture_multisample for that,
although applications don't seem to do a lot of masking in practice.
D3d10+ only has maskable multi-sampling.

[PATCH 2/5] wined3d: Limit the number of multisample levels reported by wined3d_check_device_multisample_type().

[Matteo Bruni]

2015-12-15 11:10 GMT+01:00 Henri Verbeet <hverbeet(a)gmail.com>:
> On 14 December 2015 at 23:37, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
> > @@ -4426,7 +4428,7 @@ HRESULT CDECL wined3d_check_device_multisample_type(const struct wined3d *wined3
> >      if (quality_levels)
> >      {
> >          if (multisample_type == WINED3D_MULTISAMPLE_NON_MASKABLE)
> > -            *quality_levels = gl_info->limits.samples;
> > +            *quality_levels = wined3d_log2i(gl_info->limits.samples);
> >          else
> >              *quality_levels = 1;
> >      }
> > I'm not sure this really makes sense. It may make sense to limit the
> > number of non-maskable quality levels to 8, similar to how we limit
> > e.g. shader constants and render targets for d3d9, although that would
> > need a test and a comment. I don't see how you can justify the
> > wined3d_log2i().

It seems to roughly match native, where higher quality levels for
WINED3D_MULTISAMPLE_NON_MASKABLE generally mean an increased number of
samples per pixel (at least going by the output of DX9FSAAViewer, from
bug 12652) and sample counts often go up by powers-of-two. That's not
always the case of course but it seems like we can use the idea as a
possible valid implementation of WINED3D_MULTISAMPLE_NON_MASKABLE.

Anyway I just realized this patch is missing the related changes to
surface_prepare_rb(), as-is the patch doesn't make sense... Something
like the attached patch is what I intended.

Actually there is also another option. Hardcoding the quality levels
count to 1 might be acceptable? We might then just always pick
MAX_SAMPLES (or maybe 2) as the samples count for
WINED3D_MULTISAMPLE_NON_MASKABLE renderbuffers.

[PATCH 2/5] wined3d: Limit the number of multisample levels reported by wined3d_check_device_multisample_type().

[Henri Verbeet]

On 15 December 2015 at 21:39, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
> It seems to roughly match native, where higher quality levels for
> WINED3D_MULTISAMPLE_NON_MASKABLE generally mean an increased number of
> samples per pixel (at least going by the output of DX9FSAAViewer, from
> bug 12652) and sample counts often go up by powers-of-two. That's not
> always the case of course but it seems like we can use the idea as a
> possible valid implementation of WINED3D_MULTISAMPLE_NON_MASKABLE.
>
> Anyway I just realized this patch is missing the related changes to
> surface_prepare_rb(), as-is the patch doesn't make sense... Something
> like the attached patch is what I intended.

It does make a lot more sense like that. It isn't necessarily obvious
to me that the quality levels aren't supposed to just be the sample
counts though. I guess I'll have a look at DX9FSAAViewer.