PDA

View Full Version : Diamond GL - pure C++ for OpenGL API (in development)



capitalknew
08-27-2017, 01:37 PM
I development C++ (planned fully C++14, C++17 compatible, with Visual Studio support) wrapper for OpenGL API.
https://github.com/AwokenGraphics/diamond-gl

Advantages:
- Support of GLM
- Support of STL (vector, string)
- Utils for simpler access
- Same paradigm as in OpenGL (nearest as can)
- Object oriented for C++
- Combined most OpenGL functions
- Support of DSA

Disadvantages:
- Not all API functions released at now (in TODO - FBO, query, state functions)
- No CUDA support
- In early stage of development

Alfonse Reinheart
08-27-2017, 06:56 PM
If you're looking for some advice, I find many of your design choices to be... dubious.

Your API is needlessly objectified. As an example, take `state.hpp` (https://github.com/AwokenGraphics/diamond-gl/blob/master/include/diamond/state.hpp).

You have this `_blend` type, and you create a global, non-constant `blend` object. Why? What advantages does this offer over simply having those be namespace-scoped functions? None of those functions modify the members of the `_blend` object. In fact, there are no members of the `_blend` object. So what's the point of them being in an object? If you want to group those functions together, then use a namespace, or static members.

The user gets zero benefit from calling `blend.func` vs. `blend::func`.

Also, there are a number of places where you don't take advantage of obvious avenues for C++-ification of the OpenGL API. For example, you don't use `enum class` where appropriate. Admittedly, `gl.xml` doesn't make it easy to extract out the appropriate enumerators for every function (though it's much better now than it used to be). But any good C++ wrapper of OpenGL ought to at least make an attempt to support this.

And then there's just some very unfortunate design choices.

First, `uniform::set` takes `vector<T>`. That's wonderful... for people who store their data in `vector`s. For everyone else, it's a terrible API. They now have to copy that data to a `vector` before they can pass it to you. Which means every time you send data to a uniform, you have to allocate memory that will be almost immediately deleted.

A pointer and a size would be an interface that anyone could use, regardless of their container of choice. Or even better, the GSL type `span`.

