[PATCH v2 2/6] ddraw: Create a separate texture for drawing with bindable sysmem surfaces.

[Paul Gofman]

On 3/2/21 20:30, Henri Verbeet wrote:
> On Tue, 2 Mar 2021 at 18:03, Paul Gofman <pgofman at codeweavers.com> wrote:
>> On 3/2/21 19:16, Henri Verbeet wrote:
>>>> @@ -6951,6 +6951,12 @@ static HRESULT d3d_device_init(struct d3d_device *device, struct ddraw *ddraw, c
>>>>      device->hardware_device = IsEqualGUID(&IID_IDirect3DTnLHalDevice, guid)
>>>>              || IsEqualGUID(&IID_IDirect3DHALDevice, guid);
>>>>
>>>> +    if (device->hardware_device && !(target->surface_desc.ddsCaps.dwCaps & DDSCAPS_VIDEOMEMORY))
>>>> +    {
>>>> +        WARN("Surface %p is not in video memory.\n", target);
>>>> +        return D3DERR_SURFACENOTINVIDMEM;
>>>> +    }
>>>> +
>>>>      if (outer_unknown)
>>>>          device->outer_unknown = outer_unknown;
>>>>      else
>>>> @@ -7038,12 +7044,6 @@ HRESULT d3d_device_create(struct ddraw *ddraw, const GUID *guid, struct ddraw_su
>>>>          return DDERR_OUTOFMEMORY;
>>>>      }
>>>>
>>>> -    if (!(target->surface_desc.ddsCaps.dwCaps & DDSCAPS_VIDEOMEMORY))
>>>> -    {
>>>> -        WARN("Surface %p is not in video memory.\n", target);
>>>> -        return D3DERR_SURFACENOTINVIDMEM;
>>>> -    }
>>>> -
>>> Was there any particular reason to move this? It's fine if there's a
>>> reason, but on first sight it seems somewhat arbitrary.
>> Checking the guids seemed a bit more reasonable to me in
>> d3d_device_init() which fills all the fields of the struct d3d_device,
>> including hardware_device, so I moved the check after the estimation of
>> that field. Otherwise, I could check guids in d3d_device_create() and
>> pass hardware_device BOOL to d3d_device_init. Should I change that?
>>
> Oh, I see, we don't have device->hardware_device yet in
> d3d_device_create(). That's fine, no need to change it.
>
>>>> +static void ddraw_surface_create_draw_texture(struct ddraw_surface *surface)
>>>> +{
>>>> +    DDSURFACEDESC2 *desc = &surface->surface_desc;
>>>> +    struct wined3d_resource *draw_texture_resource;
>>>> +    struct wined3d_resource_desc wined3d_desc;
>>>> +    unsigned int i, layer_count, level_count;
>>>> +    struct wined3d_texture *draw_texture;
>>>> +    struct ddraw_surface *parent;
>>>> +    unsigned int bind_flags;
>>>> +    HRESULT hr;
>>>> +
>>>> +    if (!(desc->ddsCaps.dwCaps & DDSCAPS_SYSTEMMEMORY))
>>>> +        return;
>>>> +
>>> Shouldn't we also check whether we have a hardware device or not here?
>> Do you mean checking if we have a hardware renderer at all and skip the
>> attempt to create draw_texture if we don't? But wouldn't it be easier to
>> just let it fail without adding an extra check?
> No, the other way around, there's no need to create the extra texture
> if we do have a hardware device. Granted, that only makes a difference
> if applications create system memory surfaces with the relevant caps
> on a hardware device, but well, ddraw applications.

But the application device might not exist yet at the moment it creates
the surface, so it is not clear yet if it will be software or hardware
device. Then, if the application will create the sysmem render target
and will try to attach it to hardware device, it will fail anyway. Then,
while we currently support just one ddraw device as I understand there
is no such limitation on Windows and looks a bit undesirable to me to
depend on the present limitation. Or am I missing something?