[PATCH 1/3] d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromClipper (try 5).

[Aaryaman Vasishta]

That d3drm_clipper variable is pointless, but please let me know if the
rest of it is fine.

Thank you!
Jam

On Fri, Jun 26, 2015 at 1:41 AM, Aaryaman Vasishta <
jem456.vasishta at gmail.com> wrote:

> This is by no means a full review, just a couple of obvious things I noticed.
>>
>> On 24 June 2015 at 17:02, Aaryaman Vasishta <jem456.vasishta at gmail.com <https://www.winehq.org/mailman/listinfo/wine-devel>> wrote:
>> >* +static HRESULT CALLBACK surface_callback(IDirectDrawSurface *surface, DDSURFACEDESC *desc, void *context)
>> *>* +{
>> *>* +    IDirectDrawClipper *d3drm_clipper = NULL;
>> *Initializing this to NULL is pointless, you initialize it before all
>> uses with the IDirectDrawSurface_GetClipper() call below.
>>
>> >* +        if (context)
>> *>* +        {
>> *"context" should always be non-NULL here.
>>
>> >* +            hr = IDirectDrawSurface_GetClipper(surface, &d3drm_clipper);
>> *>* +            ok(hr == DD_OK, "Cannot get attached clipper from primary surface (hr = %x).\n", hr);
>> *>* +            if (SUCCEEDED(hr))
>> *You have an ok() one line above that states "hr == DD_OK" at this
>> point. If the GetClipper() call failed you'd get a test failure.
>>
>> Once you've found the primary you can of course return DDENUMRET_CANCEL.
>>
>> I think you're making things too complicated though. I'd just call
>> EnumSurfaces() with "&primary" as context pointer, set it in the
>> callback, and then just do the rest of the tests after EnumSurfaces()
>> returns, instead of trying to fit those into the callback.
>>
>> Thank you for the suggestions! I have decided to return the primary
> surface directly, and set it to NULL if it doesn't exist via the context,
> something like this, which seems to run fine on my windows machine:
>
>> static HRESULT CALLBACK surface_callback(IDirectDrawSurface *surface,
>> DDSURFACEDESC *desc, void **context)
>> {
>>     IDirectDrawClipper *d3drm_clipper;
>>     HRESULT hr;
>>
>>     if (desc->ddsCaps.dwCaps & DDSCAPS_PRIMARYSURFACE)
>>     {
>>         *context = surface;
>>         return DDENUMRET_CANCEL;
>>     }
>>     IDirectDrawSurface_Release(surface);
>>
>>     return DDENUMRET_OK;
>> }
>>
> Then in the tests, simply check if the context returned is NULL, if yes
> then perform related tests on it inside the test.
>
>
> Thank you!
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20150626/dfc97ceb/attachment.html>