June 2021 - Wine-Devel - list.winehq.org

Re: [PATCH vkd3d 1/7] vkd3d-shader: Introduce shader_sm4_read_descriptor_register_range().

by Henri Verbeet

On Wed, 30 Jun 2021 at 05:05, Conor McCarthy <cmccarthy(a)codeweavers.com> wrote: > +static void shader_sm4_read_descriptor_register_range(struct vkd3d_sm4_data *priv, const DWORD **ptr, > + const DWORD *end, struct vkd3d_shader_register *reg, struct vkd3d_shader_register_range *range) > +{ > + shader_sm4_read_register_space(priv, ptr, end, &range->space); > + range->first = reg->idx[shader_is_sm_5_1(priv) ? 1 : 0].offset; > + range->last = reg->idx[shader_is_sm_5_1(priv) ? 2 : 0].offset; > + if (range->last < range->first) > + FIXME("Invalid register range [%u:%u].\n", range->first, range->last); > +} > + That function is not reading the register range though; it first reads the register space, and then subsequently resolves the range from the previously read indices. I'd suggest to keep those separate.

June 30, 2021

Re: [PATCH v3] dinput: Introduce new HID joystick backend.

by Rémi Bernon

On 6/30/21 2:30 PM, Rémi Bernon wrote: > On 6/30/21 1:08 PM, Arkadiusz Hiler wrote: >> On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote: >>> + >>> +static HRESULT hid_joystick_enum_device( DWORD type, DWORD flags, >>> DIDEVICEINSTANCEW *instance, >>> + DWORD version, int index ) >>> +{ >>> + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; >>> + PHIDP_PREPARSED_DATA preparsed; >>> + WCHAR device_path[MAX_PATH]; >>> + HIDP_CAPS caps; >>> + HANDLE device; >>> + HRESULT hr; >>> + >>> + TRACE( "type %x, flags %#x, instance %p, version %04x, index >>> %d\n", type, flags, instance, version, index ); >>> + >>> + hr = hid_joystick_device_open( index, 0, 0, device_path, >>> &device, version, >>> + &preparsed, &attrs, &caps, >>> instance ); >> >> hid_joystick_device_open() is getting a bit unwieldy with all the >> possible parameter combinations. I don't think it needs the enumeration >> / vidpid lookup logic. >> >> IMO it would read better if hid_joystick_device_open() would take the >> path or RI handle and enum logic would be moved either here or to >> a helper. >> > > I don't think we need yet another device list cache, and I would rather > avoid it if possible. It's really just two call sites. > > Instead of the two additional params we could use the DIDEVICEINSTANCEW > parameter as an in/out filter, providing the desired GUIDs to match (or > zeroed when enumerating all devices). > >>> + if (hr != DI_OK) return hr; >>> + >>> + HidD_FreePreparsedData( preparsed ); >>> + CloseHandle( device ); >>> + >>> + if (instance->dwSize != sizeof(DIDEVICEINSTANCEW)) >>> + return S_FALSE; >>> + if (version < 0x0800 && type != DIDEVTYPE_JOYSTICK) >>> + return S_FALSE; >>> + if (version >= 0x0800 && type != DI8DEVCLASS_ALL && type != >>> DI8DEVCLASS_GAMECTRL) >>> + return S_FALSE; >>> + >>> + if (device_disabled_registry( "HID", TRUE )) >>> + return DIERR_DEVICENOTREG; >>> + >>> + TRACE( "Found device %s, usage %04x:%04x, product %s, instance >>> %s, name %s\n", debugstr_w(device_path), >>> + instance->wUsagePage, instance->wUsage, debugstr_guid( >>> &instance->guidProduct ), >>> + debugstr_guid( &instance->guidInstance ), >>> debugstr_w(instance->tszInstanceName) ); >>> + >>> + return DI_OK; >>> +} >>> + >>> +static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, >>> REFGUID guid, IDirectInputDevice8W **out ) >>> +{ >>> + DIDEVICEINSTANCEW instance = {sizeof(instance)}; >>> + GUID inst_guid = *guid, prod_guid = *guid; >>> + DWORD size = sizeof(struct hid_joystick); >>> + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; >>> + struct hid_joystick *impl = NULL; >>> + PHIDP_PREPARSED_DATA preparsed; >>> + UINT32 handle = 0, vid_pid = 0; >>> + DIDATAFORMAT *format = NULL; >>> + WCHAR device_path[MAX_PATH]; >>> + HIDP_CAPS caps; >>> + HANDLE device; >>> + HRESULT hr; >>> + >>> + TRACE( "dinput %p, guid %s, out %p\n", dinput, debugstr_guid( >>> guid ), out ); >>> + >>> + *out = NULL; >>> + >>> + inst_guid.Data3 = hid_joystick_guid.Data3; >>> + prod_guid.Data1 = DInput_PIDVID_Product_GUID.Data1; >>> + if (IsEqualGUID( &hid_joystick_guid, &inst_guid )) handle = >>> guid->Data3; >> >> Data3 is only a short. I think we should use 64 bits for our magic and >> another 64 bits for the RawInput handle. >> >> Or 32 if we go with our internal knowledge about how those handles are >> implemented in Wine. >> > > Should we really worry about overflowing the short capacity for the HID > handle? If we do we could combine Data2 and Data3, or XOR it in Data1 > (just to keep a bit of randomness), but as the property is also checked > to be 32bit I don't think we need to use more than that. For instance I think something along those lines would actually be nice, with the "instance" filled in try_open, and then returned into "filter" as output when device matches: > > /* enumerate device by GUID */ > if (IsEqualGUID( &filter->guidProduct, &instance.guidProduct )) break; > if (IsEqualGUID( &filter->guidInstance, &instance.guidInstance )) break; > > /* enumerate all devices */ > if (IsEqualGUID( &filter->guidProduct, &DInput_PIDVID_Product_GUID ) && > IsEqualGUID( &filter->guidInstance, &hid_joystick_guid ) && > !index--) > break; > And the two call sites would look like: > > instance->guidProduct = DInput_PIDVID_Product_GUID; > instance->guidInstance = hid_joystick_guid; > hr = hid_joystick_device_open( index, instance, device_path, &device, &preparsed, > &attrs, &caps, version ); > if (hr != DI_OK) return hr; > DIDEVICEINSTANCEW instance = {.dwSize = sizeof(instance), .guidProduct = *guid, .guidInstance = *guid}; > instance.guidProduct.Data1 = DInput_PIDVID_Product_GUID.Data1; > instance.guidInstance.Data1 = hid_joystick_guid.Data1; > if (IsEqualGUID( &DInput_PIDVID_Product_GUID, &instance.guidProduct )) > { > instance.guidProduct = *guid; > instance.guidInstance = hid_joystick_guid; > } > else if (IsEqualGUID( &hid_joystick_guid, &instance.guidInstance )) > { > instance.guidProduct = DInput_PIDVID_Product_GUID; > instance.guidInstance = *guid; > } > else > { > return DIERR_DEVICENOTREG; > } > > hr = hid_joystick_device_open( 0, &instance, device_path, &device, &preparsed, > &attrs, &caps, dinput->dwVersion ); > if (hr != DI_OK) return hr; It also fulfill the vid_pid matching nicely. -- Rémi Bernon <rbernon(a)codeweavers.com>

June 30, 2021

[PATCH] ntdll: Don't pretend that the whole address space is reserved on non-i386.

by Roman Pišl

This is analogous to 61dcca5dbf4d0021f420dc87248b39a1dc1542db. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50735 Signed-off-by: Roman Pišl <rpisl(a)seznam.cz> --- dlls/ntdll/unix/virtual.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index ef623c0b17a..2484450055d 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -4012,10 +4012,18 @@ static int CDECL get_free_mem_state_callback( void *start, SIZE_T size, void *ar else /* outside of the reserved area, pretend it's allocated */ { info->RegionSize = (char *)start - (char *)info->BaseAddress; +#ifdef __i386__ info->State = MEM_RESERVE; info->Protect = PAGE_NOACCESS; info->AllocationProtect = PAGE_NOACCESS; info->Type = MEM_PRIVATE; +#else + info->State = MEM_FREE; + info->Protect = PAGE_NOACCESS; + info->AllocationBase = 0; + info->AllocationProtect = 0; + info->Type = 0; +#endif } return 1; } -- 2.30.2

June 30, 2021

Re: [PATCH vkd3d 1/5] vkd3d-shader: Allocate the $Params buffer in hlsl_ctx_init().

by Matteo Bruni

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>

June 30, 2021

Re: [PATCH vkd3d 2/5] vkd3d-shader: Track bytecode buffer size in bytes.

by Matteo Bruni

On Tue, Jun 29, 2021 at 8:00 PM Zebediah Figura <zfigura(a)codeweavers.com> wrote: > > And change the way we handle alignment. > > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > libs/vkd3d-shader/hlsl.h | 4 +- > libs/vkd3d-shader/hlsl_codegen.c | 92 +++++++++++++------------------- > 2 files changed, 39 insertions(+), 57 deletions(-) > > diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c > index 9afa590a..27bbd9b6 100644 > --- a/libs/vkd3d-shader/hlsl_codegen.c > +++ b/libs/vkd3d-shader/hlsl_codegen.c > @@ -1631,20 +1612,21 @@ static void write_sm1_uniforms(struct hlsl_ctx *ctx, struct bytecode_buffer *buf > { > if (!var->semantic.name && var->reg.allocated) > { > - set_dword(buffer, vars_start + (uniform_count * 5), (buffer->count - ctab_start) * sizeof(*buffer->data)); > + size_t var_offset = vars_start + (uniform_count * 5 * sizeof(uint32_t)); > + > + set_dword(buffer, var_offset, buffer->size - ctab_start); > put_string(buffer, var->name); > > write_sm1_type(buffer, var->data_type, ctab_start); > - set_dword(buffer, vars_start + (uniform_count * 5) + 3, > - (var->data_type->bytecode_offset - ctab_start) * sizeof(*buffer->data)); > + set_dword(buffer, var_offset + 3 * sizeof(uint32_t), var->data_type->bytecode_offset - ctab_start); > ++uniform_count; > } > } > > offset = put_string(buffer, vkd3d_shader_get_version(NULL, NULL)); > - set_dword(buffer, creator_offset, (offset - ctab_start) * sizeof(*buffer->data)); > + set_dword(buffer, creator_offset, offset - ctab_start); > > - set_dword(buffer, size_offset, D3DSIO_COMMENT | ((buffer->count - (ctab_start - 1)) << 16)); > + set_dword(buffer, size_offset, D3DSIO_COMMENT | (((buffer->size - (ctab_start - 1)) / sizeof(uint32_t)) << 16)); I think this is broken, or at least changed from previous output, for two different reasons: buffer->size is now not aligned (since alignment happens on the following put_bytes()) and we want to subtract ctab_start - 1 * sizeof(DWORD) (or, equivalently, strlen("CTAB")) from the current offset. I suggest to replace all the raw uses of buffer->size with a helper call e.g. size_t get_current_offset_aligned(buffer, alignment) to make sure we're not falling into this trap.

June 30, 2021

Re: [PATCH v3] dinput: Introduce new HID joystick backend.

by Rémi Bernon

On 6/30/21 1:08 PM, Arkadiusz Hiler wrote: > On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote: >> This adds a new joystick backend, implemented on top of HID and without >> any host dependencies. This will be progressively implementated, and >> it's not going to be usable until at least a few more patches. >> >> Because of that, and because it may also introduce regressions compared >> to the existing backends, it is disabled by default and is optionally >> enabled using the following global registry key: >> >> [HKCU\\Software\\Wine\\DirectInput\\Joysticks] >> "HID"="enabled" >> >> Or using the corresponding AppDefaults registry key: >> >> [HKCU\\Software\\Wine\\AppDefaults\\<app.exe>\\DirectInput\\Joysticks] >> "HID"="enabled" >> >> This setting will be removed later, when it becomes usable enough, to >> use the individual device disable mechanism available in joy.cpl. >> >> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> >> --- >> >> v3: Use setupapi directly, as user32 api usage was confusing. This also >> now matches device on either their index, HID handle or vid_pid, >> depending on what's been provided. >> >> The instance GUID now includes the HID handle, which will be more >> stable than the device index in setupapi device list. >> >> Matching the vid_pid assumes that setupapi device path is upper >> case, which I think is the case. We could make sure otherwise but >> I don't think we have to and it's not very convenient without wcsupr. > > Yes, setupapi seems to be always uppercasing thing. > >> +static HRESULT hid_joystick_device_open( DWORD index, UINT32 handle, UINT32 vid_pid, WCHAR *device_path, >> + HANDLE *device, DWORD version, PHIDP_PREPARSED_DATA *preparsed, >> + HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, DIDEVICEINSTANCEW *instance ) >> +{ >> + static const WCHAR vid_pid_fmt_w[] = {'V','I','D','_','%','0','4','X','&','P','I','D','_','%','0','4','X',0}; >> + char buffer[sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W) + MAX_PATH * sizeof(WCHAR)]; >> + SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail = (void *)buffer; >> + SP_DEVICE_INTERFACE_DATA iface = {sizeof(iface)}; >> + SP_DEVINFO_DATA devinfo = {sizeof(devinfo)}; >> + WCHAR vid_pid_w[18] = {0}; >> + UINT32 i = 0, hid_handle; >> + HDEVINFO set; >> + DWORD type; >> + GUID hid; >> + >> + TRACE( "index %u, handle %u, vid_pid %d\n", index, handle, vid_pid ); >> + >> + HidD_GetHidGuid( &hid ); >> + snprintfW( vid_pid_w, ARRAY_SIZE(vid_pid_w), vid_pid_fmt_w, vid_pid & 0xffff, vid_pid >> 16 ); >> + >> + set = SetupDiGetClassDevsW( &hid, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT ); >> + if (set == INVALID_HANDLE_VALUE) return DIERR_DEVICENOTREG; >> + >> + *device = NULL; >> + *preparsed = NULL; >> + while (SetupDiEnumDeviceInterfaces( set, NULL, &hid, i++, &iface )) >> + { >> + detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W); >> + if (!SetupDiGetDeviceInterfaceDetailW( set, &iface, detail, sizeof(buffer), NULL, &devinfo )) >> + continue; > > This should never fail because MAX_PATH, so how about a FIXME / WARN in > addition to that continue? > Well I'm not sure if it's even worth it if it's not even supposed to fail. I also considered adding TRACEs to the enumeration, to be able to more easily see which devices were skipped, but this quickly becomes verbose. >> + if (!SetupDiGetDevicePropertyW( set, &devinfo, &DEVPROPKEY_HID_HANDLE, &type, >> + (BYTE *)&hid_handle, sizeof(hid_handle), NULL, 0 ) || >> + type != DEVPROP_TYPE_UINT32) >> + continue; > > I was thinking about sticking to RawInput enumeration and using the > handles that you get from there instead of the indexes. This way you > would need to do GetRawInputDeviceInfo() only in > hid_joystick_enum_device() or maybe another helper that would try to map > vidpid from product GUID to a handle. > > With that you would be able to make hid_joystick_device_open() take the > RawInput handle as the only parameter identifying the device. > > This would also work on Windows (after the PE conversion), but I don't > think it's our goal, so I believe that setupapi may be a better choice > here. > Yeah it's actually inconvenient that it doesn't work anymore on Windows (compared to the previous version), I was trying the PE-converted dinput on Windows too and only realized late that it doesn't work anymore. It's probably not too much of an issue, but if we're interested in making it work there we could perhaps use "i" as a hid_handle fallback? Assuming that all HID devices will have a handle on Wine it should not have any impact there. I don't have any better idea right now, and using setupapi is actually an improvement IMHO. In any case it can probably be done as a separate later change, after the conversion, as there's the critical section debug field usage which is also incompatible. >> + if (!hid_joystick_device_try_open( detail->DevicePath, device, preparsed, attrs, caps, instance )) >> + continue; >> + >> + if (!handle && !vid_pid && !index--) break; >> + if (handle && handle == hid_handle) break; >> + if (vid_pid && strstrW( detail->DevicePath, vid_pid_w )) break; > > You have the device handle here, so why not just use HidD_GetAttibutes()? > > Another option would be to use GetRawInputDeviceInfo() with the > hid_handle. > Indeed, much better idea to use the attrs, I moved the code around a few times after adding the vid_pid match and didn't reconsider that logic. >> + CloseHandle( *device ); >> + HidD_FreePreparsedData( *preparsed ); >> + *device = NULL; >> + *preparsed = NULL; >> + } >> + >> + SetupDiDestroyDeviceInfoList( set ); >> + if (!*device || !*preparsed) return DIERR_DEVICENOTREG; >> + >> + lstrcpynW( device_path, detail->DevicePath, MAX_PATH ); >> + instance->guidInstance = hid_joystick_guid; >> + instance->guidInstance.Data3 = hid_handle; >> + instance->guidProduct = DInput_PIDVID_Product_GUID; >> + instance->guidProduct.Data1 = MAKELONG( attrs->VendorID, attrs->ProductID ); >> + instance->dwDevType = get_device_type( version, caps->Usage != HID_USAGE_GENERIC_GAMEPAD ) | DIDEVTYPE_HID; >> + instance->guidFFDriver = GUID_NULL; >> + instance->wUsagePage = caps->UsagePage; >> + instance->wUsage = caps->Usage; >> + return DI_OK; >> +} >> + >> +static HRESULT hid_joystick_enum_device( DWORD type, DWORD flags, DIDEVICEINSTANCEW *instance, >> + DWORD version, int index ) >> +{ >> + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; >> + PHIDP_PREPARSED_DATA preparsed; >> + WCHAR device_path[MAX_PATH]; >> + HIDP_CAPS caps; >> + HANDLE device; >> + HRESULT hr; >> + >> + TRACE( "type %x, flags %#x, instance %p, version %04x, index %d\n", type, flags, instance, version, index ); >> + >> + hr = hid_joystick_device_open( index, 0, 0, device_path, &device, version, >> + &preparsed, &attrs, &caps, instance ); > > hid_joystick_device_open() is getting a bit unwieldy with all the > possible parameter combinations. I don't think it needs the enumeration > / vidpid lookup logic. > > IMO it would read better if hid_joystick_device_open() would take the > path or RI handle and enum logic would be moved either here or to > a helper. > I don't think we need yet another device list cache, and I would rather avoid it if possible. It's really just two call sites. Instead of the two additional params we could use the DIDEVICEINSTANCEW parameter as an in/out filter, providing the desired GUIDs to match (or zeroed when enumerating all devices). >> + if (hr != DI_OK) return hr; >> + >> + HidD_FreePreparsedData( preparsed ); >> + CloseHandle( device ); >> + >> + if (instance->dwSize != sizeof(DIDEVICEINSTANCEW)) >> + return S_FALSE; >> + if (version < 0x0800 && type != DIDEVTYPE_JOYSTICK) >> + return S_FALSE; >> + if (version >= 0x0800 && type != DI8DEVCLASS_ALL && type != DI8DEVCLASS_GAMECTRL) >> + return S_FALSE; >> + >> + if (device_disabled_registry( "HID", TRUE )) >> + return DIERR_DEVICENOTREG; >> + >> + TRACE( "Found device %s, usage %04x:%04x, product %s, instance %s, name %s\n", debugstr_w(device_path), >> + instance->wUsagePage, instance->wUsage, debugstr_guid( &instance->guidProduct ), >> + debugstr_guid( &instance->guidInstance ), debugstr_w(instance->tszInstanceName) ); >> + >> + return DI_OK; >> +} >> + >> +static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, REFGUID guid, IDirectInputDevice8W **out ) >> +{ >> + DIDEVICEINSTANCEW instance = {sizeof(instance)}; >> + GUID inst_guid = *guid, prod_guid = *guid; >> + DWORD size = sizeof(struct hid_joystick); >> + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; >> + struct hid_joystick *impl = NULL; >> + PHIDP_PREPARSED_DATA preparsed; >> + UINT32 handle = 0, vid_pid = 0; >> + DIDATAFORMAT *format = NULL; >> + WCHAR device_path[MAX_PATH]; >> + HIDP_CAPS caps; >> + HANDLE device; >> + HRESULT hr; >> + >> + TRACE( "dinput %p, guid %s, out %p\n", dinput, debugstr_guid( guid ), out ); >> + >> + *out = NULL; >> + >> + inst_guid.Data3 = hid_joystick_guid.Data3; >> + prod_guid.Data1 = DInput_PIDVID_Product_GUID.Data1; >> + if (IsEqualGUID( &hid_joystick_guid, &inst_guid )) handle = guid->Data3; > > Data3 is only a short. I think we should use 64 bits for our magic and > another 64 bits for the RawInput handle. > > Or 32 if we go with our internal knowledge about how those handles are > implemented in Wine. > Should we really worry about overflowing the short capacity for the HID handle? If we do we could combine Data2 and Data3, or XOR it in Data1 (just to keep a bit of randomness), but as the property is also checked to be 32bit I don't think we need to use more than that. -- Rémi Bernon <rbernon(a)codeweavers.com>

June 30, 2021

Re: [PATCH v2 5/5] wined3d: Implement NOOVERWITE maps in wined3d_deferred_context_prepare_upload_bo().

by Henri Verbeet

On Wed, 30 Jun 2021 at 06:34, Zebediah Figura <z.figura12(a)gmail.com> wrote: > @@ -3452,12 +3452,31 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co > return NULL; > } > > - if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD)) > + if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)) > { > FIXME("Unhandled flags %#x.\n", flags); > return NULL; > } > > + if (flags & WINED3D_MAP_NOOVERWRITE) > + { > + int i = deferred->upload_count; > + > + while (i--) > + { > + struct wined3d_deferred_upload *upload = &deferred->uploads[i]; > + > + if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx) > + { > + address->buffer_object = 0; > + address->addr = upload->sysmem; > + return upload->sysmem; > + } > + } > + > + return NULL; > + } > + I.e., wined3d_deferred_context_get_upload_bo(), right?

