[PATCH v2 1/1] d3drm: Implement IDirect3DRMFrame{1/2/3}::SetSceneBackgroundImage

Zebediah Figura zfigura at codeweavers.com
Tue Jun 21 13:37:12 CDT 2022

On 6/21/22 06:53, Alistair Leslie-Hughes wrote:
> From: Alistair Leslie-Hughes <leslie_alistair at hotmail.com>
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair at hotmail.com>
> ---
>   dlls/d3drm/d3drm_private.h |  1 +
>   dlls/d3drm/frame.c         | 38 +++++++++++++++++++++++++++++++-------
>   2 files changed, 32 insertions(+), 7 deletions(-)

In general I'm concerned about these commits adding state tracking code 
that's never actually used. Most notably, it means we can't test 
anything but the state tracking, and while there's not *likely* to be 
anything wrong with the state tracking as it is, it's not impossible either.

It's also going to make it harder to implement the real workhorse 
methods. Once you finally get to the point where you need to implement 
IDirect3DRMViewport::Render() [and, although it hasn't been stated, I'm 
guessing you're dealing with a d3drm application that is eventually 
going to call it], you'll have a whole lot of state that you'll suddenly 
need to implement, or else basically have methods that silently return 
success without doing anything useful, which is not good for debugging.

In this case it's even worse because "backgroundimage" is never read, so 
it's completely dead code.

More information about the wine-devel mailing list