diamond problem with interfaces and down-casting

Started by
19 comments, last by mdias 9 years, 9 months ago

Hi,

I'm facing a problem I can't seem to find a good answer for.

I'm trying to build a wrapper for D3D11 and OpenGL. For this I have some interfaces similiar to D3D11's.

Let's assume these interfaces:


class IDeviceChild;
class IResource : public IDeviceChild;
class IBuffer : public IResource;

Now, what I wish to do, is for each of those interfaces to have their own implementation class, like this:


class oglDeviceChild : public IDeviceChild;
class oglResource : public IResource, public oglDeviceChild;
class oglBuffer : public IBuffer, public oglResource;

Now this obviously won't work like this because of the diamond problem I have here, and the only way to solve it is to have virtual inheritance in the interface classes themselves.
But that leads to another problem! If I have an oglResource, I can't static_cast to an oglBuffer.

It sure must be possible to solve this, since D3D does it (and doesn't use virtual inheritance in the interfaces).

It also looks like virtual inheritance only signals the class being inherited to be virtual, instead of that class plus it's parents...

The only way out I see right now is to avoid multiple inheritance and only inherit the interface, but that doesn't look like a proper solution to me...

Can anybody shed some light?

Thanks in advance.

Advertisement
You've designed your interfaces poorly.

An interface should not be coupled to an implementation. Moreover, it should not be necessary to know the concrete implementation of anything to use its interface.

Less abstractly: I should be able to do anything rendering-wise that I need to with your independent layer alone, without ever referencing a specific graphics API.


You only really need two things: an interface representing the API-independent functionality of each type of object, and an implementation of that interface for a given graphics API. If you want the internal API-specific code to be able to know things that are "off limits" to the rest of the world, add functions to the internal implementation's class.

Don't be too hasty to represent everything via implementation inheritance. It's almost always the wrong tool for the job. If you're using interface inheritance, you can solve all your problems with two classes and you're all done. Don't forget that composition is also a good option.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

I think I probably didn't explain my problem well enough. I agree with what you're saying, however this is not the problem. The clients using this wrapper will only interact with the interfaces without ever knowing about the implementation.


If you want the internal API-specific code to be able to know things that are "off limits" to the rest of the world, add functions to the internal implementation's class.

This is the problem. Imagine this piece of code on the client side:


ITexture* myTexture = ...;

myAbstractedDevice->SetTexture( 0, myTexture );

And this is the implementation:


class ITexture : public IResource
{
   ...
};

class oglTexture : public ITexture, public oglResource
{
   ...
};

void oglDevice::SetTexture( unsigned slot, ITexture* texture )
{
   assert( texture );
   assert( texture->GetServer() == this->GetServer() );

   oglTexture* ogl_texture = static_cast<oglTexture*>(texture);
   ...
}

If oglTexture only inherited from ITexture, this would work perfectly, but then I'd have to re-implement the other interfaces (IResource, IDeviceChild, ...) with mostly duplicated code, which makes me think it's not the right way to solve this.

Can you show some examples of what these interfaces actually contain? As it stands I see absolutely no justification for using inheritance at all, let alone multiple inheritance. You're also still upcasting, which is a telltale sign that you're too fixated on inheritance as an organizational method, where it doesn't belong at all.

If oglDevice::SetTexture() can't do everything it needs to do with an ITexture by calling methods that live on ITexture, you've designed a poor interface.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

Actually the current workaround I have found is to have a void* ITexture::GetPrivateDeviceData() which doesn't feel very clean but works for now. I'm still searching for a better way.


Can you show some examples of what these interfaces actually contain?

oglTexture:oglTexture( oglGraphicsContext& ctx, const TextureDescription& desc )
{
   glGenTextures( 1, &_gl_texture ) // _gl_texture is a private/protected member variable
   ... // bind and create texture with content from "desc"
}

GLuint oglTexture::GetGLTexture() const // this is the method I wish oglGraphicsContext to see
{
   return _gl_texture;
}


bool oglGraphicsContext::SetTexture( unsigned slot, ITexture* texture )
{
   // check that texure is a texture created by this graphics context
   if( texture->GetDeviceCtx() != this )
      return false;

   auto ogl_texture = static_cast<oglTexture*>(texture); // this is the cast that's needed and won't work with virtual inheritance

   GLuint native_texture = ogl_texture->GetGLTexture();
   ... // use native_texture for whatever...
}
which is a telltale sign that you're too fixated on inheritance as an organizational method, where it doesn't belong at all.

I understand and I've been thinking myself that maybe I'm not choosing the best approach, but can't really think of anything as "clean"/"elegant" as this, which is why I posted here smile.png


If oglDevice::SetTexture() can't do everything it needs to do with an ITexture by calling methods that live on ITexture, you've designed a poor interface.

If I implement the ITexture::GetPrivateDeviceData() method as I mentioned above, I can do this, but I still feel this shouldn't really be a visible method. Plus having virtual inheritance on the interfaces will bring more problems to the end client trying to cast for example from IResource to ITexture... This could again be worked around by having a IDeviceChild::CastTo( Type ) but then it will have a performance cost and ugly feel...

You're doing it backwards.


Objects should be thought of like little hives of activity, with some associated state. If you want something to happen, you don't grab the guts of the object and smack them around until they do what you want. Instead, you instruct the object to do what you want, and give it any required data that it doesn't already have inside it.


For your example, instead of a oglDevice reaching into the private details of an ITexture in order to send it to the OpenGL API, add a method called MakeActive to ITexture.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

Liskov Substitution principle is kind of important. You have a function:

bool oglGraphicsContext::SetTexture( unsigned slot, ITexture* texture )

What happens if you give it a D3DTexture? By definition, that should be totally fine. However, in the context of the problem you're trying to solve, it inherently isn't totally fine. You can try to fight this, but fundamentally, it's going to fight you back every step of the way, and you'll end up with an API even you don't like. If a function is only valid with a particular subclass, the interface needs to require that subclass.

The problem with your API is that you're not actually trying to abstract ogl and d3d into a single interface -- you're trying to factor out junk that both APIs use. In the end, you're left with two broken APIs that look superficially similar to eachother.

To actually create a single API, you absolutely cannot do this double-dipping inheritance thing. Something has to be uniform, whether it's the texture or device context depends on how you do it.



For your example, instead of a oglDevice reaching into the private details of an ITexture in order to send it to the OpenGL API, add a method called MakeActive to ITexture.

This would certainly solve some problems, such as not needing to cast anymore. However, the way I see it, the Texture object would be modifying the owner's (oglGraphicsContext) state which doen't make much sense. I guess this is also the reason the D3D API goes this way instead of backwards.

[Edit] It would also make it impossible to bind more than just one object in a single call.


What happens if you give it a D3DTexture? By definition, that should be totally fine.

I disagree. The fact that SetTexture contains an ITexture doesn't automatically mean it will be OK to pass any ITexture. I can certainly write in the documentation that the ITexture must be created by the same IGraphicsContext and be in a specific state.


You can try to fight this, but fundamentally, it's going to fight you back every step of the way, and you'll end up with an API even you don't like.

I think I would like the API but not the fact that I would be double checking if the objects are valid everywhere.

I am trying to adopt concepts of future graphics APIs such as PSOs which aliviate a lot of this by only checking at PSOs creation time. However some state just doesn't belong in a PSO...

I am inspiring my class design on D3D11's, which seems to do things exacly like I'm trying to implement, but they somehow manage to not need virtual inheritance.

I think I might be worrying too much about correctness and should think more about ease of use (adopt ::MakeActive() and ignore the fact that I'm modifying state that doesn't belong to this child object), but I'm afraid that by doing so I might be commiting suicide in the long run...

This still doesn't solve being unable to static_cast from an IResource to an ITexture for example. How does D3D do it? The only way I see it working is to avoid shared implementations and only implement things in the final class, which can be a maintenance nightmare...

This would certainly solve some problems, such as not needing to cast anymore. However, the way I see it, the Texture object would be modifying the owner's (oglGraphicsContext) state which doen't make much sense. I guess this is also the reason the D3D API goes this way instead of backwards.

[Edit] It would also make it impossible to bind more than just one object in a single call.



Lies. Pass a container of ITexture* pointers.





I disagree. The fact that SetTexture contains an ITexture doesn't automatically mean it will be OK to pass any ITexture. I can certainly write in the documentation that the ITexture must be created by the same IGraphicsContext and be in a specific state.


Your opinion is relatively unimportant. People who have spent their careers studying software architecture - and won very prestigious awards because of it - created the principles we're talking about. They aren't just subjective hooplah, they are very important for creating good APIs. The Liskov Substitution Principle in question here is not exactly something you are qualified to argue with.

Nothing personal, man, but if you're asking entry level API design questions in a programming forum, you're not ready to debate the validity of LSP in your designs.



I think I would like the API but not the fact that I would be double checking if the objects are valid everywhere.


What are you talking about? If you design your code correctly, the compiler will check for your correctness as much as possible, and you don't need to dick around with redundant validity checks. Your software becomes correct by design, not by "I'll put it in the documentation" or "I'll check it every three lines" or some similar rubbish.

I am inspiring my class design on D3D11's, which seems to do things exacly like I'm trying to implement, but they somehow manage to not need virtual inheritance.


No, dude. They aren't doing anything like what you're doing. D3D11 is not trying to implement OpenGL support. This should be blindingly obvious. Therefore they are solving a different problem and their solution is not automatically something you should try to adopt for your (vastly different) situation.


I think I might be worrying too much about correctness and should think more about ease of use (adopt ::MakeActive() and ignore the fact that I'm modifying state that doesn't belong to this child object), but I'm afraid that by doing so I might be commiting suicide in the long run...


You're not worrying about correctness at all. You're worrying about superstition (your "suicide" worries) and misunderstandings (your completely left-field belief that objects should not modify each other). If ITexture::MakeActive() needs to do something with a FooObject, just call the appropriate method on the FooObject. Done.


This still doesn't solve being unable to static_cast from an IResource to an ITexture for example. How does D3D do it? The only way I see it working is to avoid shared implementations and only implement things in the final class, which can be a maintenance nightmare...


static_cast from a base to a derived class is totally trivial, provided you're not screwing with virtual inheritance. It's still ugly and not cool, but it isn't exactly the bundle of snakes that you've uncovered in your approach.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

I don't think we're understanding each other at all. Maybe the fact that english is not my native language is somehow impeding me to explain my problem properly.


Lies. Pass a container of ITexture* pointers.

Where to exacly should I pass an array/container of ITexture pointers if you're telling me I should use ITexture::MakeActive() ?

And doesn't that mean again that this container could have ITexture pointers created from another context?


Your opinion is relatively unimportant. People who have spent their careers studying software architecture - and won very prestigious awards because of it - created the principles we're talking about. They aren't just subjective hooplah, they are very important for creating good APIs. The Liskov Substitution Principle in question here is not exactly something you are qualified to argue with.

Nothing personal, man, but if you're asking entry level API design questions in a programming forum, you're not ready to debate the validity of LSP in your designs.

I'm not really questioning the validity of LSP. I just don't see a way to fully comply with it in this particular problem.

I must add constraints to what is accepted. We don't live in a perfect world, there are exceptions I want to deal with appropriately. Say I give it a texture that is resident in another GPUs memory; I want to return an error from that (instead of invalid rendering or crash), even if in theory it should work, I could document that behaviour.

The *::MakeActive approach would solve this though, but I would like to be able to MakeActive several textures in one call.


What are you talking about? If you design your code correctly, the compiler will check for your correctness as much as possible, and you don't need to dick around with redundant validity checks. Your software becomes correct by design, not by "I'll put it in the documentation" or "I'll check it every three lines" or some similar rubbish.

Again, you misunderstood me. I mean the correctness of the internal state of the objects. I'm not talking about language correctness. In this specific case, by correctness I mean "the texture must belong to the same context you're trying to bind it to for rendering".

Curious question: how would D3D behave if it were given, say a shader, from another device?


No, dude. They aren't doing anything like what you're doing. D3D11 is not trying to implement OpenGL support. This should be blindingly obvious. Therefore they are solving a different problem and their solution is not automatically something you should try to adopt for your (vastly different) situation.

Well the OpenGL or not problem is not relevant at all to the question I'm asking. They still have interface inheritance to which they must provide implementations somewhere. When I said "how does D3D do it", I meant do they duplicate code for each object that needs to provide an implementation to ID3D11Resource or solve this problem any other way? They still must cast it to an implementation pointer somewhere...


If ITexture::MakeActive() needs to do something with a FooObject, just call the appropriate method on the FooObject. Done.

This makes sense and I had already thought of it. However then ITexture::MakeActive() would be nothing but a bridge to the oglGraphicsContext's method. Which I don't have a problem with except if there's a better solution. Also, again this would mean a call to MakeActive() for each texture, and also not the way D3D does it.


static_cast from a base to a derived class is totally trivial, provided you're not screwing with virtual inheritance. It's still ugly and not cool, but it isn't exactly the bundle of snakes that you've uncovered in your approach.

Exacly. So do you agree (excluding personal preference) with me that oglGraphicsContext could accept an ITexture and down-cast it there?

Maybe I should try to put my main question in another way: How can I implement an interface hierarchy and reuse implementation code (without ugly "hacks" like macros) while avoiding multiple inheritance?

This topic is closed to new replies.

Advertisement