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

[Aaryaman Vasishta]

>
> 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/4c67b64c/attachment.html>