[PATCH 0/1] MR289: d3drm: Implement IDirect3DRMFrame2::SetSceneBackgroundImage

[Alistair Leslie-Hughes (＠alesliehughes)]

Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/289

[PATCH 1/1] d3drm: Implement IDirect3DRMFrame2::SetSceneBackgroundImage

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>

Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
---
 dlls/d3drm/d3drm_private.h |  1 +
 dlls/d3drm/frame.c         | 23 +++++++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/dlls/d3drm/d3drm_private.h b/dlls/d3drm/d3drm_private.h
index 2fb6bafe951..96a9c3e7f60 100644
--- a/dlls/d3drm/d3drm_private.h
+++ b/dlls/d3drm/d3drm_private.h
@@ -81,6 +81,7 @@ struct d3drm_frame
     IDirect3DRMFrame3 IDirect3DRMFrame3_iface;
     IDirect3DRM *d3drm;
     LONG ref;
+    IDirect3DRMTexture3 *backgroundimage;
     struct d3drm_frame *parent;
     SIZE_T nb_children;
     SIZE_T children_size;
diff --git a/dlls/d3drm/frame.c b/dlls/d3drm/frame.c
index 5cc3ad585d4..d1e8d6d55b2 100644
--- a/dlls/d3drm/frame.c
+++ b/dlls/d3drm/frame.c
@@ -2222,25 +2222,32 @@ static HRESULT WINAPI d3drm_frame1_SetSceneBackgroundDepth(IDirect3DRMFrame *ifa
 static HRESULT WINAPI d3drm_frame3_SetSceneBackgroundImage(IDirect3DRMFrame3 *iface,
         IDirect3DRMTexture3 *texture)
 {
-    FIXME("iface %p, texture %p stub!\n", iface, texture);
+    struct d3drm_frame *frame = impl_from_IDirect3DRMFrame3(iface);
+    TRACE("iface %p, texture %p\n", iface, texture);
 
-    return E_NOTIMPL;
+    if (frame->backgroundimage)
+        IDirect3DRMTexture3_Release(frame->backgroundimage);
+    frame->backgroundimage = texture;
+    if (frame->backgroundimage)
+        IDirect3DRMTexture3_AddRef(frame->backgroundimage);
+
+    return S_OK;
 }
 
 static HRESULT WINAPI d3drm_frame2_SetSceneBackgroundImage(IDirect3DRMFrame2 *iface,
         IDirect3DRMTexture *texture)
 {
-    FIXME("iface %p, texture %p stub!\n", iface, texture);
-
-    return E_NOTIMPL;
+    struct d3drm_frame *frame = impl_from_IDirect3DRMFrame2(iface);
+    TRACE("iface %p, texture %p\n", iface, texture);
+    return d3drm_frame3_SetSceneBackgroundImage(&frame->IDirect3DRMFrame3_iface, (IDirect3DRMTexture3*)texture);
 }
 
 static HRESULT WINAPI d3drm_frame1_SetSceneBackgroundImage(IDirect3DRMFrame *iface,
         IDirect3DRMTexture *texture)
 {
-    FIXME("iface %p, texture %p stub!\n", iface, texture);
-
-    return E_NOTIMPL;
+    struct d3drm_frame *frame = impl_from_IDirect3DRMFrame(iface);
+    TRACE("iface %p, texture %p\n", iface, texture);
+    return d3drm_frame3_SetSceneBackgroundImage(&frame->IDirect3DRMFrame3_iface, (IDirect3DRMTexture3*)texture);
 }
 
 static HRESULT WINAPI d3drm_frame3_SetSceneFogEnable(IDirect3DRMFrame3 *iface, BOOL enable)
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/289

[PATCH 1/1] d3drm: Implement IDirect3DRMFrame2::SetSceneBackgroundImage

[Stefan Dösinger]

> Am 21.06.2022 um 09:44 schrieb Alistair Leslie-Hughes <wine(a)gitlab.winehq.org>:
>
> +    if (frame->backgroundimage)
> +        IDirect3DRMTexture3_Release(frame->backgroundimage);
> +    frame->backgroundimage = texture;
> +    if (frame->backgroundimage)
> +        IDirect3DRMTexture3_AddRef(frame->backgroundimage);

It is better to addref the new one before releasing the old one. In the current code you might destroy the image when the application is redundantly re-setting the current image and the only remaining reference is held by the d3drm frame.
> +    struct d3drm_frame *frame = impl_from_IDirect3DRMFrame2(iface);
> +    TRACE("iface %p, texture %p\n", iface, texture);
> +    return d3drm_frame3_SetSceneBackgroundImage(&frame->IDirect3DRMFrame3_iface, (IDirect3DRMTexture3*)texture);

This seems wrong, IDirect3DRMTexture3 and IDirect3DRMTexture2 (same for texture1) are different interfaces and have different vtable members in d3drm_texture. Later code that does impl_from_IDirect3DRMTexture3 will get a wrong implementation pointer.

What you can do here depends on a few things. It also ties to this line of code here:
> +++ b/dlls/d3drm/d3drm_private.h
> @@ -81,6 +81,7 @@ struct d3drm_frame
>     IDirect3DRMFrame3 IDirect3DRMFrame3_iface;
>     IDirect3DRM *d3drm;
>     LONG ref;
> +    IDirect3DRMTexture3 *backgroundimage;

If you need to be able to accept "wrong" texture versions and return the interface the application passed back to the application in GetBackgroundImage you can store the assigned texture as IUnknown or IDirect3DRMObject *. Then whenever some code accesses the texture methods it needs to QI the version it wants to call. If it needs the members, QI to a known version, then impl_from_IDirect3DRMTextureX. This option - store the interface - is especially the way to go if you might deal with interface implementations different from ours, i.e. if the application has its own implementation of d3drm textures and expects you to work with them.

Otherwise you can get the struct d3drm_texture from the version passed in in each of the SetSceneBackgroundImage versions and store a struct d3drm_texture * in struct d3drm_frame.