adventuregamestudio / ags

AGS editor and engine source code
Other
708 stars 159 forks source link

Implemented a RAII wrapper over IUnknown, for Direct3D #2353

Closed ivan-mogilko closed 8 months ago

ivan-mogilko commented 8 months ago

Resolves #2343.

An automatically releasing pointer for IDirect3D objects.

ericoporto commented 8 months ago

I think the code for IUnknownPtr needs to be wrapped in a #if AGS_PLATFORM_OS_WINDOWS clause

ivan-mogilko commented 8 months ago

I think the code for IUnknownPtr needs to be wrapped in a #if AGS_PLATFORM_OS_WINDOWS clause

It has zero dependency on windows headers, and may work with anything that has AddRef/Release methods.

But I have second thoughts about this approach, will revise this later.

ivan-mogilko commented 8 months ago

Hmm, maybe this is still a not so bad approach. I found that it's not much different to a variant found in this MS dev article, and they even have a similar method of retrieving internal pointer field address for assignment (their function is called "GetAddressOf", while mine is "Acquire"): https://learn.microsoft.com/en-us/archive/msdn-magazine/2015/february/windows-with-c-com-smart-pointers-revisited

I still need to reorganize this a bit, and I found there's some bug, occuring when resizing a window (but strangely, not when changing from windowed to fullscreen). I will have to carefully recheck all usages of the new ptr type.

EDIT: since I'm at this, i might also check out why there's constantly a warning about 1 remaining reference to IDirect3D on exit.

ericoporto commented 8 months ago

I threw a bunch of games on this and they all seem to work.

Small things in smart_ptr.h.

EDIT: since I'm at this, i might also check out why there's constantly a warning about 1 remaining reference to IDirect3D on exit.

Right, I do still get the WARNING: Not all of the Direct3D resources have been disposed; ID3D ref count: 1 at exit too, but this is maybe a different PR.

The CirrusCI failure just looks like a temporary network bug in their end... Rerunning it should work.

ericoporto commented 8 months ago

Oh, I meant just replacing. The <TInterface> next to ComPtr could omitted.

It can be left there too if it makes things clearer.

Edit: actually perhaps it's better to leave it there, not sure how different compilers see this. I only tested with MSVC. :/