Jump to content
  • Advertisement
Sign in to follow this  
mdias

diamond problem with interfaces and down-casting

This topic is 1514 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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.

Share this post


Link to post
Share on other sites
Advertisement

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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

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...

Edited by mdias

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites


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...

Edited by mdias

Share this post


Link to post
Share on other sites

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?

Edited by mdias

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!