Re: [PATCH 4/4] ddraw: Test if P8 primaries can be converted to RGB.

[Henri Verbeet]

On 6 July 2014 18:06, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
> diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
> index 3d61e61..7367c01 100644
> --- a/dlls/ddraw/surface.c
> +++ b/dlls/ddraw/surface.c
> @@ -5838,6 +5838,7 @@ HRESULT ddraw_surface_create(struct ddraw *ddraw, const DDSURFACEDESC2 *surface_
>          {
>              enum wined3d_resource_type rtype;
>              DWORD usage = 0;
> +            enum wined3d_format_id adapter_format = mode.format_id;
>
> if (desc->ddsCaps.dwCaps2 & DDSCAPS2_CUBEMAP)
>                  rtype = WINED3D_RTYPE_CUBE_TEXTURE;
> @@ -5851,8 +5852,11 @@ HRESULT ddraw_surface_create(struct ddraw *ddraw, const DDSURFACEDESC2 *surface_
>              else if (desc->ddsCaps.dwCaps & DDSCAPS_3DDEVICE)
>                  usage = WINED3DUSAGE_RENDERTARGET;
>
> +            if (desc->ddsCaps.dwCaps & DDSCAPS_PRIMARYSURFACE)
> +                adapter_format = WINED3DFMT_B8G8R8X8_UNORM;
> +
>              if (SUCCEEDED(hr = wined3d_check_device_format(ddraw->wined3d, WINED3DADAPTER_DEFAULT,
> -                    WINED3D_DEVICE_TYPE_HAL, mode.format_id, usage, rtype, wined3d_desc.format)))
> +                    WINED3D_DEVICE_TYPE_HAL, adapter_format, usage, rtype, wined3d_desc.format)))

I don't think this fix is quite correct either. The drawable format is
a wined3d internal thing, ddraw shouldn't have to know about it. In
principle it wouldn't necessarily need to be WINED3DFMT_B8G8R8X8_UNORM
either, for example if we ever get around to supporting 10 bpc display
modes. So I think the fix for this would need to be in
wined3d_check_device_format(), or its utility functions.

[PATCH 4/4] ddraw: Test if P8 primaries can be converted to RGB.

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2014-07-07 14:53, schrieb Henri Verbeet:
> I don't think this fix is quite correct either. The drawable format
> is a wined3d internal thing, ddraw shouldn't have to know about it.
> In principle it wouldn't necessarily need to be
> WINED3DFMT_B8G8R8X8_UNORM either, for example if we ever get around
> to supporting 10 bpc display modes.
> The drawable format is a winex11.drv / winemac.drv internal thing.
> wined3d calls ChangeDisplaySettingsEx to set an 8 bpp mode, but
> winex11 never really complies with that request. Wined3d depends on
> that non-compliance though. I guess wined3d P8 support currently fails
> on Windows.

In practice WINED3DFMT_B8G8R8X8_UNORM (or any other non-indexed RGB
format) gives us the right answer. Ideally we'd make the emulated P8
mode explicit, but then we have to take care that applications that
query the display mode with user32 don't break.
> So I think the fix for this would need to be in 
> wined3d_check_device_format(), or its utility functions.
> I don't think so. Wined3d_check_device_format doesn't know what ddraw
> is going to do with the source surface. What if e.g. the application
> tries to create a YUY2 offscreen surface in P8 mode?

I'm open to the argument that blits between converted surfaces with
the same format should be reported as supposed. But in that case we'd
get the right answer for the wrong reason.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJTup/tAAoJEN0/YqbEcdMwvS8P+wTD77w6V4o8BvYeil5V5LWp
sS3iXWW4GqXx5rMXv84O4qNkXcfQtTfdAOwtOz3bngFdlF3UEHEd8P7XIJzWJpH6
LgV2CCCz37LBnNWEaU8Nrd13o7jlDuoGS10c2hyBSwJCPC/84jbnJfVwiaCwizyH
N3F1q+GxXW5vMM7FeEZk0hkrDSPaL4dQ/6qIVR5CKMwNe/+86voXb3VvVwzlUraw
pexRhte+/uebPbwo16PUXhTaH8E6UGuPpVH3ANxNJweK4aLbbJmEhayMgDGmgFgo
S52sIGOjAEV2zIZqstild7n90Pdp9e9rfCrhQ4lTLnyseveyUHoEjqHaDjovzcoN
W1c77gLIhPKgB8reHI6HZAofyuYC5osobK6cikHJNF4ixsiAA87f+mizxrSCql3q
uYlcYWqoWHy9GcChkkNFFayMdOa7+dhLlkfp+GYrk0oQkOyU6zs8fN2Jej9GLf/b
NN4vAd9jhyPvxLuUztrlx3LCzZ4Dt/iJ7O1nGCusU+Jq/m/61XZOXj+luF3wGREA
u0DftXL6kEu/ucx+/VD6KwYCuNpdy9/5OgnLd9NIqzFB91xegoAzEqLsubnik2Iq
8hYvTOcju7mgTUF+ZnGnqy4U88F2CqLAyK10PTt4qLFDMls/pQbv5R58A0z27021
CcTfiLPJLJC5+HF138YP
=4KW+
-----END PGP SIGNATURE-----

[PATCH 4/4] ddraw: Test if P8 primaries can be converted to RGB.

[Henri Verbeet]

On 7 July 2014 15:26, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
> The drawable format is a winex11.drv / winemac.drv internal thing.
> In a sense, but wined3d picks it in context_create() /
> context_choose_pixel_format(). That's also where WINED3DFMT_P8_UINT is
> overridden to WINED3DFMT_B8G8R8A8_UNORM.
> > So I think the fix for this would need to be in
> > wined3d_check_device_format(), or its utility functions.
> > I don't think so. Wined3d_check_device_format doesn't know what ddraw
> > is going to do with the source surface. What if e.g. the application
> > tries to create a YUY2 offscreen surface in P8 mode?

Does it really matter what ddraw is going to do with it, beyond what's
passed in through usage / resource_type? In principle I see no reason
why wined3d shouldn't allow offscreen YUY2 surfaces in P8 mode.
> I'm open to the argument that blits between converted surfaces with
> the same format should be reported as supposed.
> In some cases, yes, but when e.g. stretching is involved that's not
> necessarily true either.