Oh, and FYI: if you want to test if a type is the same as another type, you use `std::is_same` (http://en.cppreference.com/w/cpp/types/is_same); you don't use `typeid` unless you need to make a runtime determination.

Then, there's your `base` type (https://github.com/AwokenGraphics/diamond-gl/blob/master/include/diamond/opengl.hpp). This is a type whose sole purpose is to allocate storage for OpenGL objects. Ignoring the bug you have in it (not deallocating memory (https://github.com/AwokenGraphics/diamond-gl/issues/1)), you derive from this class a lot. Publicly.

But you never use it as a public base class. You aren't using inheritance for polymorphism, since `base` doesn't have any virtual functions. You treat it like a member variable. So... why isn't it a member variable? Why are you inheriting from it?

You also use it in a lot of places where it is quite frankly unnecessary and needlessly performance-unfriendly. Consider `shader` and `program`. `base` has the ability to allocate an array of objects; you tell it how many to allocate. And yet, `shader` and `program` both... only ever allocate one such object. In fact, OpenGL doesn't even allow you to allocate an array of shader/program objects. So neither `shader` nor `program` needs dynamic allocation at all; it just needs to store a `GLuint`.

This is even further abused with the `_mode` class (https://github.com/AwokenGraphics/diamond-gl/blob/master/include/diamond/command.hpp). There, you're not even using the dynamic allocation to store an OpenGL object; you're just storing an integer. So why are you using `base` for this?

I haven't done a detailed look through this, but thus far, I cannot say that I find many of the choices made in the design of this system to be particularly good.

capitalknew
08-28-2017, 07:37 PM
About state functions (blend, etc.). I reserved for future multi-context purposes.
About shaders, programs, I tried to reduce this gap. Mainly allocation doing for a single (base) variable. But when I just put the address of the program, it removing by GC.
I replaced typeid to is_same.

About "why I use pointers"? I reserved for multiple object creation. It doing for tuples.

Alfonse Reinheart
08-29-2017, 09:10 AM
About "why I use pointers"? I reserved for multiple object creation. It doing for tuples.

My point is that `glCreateShader/Program` cannot do multiple object creation. Those functions create a single object at a time. So it makes no sense for your shader and program types to support something that OpenGL does not.

capitalknew
08-30-2017, 10:38 AM
I know. I tried to make combined method for allocations. I used smart pointers for GLuint object.

Alfonse Reinheart
08-30-2017, 11:15 AM
I know. I tried to make combined method for allocations. I used smart pointers for GLuint object.

Yeah, I saw that. You've fixed the memory leak, but that doesn't fix the defects in your overall design.

Your design with this `base` class is just... pointless. Dynamically allocating a `GLuint` serves no purpose. There's no point in having a class whose only purpose is to dynamically allocate a `GLuint`. At least before, when it was possible that you could allocate an array of `GLuint`s, `base` made sense. But once you took that away, `base` stopped having a purpose.

It would be far better to just give every derived class a `GLuint` member. And then not have them derive from `base` at all.

Also, every one of those classes should be non-copyable. And they should have a move constructor whose job it is to manage ownership of their object (blanking out the original as it copies to the new).

This is a far superior mechanism for doing what you're attempting:



template<class ObjectBuilder>
class opengl_object
{
private:
GLuint obj_ = 0;

public:
opengl_object() = default;
opengl_object()
{
reset();
}

//non-copyable
opengl_object(const opengl_object&) = delete;
opengl_object& operator=(const opengl_object&) = delete;

//moveable
opengl_object(opengl_object &&other)
: obj_(other.release())
{
}

opengl_object& operator=(opengl_object &&other)
{
reset(other.release());
}

template<typename ...Args>
void construct(Args ...&&args)
{
reset(ObjectBuilder::create_object(std::forward<Args>(args)...));
}

GLuint get() const {return obj_;}

GLuint release()
{
auto ret = obj_;
obj_ = 0;
return ret;
}

void reset(GLuint new_obj = 0)
{
if(obj_)
ObjectBuilder::delete_object(obj_);
obj_ = new_obj;
}
};


This class does no dynamic allocation. It has absolutely zero overhead compared to a `GLuint` object. The class exists solely to manage the object. Its destructor ensures that, if it currently manages a real OpenGL object, the object will be destroyed.

The 'ObjectBuilder` template parameter is a type that provides static methods to create and destroy OpenGL objects of a particular kind. So you would have a `texture_builder`, `buffer_builder`, `shader_builder`, etc. Note that `opengl_object::construct` is a variadic function; this allows `create_object` to have parameters, which is vital for calling `glCreateTextures`. Here are some examples:



struct texture_builder
{
static GLuint create_object(GLenum target)
{
GLuint ret;
glCreateTextures(target, &ret, 1);
return ret;
}

static void delete_object(GLuint obj)
{
glDeleteBuffers(&obj, 1);
}
};

struct buffer_builder
{
static GLuint create_object()
{
GLuint ret;
glCreateBuffers(&ret, 1);
return ret;
}

static void delete_object(GLuint obj)
{
glDeleteBuffers(&obj, 1);
}
};


So, a texture type would have a member of type `opengl_object<texture_builder>`. Note that it should not derive from this class; it should just have it as a member. You can fetch the OpenGL object with the `get` method.

capitalknew
08-30-2017, 01:05 PM
I have operator GLuint for return gl object.
Also, I designed for multiply creation of buffers, multi-bind, and linking. If create vectors need count reference (if make movable only as you said, gl buffer deletion can be just blocked, because moved).
Main question - how to be with sub-objects, where have link with original object? (it is uniform, VAO, texture_level)

I remedy design.

Alfonse Reinheart
08-30-2017, 05:00 PM
Also, I designed for multiply creation of buffers, multi-bind, and linking.

But the `shared_ptr` design you had before wouldn't work with any of that. You had no facility for allocating an array of objects via `shared_ptr`. Or at least, I didn't see it. So how would multiple creation, multi-bind, and so forth work?


If create vectors need count reference (if make movable only as you said, gl buffer deletion can be just blocked, because moved).

I don't know what you mean by that.


Main question - how to be with sub-objects, where have link with original object? (it is uniform, VAO, texture_level)

Outside of VAOs... why do those need to be objects at all? You can't use them independently of their owner, so why do you need them to be objects?

I prefer to follow the C++ axiom: do not pay for what you don't use. You shouldn't be forced into shared ownership overhead, just because you want to upload to a mipmap level of a texture. Wrappers like you're writing should minimize their overhead; you should never give someone a reason to avoid using your wrapper.

capitalknew
09-10-2017, 11:57 AM
The project is not completely abandoned. It's just that now it's not very relevant for me. I do not really want to wrap myself personally. But I would like to realize my concept. I even know how I could do it differently (via C ++ map). On the one hand, I plan to rewrite the project, but this should not prevent the implementation of the wrapper as a whole. Just now, all my forces are thrown on another project - ray tracing in real time (though OpenGL, but without the wrapper).