Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: Could this be used as IDL? #9

Closed
bkaradzic opened this Issue Feb 21, 2019 · 30 comments

Comments

Projects
None yet
3 participants
@bkaradzic
Copy link
Contributor

bkaradzic commented Feb 21, 2019

I've been looking for some IDL (Interface Description Language) solution to generate C/C++ interface from common definition. This project got my attention since it's simple, and it supports Lua (matches build system I'm using).

I started this IDL in Go language, but I'm not happy with not being easily scriptable.

This is my current half-done solution:

	{
		"VertexBufferHandle", "createVertexBuffer", "create_vertex_buffer",
		[]Arg{
			{"const Memory *", "_mem"},
			{"const VertexDecl &", "_decl"},
			{"uint16_t", "_flags"},
		},
	},

	{
		"void", "destroy", "destroy_vertex_buffer",
		[]Arg{
			{"VertexBufferHandle", "_handle"},
		},
	},

Idea is that this IDL would then generate following functions:

C++:

VertexBufferHandle createVertexBuffer(const Memory* _mem, const VertexDecl& _decl, uint16_t _flags)
{
	BX_CHECK(NULL != _mem, "_mem can't be NULL");
	BX_CHECK(isValid(_decl), "Invalid VertexDecl.");
	return s_ctx->createVertexBuffer(_mem, _decl, _flags);
}

void destroy(VertexBufferHandle _handle)
{
	s_ctx->destroyVertexBuffer(_handle);
}

C99:

BGFX_C_API bgfx_vertex_buffer_handle_t bgfx_create_vertex_buffer(const bgfx_memory_t* _mem, const bgfx_vertex_decl_t* _decl, uint16_t _flags)
{
	const bgfx::VertexDecl& decl = *(const bgfx::VertexDecl*)_decl;
	union { bgfx_vertex_buffer_handle_t c; bgfx::VertexBufferHandle cpp; } handle;
	handle.cpp = bgfx::createVertexBuffer( (const bgfx::Memory*)_mem, decl, _flags);
	return handle.c;
}

BGFX_C_API void bgfx_destroy_vertex_buffer(bgfx_vertex_buffer_handle_t _handle)
{
	union { bgfx_vertex_buffer_handle_t c; bgfx::VertexBufferHandle cpp; } handle = { _handle };
	bgfx::destroy(handle.cpp);
}

And C99 dynamic lib which would call function via bgfx_interface_vtbl_t, and C++ dynamic lib which would call C99 dynamic lib function in the same way as above example calls C++ from C99 function wrapper.

Assumption here is that there would be one function body implemented somewhere (like one from C++ snippet above), but the rest function bodies would be generated automatically from IDL.

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 22, 2019

It's an interesting idea.

But sproto is not design for that purpose, so using it as a IDL may not easy to maintain. I suggest to design a IDL syntax by lua like premake/GENie does.

For example,

typedef "uint16_t"
typedef.Memory "bgfx_memory_t"
typedef.VertexDecl "bgfx_vertex_decl_t"

 -- cname is optional, maybe we can generate it by the rule
func.createVertexBuffer { cname = "bgfx_create_vertex_buffer" } 
	"VertexBufferHandle"  -- return type
	._mem "const Memory *"  -- args
	._decl "const VertexDecl &"
	._flags "uint16_t"

I can write a lua module to support above IDL like script this weekend.

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 23, 2019

Ah interesting. Yeah ideally this would just work with GENie as is.

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 23, 2019

https://github.com/cloudwu/bgfxidl

I wrote a small module for this. I hope you will like it. @bkaradzic

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 23, 2019

I'm going to try it now! Thanks! :)

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 23, 2019

It's excellent. The only thing I had to add is if v ~= "" then here:

local function camelcase_to_underscorecase(name)
	local tmp = {}
	for v in name:gmatch "%u?[%l%d]*" do
		if v ~= "" then
			tmp[#tmp+1] = v:lower()
		end
	end
	return table.concat(tmp, "_")
end

For some reason gmatch adds empty string at the end causing function to generate underscore at the end.

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 24, 2019

Which lua version do you use? I didn't have this issue with lua 5.3.5 , and it seems that the behavior of string.gmatch is different in some old lua versions.

Pattern "%u*[%l%d]+" would be better , it captures at least one character. But it can't capture the full uppercase words like "UPPER".

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 24, 2019

I'm just using it with GENie directly as a test (5.3.0) genie --file=test.lua vs2017, since I intend to be just option there to generate bgfx API files.

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 24, 2019

How this is going to work?

BGFX_C_API void bgfx_vertex_decl_begin(bgfx_vertex_decl_t* _decl, bgfx_renderer_type_t _renderer)
{
	bgfx::VertexDecl* decl = (bgfx::VertexDecl*)_decl;
	decl->begin(bgfx::RendererType::Enum(_renderer) );
}
@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 24, 2019

Another issue:

typedef "Attrib::Enum" outputs bgfx_attrib_enum_t, but ideally it would be bgfx_attrib_t. Using typedef.Attrib::Enum has issue with ::.

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 24, 2019

vargs like ... are also issue for: bgfx_dbg_text_printf

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 24, 2019

Since foo.bar is equivalent to foo["bar"] in lua , you can write

typedef["Attrib::Enum"] "bgfx_attrib_t"

And you can also use this format :

typedef "Attrib::Enum" "bgfx_attrib_t"

For the vararg functions , I suggest add an attribute "..." to the function , just like ,

func.dbgTextPrintf { "..." , vararg = "dbgTextPrintfVargs" } 
	"void"
	._x "uint16_t"
	._y "uint16_t"
	._attr "uint16_t"
	._format "const char *"

"..." may be omitted, because it always need a vararg function for binding.

btw, I don't think all the c99 functions can be generated by the rule, can we need add an attribute for special case like bgfx_init ?

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 24, 2019

I add vararg support in a branch 6dbc23b

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 24, 2019

How this is going to work?

BGFX_C_API void bgfx_vertex_decl_begin(bgfx_vertex_decl_t* _decl, bgfx_renderer_type_t _renderer)
{
	bgfx::VertexDecl* decl = (bgfx::VertexDecl*)_decl;
	decl->begin(bgfx::RendererType::Enum(_renderer) );
}

How about this

func.begin { memfunc = "_decl" , cname = "bgfx_vertex_decl_begin" }
  "void",
  ._decl "VertexDecl *",
  ._renderer "RendererType::Enum"

or

func.begin { "memfunc" , cname = "bgfx_vertex_decl_begin" }  -- use first arg as this
  "void",
  ._decl "VertexDecl *",
  ._renderer "RendererType::Enum"
@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 24, 2019

memfunc.begin { cname...

or

func.vertexDeclBegin where I have some mechanism write function body would be fine, cfunc similar to cname.

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 25, 2019

I support class member function just now, and I will add cfunc later.

btw, for the output reference like attrib of VertexDecl::decode , I generate code

        const bgfx::VertexDecl* This = (const bgfx::VertexDecl*)_this;
        bgfx::AttribType::Enum & type = *(bgfx::AttribType::Enum *)_type;
        This->decode((bgfx::Attrib::Enum)_attrib, *_num, type, *_normalized, *_asInt);

But bgfx is

	bgfx::AttribType::Enum type;
	const bgfx::VertexDecl* decl = (const bgfx::VertexDecl*)_decl;
	decl->decode(
		  bgfx::Attrib::Enum(_attrib)
		, *_num
		, type
		, *_normalized
		, *_asInt
		);
	*_type = (bgfx_attrib_type_t)type;

I think *_type = (bgfx_attrib_type_t)type; is better ,but the code generator can't recognize which arg is for output. Maybe we should mark the output in IDL (like COM does) ?

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 25, 2019

Maybe we should mark the output in IDL (like COM does) ?

Sure.

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 25, 2019

How this is going to work?

BGFX_C_API void bgfx_vertex_decl_begin(bgfx_vertex_decl_t* _decl, bgfx_renderer_type_t _renderer)
{
	bgfx::VertexDecl* decl = (bgfx::VertexDecl*)_decl;
	decl->begin(bgfx::RendererType::Enum(_renderer) );
}

I introduced a new mechanism ,

func.VertexDecl.begin
	"void"
	.renderer        "RendererType::Enum"

func.VertexDecl.decode { const }
	"void"
	.attrib          "Attrib::Enum"
	.num             "uint8_t &"          { out }
	.type            "AttribType::Enum &" { out }
	.normalized      "bool &"             { out }
	.asInt           "bool &"             { out }

c api would be

BGFX_C_API void bgfx_vertex_decl_add(bgfx_vertex_decl_t* _this, bgfx_attrib_t _attrib, uint8_t _num, bgfx_attrib_type_t _type, bool _normalized, bool _asInt)
{
        bgfx::VertexDecl* This = (bgfx::VertexDecl*)_this;
        This->add((bgfx::Attrib::Enum)_attrib, _num, (bgfx::AttribType::Enum)_type, _normalized, _asInt);

}

BGFX_C_API void bgfx_vertex_decl_decode(const bgfx_vertex_decl_t* _this, bgfx_attrib_t _attrib, uint8_t * _num, bgfx_attrib_type_t * _type, bool * _normalized, bool * _asInt)
{
        const bgfx::VertexDecl* This = (const bgfx::VertexDecl*)_this;
        bgfx::AttribType::Enum type;
        This->decode((bgfx::Attrib::Enum)_attrib, *_num, &type, *_normalized, *_asInt);
        *_type = (bgfx_attrib_type_t)type;
}
@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 25, 2019

cfunc added.

8da94a5

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 25, 2019

@bkaradzic Maybe we can use this IDL to generate binding codes, for example, python binding ( @jnadro )

https://github.com/jnadro/pybgfx/blob/master/pybgfx/bgfx.py

@jnadro

This comment has been minimized.

Copy link

jnadro commented Feb 25, 2019

@cloudwu Yes, the IDL you created would be great to auto generate the python bindings. The ones I made were half by hand, and half by a python script that parsed the bgfx header file. Having those would of made my life much easier!

It would certainty make pybgfx easier to maintain going forward when changes are made to the header.

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 25, 2019

It's good idea. But it would require defines.h and all enums to also be moved here. But then in that case I would like to generate C++ header too. :)

I would wait with this python bindings just a bit for now, until we get original requirements fully implemented and integrated into bgfx repo.

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 26, 2019

I thought { "out" } is missing here, but actually it exist, just conversion from type fails, since it should be & when calling C++:

BGFX_C_API void bgfx_vertex_decl_decode(const bgfx_vertex_decl_t* _this, bgfx_attrib_t _attrib, uint8_t * _num, bgfx_attrib_type_t * _type, bool * _normalized, bool * _asInt)
{
	const bgfx::VertexDecl* This = (const bgfx::VertexDecl*)_this;
	bgfx::AttribType::Enum type;
	This->decode((bgfx::Attrib::Enum)_attrib, *_num, &type, *_normalized, *_asInt);
	*_type = (bgfx_attrib_type_t)type;
}
@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 26, 2019

My fault, this may fix it 029f77c

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 26, 2019

I start annotating outputs, but getting "Always use & for output", for example in one case void * dst

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 26, 2019

ok, I'll fix it.

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 26, 2019

Could you show me an example ? Which api with void * dst ?

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 26, 2019

See: #5

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 26, 2019

@bkaradzic

This comment has been minimized.

Copy link
Contributor Author

bkaradzic commented Feb 26, 2019

Another issue, see getShaderUniforms, something with returning UniformHandle *: #6

@cloudwu

This comment has been minimized.

Copy link
Owner

cloudwu commented Feb 26, 2019

@bkaradzic bkaradzic closed this Feb 26, 2019

@cloudwu cloudwu transferred this issue from cloudwu/sproto Feb 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.