Closed DiligentGraphics closed 4 years ago
Thanks for looking into this. In fact, virtual tables are not compiler-specific, and all compilers will generate the same layout, so it should be safe. I was thinking about using something like what microsoft uses to define their interfaces that avoids middle layer, for example:
#if defined(__cplusplus) && !defined(CINTERFACE)
MIDL_INTERFACE("1841e5c8-16b0-489b-bcc8-44cfb0d5deae")
ID3D11DeviceChild : public IUnknown
{
public:
virtual void STDMETHODCALLTYPE GetDevice(
/* [annotation] */
_Outptr_ ID3D11Device **ppDevice) = 0;
virtual HRESULT STDMETHODCALLTYPE GetPrivateData(
/* [annotation] */
_In_ REFGUID guid,
/* [annotation] */
_Inout_ UINT *pDataSize,
/* [annotation] */
_Out_writes_bytes_opt_( *pDataSize ) void *pData) = 0;
virtual HRESULT STDMETHODCALLTYPE SetPrivateData(
/* [annotation] */
_In_ REFGUID guid,
/* [annotation] */
_In_ UINT DataSize,
/* [annotation] */
_In_reads_bytes_opt_( DataSize ) const void *pData) = 0;
virtual HRESULT STDMETHODCALLTYPE SetPrivateDataInterface(
/* [annotation] */
_In_ REFGUID guid,
/* [annotation] */
_In_opt_ const IUnknown *pData) = 0;
};
#else /* C style interface */
typedef struct ID3D11DeviceChildVtbl
{
BEGIN_INTERFACE
HRESULT ( STDMETHODCALLTYPE *QueryInterface )(
ID3D11DeviceChild * This,
/* [in] */ REFIID riid,
/* [annotation][iid_is][out] */
_COM_Outptr_ void **ppvObject);
ULONG ( STDMETHODCALLTYPE *AddRef )(
ID3D11DeviceChild * This);
ULONG ( STDMETHODCALLTYPE *Release )(
ID3D11DeviceChild * This);
void ( STDMETHODCALLTYPE *GetDevice )(
ID3D11DeviceChild * This,
/* [annotation] */
_Outptr_ ID3D11Device **ppDevice);
HRESULT ( STDMETHODCALLTYPE *GetPrivateData )(
ID3D11DeviceChild * This,
/* [annotation] */
_In_ REFGUID guid,
/* [annotation] */
_Inout_ UINT *pDataSize,
/* [annotation] */
_Out_writes_bytes_opt_( *pDataSize ) void *pData);
HRESULT ( STDMETHODCALLTYPE *SetPrivateData )(
ID3D11DeviceChild * This,
/* [annotation] */
_In_ REFGUID guid,
/* [annotation] */
_In_ UINT DataSize,
/* [annotation] */
_In_reads_bytes_opt_( DataSize ) const void *pData);
HRESULT ( STDMETHODCALLTYPE *SetPrivateDataInterface )(
ID3D11DeviceChild * This,
/* [annotation] */
_In_ REFGUID guid,
/* [annotation] */
_In_opt_ const IUnknown *pData);
END_INTERFACE
} ID3D11DeviceChildVtbl;
interface ID3D11DeviceChild
{
CONST_VTBL struct ID3D11DeviceChildVtbl *lpVtbl;
};
I noticed that VC++ puts the VTable pointer as the very beginning of the object (as long as you use pure virtual classes/interfaces). But I think Clang places some other hidden fields first (like a pointer to an RTTI table, conform the Itanium ABI spec), but I haven’t tested this yet.
Adding “official” C-compatible VTable support like you are proposing is certainly much better since it doesn’t depend on ABI implementations and works with all languages that can interface with C. Also, your approach is preferable over a “regular” flat C wrapper since it avoids the extra layer of indirection. Your clean interface-based design is certainly very helpful here!
Hm... RTTI information does live in the virtual table (the pointer to the vtbl is still at the very beginning of the object though). May worth looking into what clang and gcc generate here. Diligent does not really need RTTI and release builds are compiled without RTTI. It uses RTTI in debug builds for debug purposes only.
Perhaps IObject is already ABI-compatible with IUnknown (I'm no C++ expert). That would be great since that would mean you could use the interface directly in languages (like Delphi) that support the COM model.
Looks like IObject is mostly compatible with IUnknown. It has the methods QueryInterface, AddRef and Release are in the right order, and the parameters seem to match. Those three methods would have to conform to the stdcall calling convention on Win32 though...
I will try this out with the current code base and let you know the results...
Yes, IObject
is essentially a cross-platform analog of IUnknown
, Diligent::INTERFACE_ID
is GUID
so Diligent should be compatible with COM.
Those three methods would have to conform to the stdcall calling convention on Win32 though...
Are you targeting 32-bit Windows?
I'm not targeting Win32 specifically. I just mentioned this because Diligent currently supports Win32 as well. I'm fine with not supporting Win32 for this purpose. This will become a non-issue anyway... On non-Windows platforms, the calling convention is always fixed, so this shouldn't be an issue there either.
I just did a quick test with Delphi using the standard Win64 DLLs and it seems to work so far without any C APIs, other than the GetEngineFactory* functions. So it seems that IObject is fully compatible with IUnknown on Win64.
FYI. In Delphi I would then just write Delphi-versions of the Diligent interfaces, and the Delphi compiler will take care of calling AddRef and Release automatically whenever a (strong) reference to an interface is acquired or released. This would make a Diligent language binding for Delphi pretty easy to implement without any work on your part.
If you want to go this route, you may want to consider versioning of your interfaces (like MS does with COM interfaces) or just simply require from language bindings that their bindings stay in sync with the Diligent libraries they link to. They later option leads to a cleaner API IMHO...
Yes, the API is still in flux a bit. (For example right now I am working on improvements to pipeline resource definition and shader resource reflection), so keeping language bindings in sync sounds like a better alternative than keeping around old versions. Luckily, the API is not so large. EngineFactory interfaces can also be updated to conform to COM.
Yes, it would be great if all interfaces derive from IObject, so I can easily import them. (I was doing some manual VTable hacking to get the factory interface to work. That would not be necessary anymore if they are COM compliant).
Compliments on the clean API and it’s design. Looks like you guys put a lot of thought into it. I also like the use of structs instead of passing around a bunch of parameters. And this model maps nicely to the Delphi programming language, which is a plus for me personally.
Exploiting the vtable directly to remove the C layer could be a good idea only if the vtable layout is identical for every compiler, but this doesn't seem granted : https://reverseengineering.stackexchange.com/questions/12857/does-the-order-of-vtable-entries-vary-depending-on-the-compiler
Problem is C++ has no standard ABI and the layout of instances varies.
What could be done is to investigate how to explicitly declare class layouts, using alignas
and offsetof
, have various compile time static checks, and make sure diligent API provides an homogeneous ABI.
@DiligentGraphics could you please provide the list of interfaces that will likely be exposed?
ok everything in the interface folder :) I'm just discovering the api, looks well structured
@raizam I realize that there is no standard ABI for c++, so you shouldn't generally expect that library built with gcc will work in msvc. This is also true even if you use different versions of the same compuler. However as far as I am concerned, all major compilers (msvc, glang, and gcc) implement virtual functions in the same way, which should be binary compatible with c jump table.
if virtual methods are implemented the same way everywhere, that's great.
but there's another problem: the arguments of these methods takes various structures, and these structures need to have the same offset/alignment on every compiler... which is why I suggest the explicit use of the alignas
modifier
I was trying to arrange the structures such that members are tightly packed without any padding (this may not be true for all structures though). Potentially just checking the expected structure size with static_assert could be enough to catch unexpected compiler behavior (I do this for some internal structures)
@neslib Engine factory interfaces are now all derived from IObject
(https://github.com/DiligentGraphics/DiligentCore/issues/72). Please note that there have also been some API changes
@DiligentGraphics Great, thanks! I will try this out soon.
@neslib You may find release notes useful
@DiligentGraphics FYI, as a proof of concept, I have working Delphi versions now of tutorials 00 and 01 on Win64, for all 4 backends. I will target iOS and Android next, followed by Mac (waiting for a 64-bit compiler). Now that the basics are working, I expect that completing most the API translation will go smoothly.
I am running into an issue though when working on tutorial 02. It uses a IShaderSourceInputStreamFactory. This interface does not derive from IObject, so I cannot use it to create an input stream in Delphi. Could you derive this interface from IObject?
Also, I added a custom C-API that reports the sizes of all public structs. I use this on the Delphi side to perform sanity checks and check if the Delphi structs match the C structs (at least in size). This may be a useful addition as it helps debugging problems when structs are changed in future releases. With some compilers, you may also need to ad some custom padding to make sure the structs match.
In addition, this API also does a crude version check to make sure I load the correct library. It basically looks like this:
// Matches Diligent version 2.4.b
#define STRUCT_SIZES_VERSION 0x020402
#define STRUCT_SIZES_ERROR_SIZE_MISMATCH -1
#define STRUCT_SIZES_ERROR_BAD_VERSION -2
struct StructSizes {
int Size;
int RequestedVersion;
int ActualVersion;
// BlendState.h
int RenderTargetBlendDesc;
int BlendStateDesc;
// Buffer.h
int BufferDesc;
int BufferData;
...
};
int GetStructSizes(StructSizes* sizes) {
if (sizes == nullptr)
return sizeof(StructSizes);
if (sizes->Size < sizeof(StructSizes))
return STRUCT_SIZES_ERROR_SIZE_MISMATCH;
if (sizes->RequestedVersion > STRUCT_SIZES_VERSION)
return STRUCT_SIZES_ERROR_BAD_VERSION;
sizes->ActualVersion = STRUCT_SIZES_VERSION;
// BlendState.h
sizes->RenderTargetBlendDesc = sizeof(RenderTargetBlendDesc);
sizes->BlendStateDesc = sizeof(BlendStateDesc);
// Buffer.h
sizes->BufferDesc = sizeof(BufferDesc);
sizes->BufferData = sizeof(BufferData);
...
return sizeof(StructSizes);
}
Maybe it's an idea to add an official API like this for people creating language bindings?
I have working Delphi versions now of tutorials 00 and 01 on Win64, for all 4 backends.
That is really great news!
It uses a IShaderSourceInputStreamFactory. This interface does not derive from IObject, so I cannot use it to create an input stream in Delphi
Created an issue: https://github.com/DiligentGraphics/DiligentCore/issues/80
Also, I added a custom C-API that reports the sizes of all public structs.
I guess it is possible to add such function to engine factories, but ideally you would check this somehow at compile time, maybe using static_assert
or something similar. BTW, did you have to duplicate the structures? I think all engine structures can be made compilable in c
I guess it is possible to add such function to engine factories, but ideally you would check this somehow at compile time, maybe using
static_assert
or something similar.
That would be ideal, but in Delphi I don't know the sizes of each C++ struct at compile time, since those are defined in C++ code, and I am compiling Delphi code. Microsoft often "addresses" this issue by adding a cbSize
field to a struct and checking this each time the API is called. I don't like this approach and prefer to do all sanity checks at app startup. Anyway, this is no big deal and most other libraries don't supply this kind of information either. It is just a nice-to-have.
BTW, did you have to duplicate the structures? I think all engine structures can be made compilable in c
Yes, I have to translate each C(++) struct to Delphi code. For example:
struct BufferData
{
const void* pData = nullptr;
Uint32 DataSize = 0;
};
Is translated to:
type
TBufferData = record
Data: Pointer;
DataSize: UInt32;
end;
Now is this one of the simplest structs, but the same applies to all structs. Unfortunately, Delphi does not support inheritance of structs, but I addressed that by adding a base
field of the type of the parent struct.
Anyway, because I need to make translations, I need to make sure the layout matches the C version, hence the API I added to perform some sanity checks. I assume that developers of other language bindings will have to translate these structs as well, but then again, those languages probably cannot use your API as-is, but need a true C-API.
In other news, I ran into a couple more issues while porting over tutorial 02:
IPipelineState.GetStaticShaderVariable
is an overloaded method; it has the same name but two different signatures (one using a Char*
and one using an Uint32
parameter). In the vtable that is generated by the VC++ compiler, these two entries are swapped (that is, the Uint32
version comes before the Char*
version, while they are in opposite order in the source code). I assume this is because both methods have the same name and VC++ handles these differently. I haven't checked (yet) if this also happens with other compilers. Don't know a good solution yet other than using different names (which may be required anyway when creating a true C-API binding).
Regarding these same methods: they return an interface. Most other APIs don't return interfaces as a function result, but as an out-parameter (**
) instead. Functions that return an interface are problematic in Delphi, while functions that return interfaces as out-parameters work fine. This is an issue I can work around, so not something you have to look at. I wouldn't want you to make changes just for this specific use case.
Some APIs require an IDataBlob
field or parameter. However, there is no public way to create an object that implements the IDataBlob
interface. In C++, you would use MakeNewRCObj<DataBlobImpl>
, but that API is not available outside of the C++ side. We probably need a factory function to create a data blob.
Sorry to keep bothering you with these issues...
Microsoft often "addresses" this issue by adding a cbSize field to a struct and checking this each time the API is called.
It is easy to add something like GetApiInfo()
function to the engine factory so that the app can check compatibility when initializing the engine. It will return API version along with the struct sizes.
In the vtable that is generated by the VC++ compiler, these two entries are swapped (that is, the Uint32 version comes before the Char* version, while they are in opposite order in the source code).
That is extremely bizarre. Could either be compiler bug or some peculiarity of the standard. I've always believed that vtbl entries must appear in exactly same order that they are defined in the source. Anyway, this can be resolved by refining the method names (GetStaticShaderVariableByName
and GetStaticShaderVariableByIndex
). I guess you are right about c bindings as there is no function overload in c and this should be done anyway. https://github.com/DiligentGraphics/DiligentCore/issues/82
Regarding these same methods: they return an interface. Most other APIs don't return interfaces as a function result, but as an out-parameter (**) instead.
Diligent uses the following concept (which I believe was introduced by MSFT): when an interface is returned as out parameter (*), its reference counter is incremented. All Create methods work this way. If an interface is returned as function return value, its reference counter is not incremented. This way it is convenient to write constructs like
pSRB->GetVariable(SHADER_TYPE_PIXEL, "g_Texture")->Set(pRTColor->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE));
Reworking this with out parameters would make code cumbersome and less efficient. What is the problem with returning interfaces in Delphi? Isn't it just like returning a pointer?
Some APIs require an IDataBlob field or parameter. However, there is no public way to create an object that implements the IDataBlob interface.
Diligent completely decouples core from everything platform-specific. That is why there is no core functions to create IDataBlob
, IShaderSourceInputStreamFactory
and other platform-specific interfaces. Diligent provides default implementations but they are not part of the core. It was assumed that applications will provide their own platform-specific implementations if needed.
Adding default implementations for IDataBlob
, IShaderSourceInputStreamFactory
to core does not seem to be a big deal. However, starting with Tutorial03 everything uses image loading functionality which should definitely not be in core (it is in Tools module). All these functions are linked statically with the app. Again, the assumption is that applications will implement their own image loading routines that are orthogonal to Core graphics API, if needed.
One possible way to solve this is to build Tools module also as DLL and expose texture loading functions from there.
@neslib BTW, don't worry about posting the issues you find. Diligent Engine is supposed to be useful tool for developers in the first place and all the issues that you reveal help make the API more consistent, robust and usable.
It is easy to add something like GetApiInfo() function to the engine factory so that the app can check compatibility when initializing the engine. It will return API version along with the struct sizes.
That would be helpful. Thanks!
That is extremely bizarre. Could either be compiler bug or some peculiarity of the standard. I've always believed that vtbl entries must appear in exactly same order that they are defined in the source.
FYI: I encountered the same issue with the IShaderResourceBinding
interface. The methods are declared in source in this order:
GetVariable(char*);
GetVariableCount();
GetVariable(Uint32);
But the vtable seems to contain the methods in this order:
GetVariable(Uint32);
GetVariable(char*);
GetVariableCount();
Reworking this with out parameters would make code cumbersome and less efficient. What is the problem with returning interfaces in Delphi? Isn't it just like returning a pointer?
I agree, so don't worry about it. In Delphi, the reference counting is all automatic and performed by the compiler. When a method returns an interface, the automatic reference counting kicks in and causes strange errors I haven't investigated (yet). This is still strange though, since it shouldn't matter that Delphi automatically calls AddRef when the method returns, followed by a Release when the interface goes out of scope. Anyway, when I modify the binding to return a plain pointer instead, and then typecast that pointer to the interface, then everything works. So no need to take action on your side.
Diligent completely decouples core from everything platform-specific. That is why there is no core functions to create IDataBlob, IShaderSourceInputStreamFactory and other platform-specific interfaces.
I completely agree that these should be decoupled. I was under the impression that since IDataBlob
and IShaderSourceInputStreamFactory
are part of the core, there should be a way to create them. You are right that in C++ you can create your own implementations of these interfaces (or use some provided ones). In Delphi, that would be possible as well, except this is problematic because of the IObject.GetReferenceCounters
method, I cannot implement this method in Delphi since IReferenceCounters
is a plain C++ class and not an interface. And you cannot derive it from IObject
since that would create a catch-22 situation.
Anyway, I can do all image and shader loading without the use of the IDataBlob
and IShaderSourceInputStreamFactory
interfaces, so this should not be an issue. I managed to get tutorials 02 and 03 working in Delphi without these interfaces.
One possible way to solve this is to build Tools module also as DLL and expose texture loading functions from there.
That would be a nice-to-have feature in the future for designers of language bindings. But for now, I will just create my own texture loading functions. Consider this a low-priority request.
I encountered the same issue with the IShaderResourceBinding interface.
I refactored overloaded IShaderResourceBinding::GetVariable
and IPipelineState::GetStaticShaderVariable
methods.
Anyway, I can do all image and shader loading without the use of the IDataBlob and IShaderSourceInputStreamFactory interfaces, so this should not be an issue. I managed to get tutorials 02 and 03 working in Delphi without these interfaces.
You will definitely need to use IShaderSourceInputStreamFactory
interface at some point because this is how the engine access shader source code. I will make the interface derived from IObject
so that it is compliant with the rest of the API.
@neslib Also added API info query: https://github.com/DiligentGraphics/DiligentCore/commit/dd56cf89a936c3a1c0a7a9a607c1ef51b1f939cd. The info is queried via engine factory
@neslib Let's move Delhi-related discussion to this issue
Because Diligent uses a very clean interface-based model, it should be possible to use the C++ VTable directly from other languages. This would eliminate one level of indirection by leaving out the middle C layer.
I am currently experimenting with this to use Diligent from the Delphi language, and initial results are promising. Of course, the VTable layout varies by compiler, but the Visual Studio output is pretty straightforward. I am no C++ expert so may run into problems with other compilers. My goals is to get it to work with (clang) compilers for Mac, iOS and Android as well...
You also need to be very careful with any interface changes, as these will change the VMT. This probably means that this model will only work as long as you keep the language binding and particular Diligent version in sync.