On Sun, Jul 10, 2016 at 9:39 PM, Stefan Dösinger <stefandoesinger@gmail.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Am 2016-07-07 um 14:52 schrieb Aaryaman Vasishta:
> - {&CLSID_CDirect3DRMTexture, d3drm_create_texture_object},
> - {&CLSID_CDirect3DRMDevice, d3drm_create_device_object},
> + { &CLSID_CDirect3DRMTexture, d3drm_create_texture_object },
> + { &CLSID_CDirect3DRMDevice, d3drm_create_device_object },
> + { &CLSID_CDirect3DRMViewport, d3drm_create_viewport_object },
I guess Visual Studio or xcode re-added the spaces between { and &. Henri removed them in the slightly modified version of "d3drm: Use a table in d3drm3_CreateObject() to create objects in a generic manner."

True. I actually thought this was fine since the QI test tables were formatted in a similar way (see test_d3drm_qi, test_texture_qi etc). But I guess the current format is considered better, so I'll go with that for the implementations.

> + if (FAILED(hr = IDirect3DRMFrame3_QueryInterface(camera, &IID_IDirect3DRMFrame, (void **)&viewport->camera)))
> + goto cleanup;
Aren't you more likely to need the implementation structure of the camera in the future?

You mean accessing the struct directly instead of via the interface functions? I'm fine with that too. The interface functions do provide a way to get most of what's needed (I could be wrong though, maybe some internal data that's not revealed outside can still be utilized within calls), but I guess using the struct directly would be cleaner, and safer in the long run.

> +
> + if (material)
> + IDirect3DMaterial_Release(material);
In the success case this will destroy the material you just created. Our IDirect3DViewport::SetBackground implementation does not AddRef the material. I don't think we have any tests for this, but because SetBackground accepts a handle instead of a COM object I don't expect it to AddRef.

Overall it seems questionable to call camera->GetSceneBackground and apply the color here. I'd expect that the camera background can be changed after creating the viewport.

Ah, I was under the impression that IDirect3DViewport keeps a reference to the material (should have studied its implementation here). So I guess a reference to the material should be stored in the struct as well, and released on destroy.

Cheers,

Aaryaman