June 30, 2021

Re: [PATCH v2 4/5] wined3d: Use context->ops->prepare_upload_bo() in wined3d_device_context_map() if possible.

by Henri Verbeet

On Wed, 30 Jun 2021 at 06:34, Zebediah Figura <z.figura12(a)gmail.com> wrote: > +static bool wined3d_deferred_context_get_upload_bo(struct wined3d_device_context *context, > + struct wined3d_resource *resource, unsigned int sub_resource_idx, > + struct wined3d_box *box, struct wined3d_const_bo_address *address) > +{ > + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); > + int i = deferred->upload_count; > + > + while (i--) > + { > + struct wined3d_deferred_upload *upload = &deferred->uploads[i]; > + > + if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx) > + { > + *box = upload->box; > + address->buffer_object = 0; > + address->addr = upload->sysmem; > + return true; > + } > + } > + > + return false; > +} > + "SIZE_T i;", or at least "unsigned int i;" > HRESULT CDECL wined3d_device_context_unmap(struct wined3d_device_context *context, > struct wined3d_resource *resource, unsigned int sub_resource_idx) > { > + struct wined3d_const_bo_address addr; > + struct wined3d_box box; > + > TRACE("context %p, resource %p, sub_resource_idx %u.\n", context, resource, sub_resource_idx); > > - return context->ops->unmap(context, resource, sub_resource_idx); > + if (context->ops->get_upload_bo(context, resource, sub_resource_idx, &box, &addr)) > + { > + unsigned int row_pitch, slice_pitch; > + > + wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx, &row_pitch, &slice_pitch); > + wined3d_device_context_upload_bo(context, resource, sub_resource_idx, &box, &addr, row_pitch, slice_pitch); > + return WINED3D_OK; > + } > + else > + { > + return context->ops->unmap(context, resource, sub_resource_idx); > + } > } > Would we ever use ops->get_upload_bo() and ops->unmap() independently of each other? If not, we may as well merge those together.

