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

[Zebediah Figura]

On 6/21/22 17:12, Alistair Leslie-Hughes wrote:
> 
> 
> On 22/6/22 04:37, Zebediah Figura wrote:
>> 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.
> Yes, IDirect3DRMViewport::Render will be the have to be implemented at 
> some point and I understand the point your making.
> 
> For reference: Star Wars Rebellion
> 
> Currently this just crashes when starting a battle.  If I just fixed the 
> crash (have a patch for it already), then it displays a dialog for every 
> function that doesn't return success.  There is about 9 more patches to 
> stop all the scenarios, and in my opinion the crash is just as bad as 9+ 
> MessageBox's every render cycle.
> 
> My thought was to implement the infrastructure as much as possible 
> first, then take on Render. Maybe starting to implement Render next 
> might be good idea.

Sure, that kind of iteration is probably fine for local debugging, i.e. 
stub things out until you have a good idea of what all is going to be 
necessary. For upstream I think it's probably better to get to a state 
where we can properly render things, and then start implementing other 
states.

I recognize that implementing the entire workhorse parts of d3drm is 
probably a much more daunting task than filling out the state tracking. 
Accordingly please don't hesitate to reach out for assistance :-)