[PATCH v4 0/2] MR6069: quartz: Properly copy data to render surfaces of planar formats in VMR9.
Fixes Bug 23029 (Devil May Cry® 3 Special Edition) (6550) Intro Video is covered by green, transparent square for a majority of it. Before these patches, surface data for planar formats is not copied correctly when the application uses a custom allocator-presenter and allocates a surface of different size than the VMR9 source. Patch 2/2 adds support for performing this copy correctly when the source dimensions are less or equal than the rendering surface dimensions. --- I have some questions: - Should I also make it work for when the dst is smaller than the src? - If not, should the FIXME() in 1/2 be promoted to an error? Otherwise we might have segfaults, writing out of scope. - Should I use copy_plane(), introduced in 2/2 also in the implementation of the other formats? Changing what needs to be changed to preserve behavior of course. -- v4: quartz: Properly copy data to render surfaces of planar formats in VMR9. https://gitlab.winehq.org/wine/wine/-/merge_requests/6069
From: Francisco Casas <fcasas(a)codeweavers.com> --- dlls/quartz/vmr9.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index 9d95cda9def..270277f0196 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -189,6 +189,7 @@ static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample) REFERENCE_TIME start_time, end_time; VMR9PresentationInfo info = {}; D3DLOCKED_RECT locked_rect; + D3DSURFACE_DESC dst_desc; BYTE *data = NULL; HRESULT hr; int height; @@ -240,6 +241,16 @@ static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample) info.szAspectRatio.cy = height; info.lpSurf = filter->surfaces[(++filter->cur_surface) % filter->num_surfaces]; + if (FAILED(hr = IDirect3DSurface9_GetDesc(info.lpSurf, &dst_desc))) + { + ERR("Failed to get rendering surface description.\n"); + return hr; + } + + if (width > dst_desc.Width || abs(height) > dst_desc.Height) + FIXME("src surface (%ux%u) larger than rendering surface (%ux%u).\n", width, height, + dst_desc.Width, dst_desc.Height); + if (FAILED(hr = IDirect3DSurface9_LockRect(info.lpSurf, &locked_rect, NULL, D3DLOCK_DISCARD))) { ERR("Failed to lock surface, hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6069
From: Francisco Casas <fcasas(a)codeweavers.com> Before this patch, surface data for planar formats is not copied correctly when the application uses a custom allocator-presenter and allocates a surface of different size than the VMR9 source. This patch adds support for performing this copy correctly when the source dimensions are less or equal than the rendering surface dimensions. --- dlls/quartz/vmr9.c | 74 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 17 deletions(-) diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index 270277f0196..ef8c78951c8 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -181,6 +181,42 @@ static inline struct quartz_vmr *impl_from_IBaseFilter(IBaseFilter *iface) return CONTAINING_RECORD(iface, struct quartz_vmr, renderer.filter.IBaseFilter_iface); } +static void copy_plane(BYTE **dstp, unsigned int dst_pitch, unsigned int dst_height, + const BYTE **srcp, unsigned int src_pitch, int src_height) +{ + size_t copy_size = min(src_pitch, dst_pitch); + const BYTE *src = *srcp; + BYTE *dst = *dstp; + unsigned int i; + + if (src_height < 0) + { + TRACE("Inverting image.\n"); + + src_height = -src_height; + src += src_height * src_pitch; + + for (i = 0; i < src_height; ++i) + { + src -= src_pitch; + memcpy(dst, src, copy_size); + dst += dst_pitch; + } + } + else + { + for (i = 0; i < src_height; ++i) + { + memcpy(dst, src, copy_size); + dst += dst_pitch; + src += src_pitch; + } + } + + *srcp += src_pitch * src_height; + *dstp += dst_pitch * dst_height; +} + static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample) { struct quartz_vmr *filter = impl_from_IBaseFilter(&iface->filter.IBaseFilter_iface); @@ -257,36 +293,40 @@ static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample) return hr; } - if (height > 0 && bitmap_header->biCompression == BI_RGB) + if (bitmap_header->biCompression == mmioFOURCC('N','V','1','2')) { - BYTE *dst = (BYTE *)locked_rect.pBits + (height * locked_rect.Pitch); + BYTE *dst = locked_rect.pBits; const BYTE *src = data; - TRACE("Inverting image.\n"); + copy_plane(&dst, locked_rect.Pitch, dst_desc.Height, &src, src_pitch, height); + copy_plane(&dst, locked_rect.Pitch, dst_desc.Height / 2, &src, src_pitch, height / 2); + } + else if (bitmap_header->biCompression == mmioFOURCC('Y','V','1','2')) + { + BYTE *dst = locked_rect.pBits; + const BYTE *src = data; - while (height--) - { - dst -= locked_rect.Pitch; - memcpy(dst, src, width * depth / 8); - src += src_pitch; - } + copy_plane(&dst, locked_rect.Pitch, dst_desc.Height, &src, src_pitch, height); + copy_plane(&dst, locked_rect.Pitch / 2, dst_desc.Height / 2, &src, src_pitch / 2, height / 2); + copy_plane(&dst, locked_rect.Pitch / 2, dst_desc.Height / 2, &src, src_pitch / 2, height / 2); } - else if (locked_rect.Pitch != src_pitch) + else if (height > 0 && bitmap_header->biCompression == BI_RGB) { BYTE *dst = locked_rect.pBits; const BYTE *src = data; - height = abs(height); + copy_plane(&dst, locked_rect.Pitch, dst_desc.Height, &src, src_pitch, -height); + } + else if (locked_rect.Pitch != src_pitch) + { + BYTE *dst = locked_rect.pBits; + const BYTE *src = data; TRACE("Source pitch %u does not match dest pitch %u; copying manually.\n", src_pitch, locked_rect.Pitch); - while (height--) - { - memcpy(dst, src, width * depth / 8); - src += src_pitch; - dst += locked_rect.Pitch; - } + height = abs(height); + copy_plane(&dst, locked_rect.Pitch, dst_desc.Height, &src, src_pitch, height); } else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6069
On Mon Jul 15 23:00:18 2024 +0000, Elizabeth Figura wrote:
That's not important, though. The extra data that's copied past the pitch is alignment padding and will eventually be discarded anyway. Okay, I updated the MR to use copy_plane() except for the fast path as suggested.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6069#note_76486
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6069
participants (3)
-
Elizabeth Figura (＠zfigura) -
Francisco Casas
-
Francisco Casas (＠fcasas)