June 30, 2021

Re: [PATCH v2 2/5] wined3d: Implement wined3d_deferred_context_prepare_upload_bo().

by Henri Verbeet

On Wed, 30 Jun 2021 at 06:33, Zebediah Figura <z.figura12(a)gmail.com> wrote: > @@ -3401,9 +3418,44 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co > struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, > unsigned int row_pitch, unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address) > { > - FIXME("context %p, resource %p, sub_resource_idx %u, box %p, flags %#x, address %p, stub!\n", > - context, resource, sub_resource_idx, box, flags, address); > - return false; > + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); > + const struct wined3d_format *format = resource->format; > + struct wined3d_deferred_upload *upload; > + void *sysmem; > + size_t size; > + > + size = (box->back - box->front - 1) * slice_pitch > + + ((box->bottom - box->top - 1) / format->block_height) * row_pitch > + + ((box->right - box->left + format->block_width - 1) / format->block_width) * format->block_byte_count; > + > + if (!(flags & WINED3D_MAP_WRITE)) > + { > + WARN("Flags %#x are not valid on a deferred context.\n", flags); > + return NULL; > + } > + > + if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD)) > + { > + FIXME("Unhandled flags %#x.\n", flags); > + return NULL; > + } > + > + if (!wined3d_array_reserve((void **)&deferred->uploads, &deferred->uploads_capacity, > + deferred->upload_count + 1, sizeof(*deferred->uploads))) > + return NULL; > + > + if (!(sysmem = wined3d_allocate_sysmem(size))) > + return NULL; > + > + upload = &deferred->uploads[deferred->upload_count++]; > + upload->resource = resource; > + wined3d_resource_incref(resource); > + upload->sub_resource_idx = sub_resource_idx; > + upload->sysmem = sysmem; > + > + address->buffer_object = 0; > + address->addr = sysmem; > + return sysmem; > } > Do we need to use wined3d_allocate_sysmem() here? We could also do something like the following: upload->sysmem = heap_alloc(size + alignment - 1); [...] return address->addr = upload->sysmem & ~(alignment - 1) (And note that not all callers of wined3d_deferred_context_prepare_upload_bo() will necessarily need an allocation aligned to RESOURCE_ALIGNMENT.) > -static BOOL wined3d_resource_allocate_sysmem(struct wined3d_resource *resource) > +void *wined3d_allocate_sysmem(SIZE_T size) > { > void **p; > - SIZE_T align = RESOURCE_ALIGNMENT - 1 + sizeof(*p); > + static const SIZE_T align = RESOURCE_ALIGNMENT - 1 + sizeof(*p); > void *mem; > > - if (!(mem = heap_alloc_zero(resource->size + align))) > + if (!(mem = heap_alloc_zero(size + align))) > { > ERR("Failed to allocate system memory.\n"); > - return FALSE; > + return NULL; > } > > + TRACE("Allocated %lu bytes at %p.\n", size, mem); > p = (void **)(((ULONG_PTR)mem + align) & ~(RESOURCE_ALIGNMENT - 1)) - 1; > *p = mem; > > - resource->heap_memory = ++p; > + return ++p; > +} > If we're turning this into a generic helper, perhaps it would be more appropriate in utils.c, and we may want to consider naming it something like e.g. wined3d_allocate_aligned().

June 30, 2021

Re: [PATCH v2 1/5] wined3d: Introduce a prepare_upload_bo device context operation and use it to upload sub-resource data.

by Henri Verbeet

On Wed, 30 Jun 2021 at 06:33, Zebediah Figura <z.figura12(a)gmail.com> wrote: > +static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_context *context, > + struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, > + unsigned int row_pitch, unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address) > +{ > + FIXME("context %p, resource %p, sub_resource_idx %u, box %p, flags %#x, address %p, stub!\n", > + context, resource, sub_resource_idx, box, flags, address); > + return false; > +} > + That works, but should probably be "return NULL;" instead.

June 30, 2021