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

support wayland and x11 EGL by use of versions #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rtbo
Copy link

@rtbo rtbo commented Feb 12, 2017

In addition, it is proposed to expose void* type and request client to cast.
Rationale:
This avoids to pull-in dependencies, which would be wrong IMO as there are multiple bindings to x11 and derelict should not prefer one above another. Besides, dependencies version clash is easy to generate.
Having opaque struct declarations is hurting more than helping because:

  • casting is needed anyway due to different packages
  • it brings name conflicts in client code

Tested example of derelict-gles client with EGL, GLES2 and wayland can be found here:
https://github.com/rtbo/wayland-d/blob/master/examples/egl_window/source/egl_window.d

With dub, selecting wayland or x11 is done by clients like this:

	"subConfigurations": {
		"derelict-gles": "wayland"
	}

It defaults to x11 on linux, and should be transparent on other platforms (though I did not test that).

In addition, it is proposed to expose void* type and request client to cast.
This avoids to pull-in dependencies.
Having an opaque struct declaration will not help as casting would be needed anyway.
@mdparker
Copy link
Member

This binding needs a lot of love, but it's a low priority for me right now. I do not use GLES and know very little about EGL and its usage on different platforms. I intend to tackle it if I can ever make the time to finish up all the other work that needs doing. I'll keep this open until I'm able to spend the time I need with this.

it is proposed to expose void* type and request client to cast.

I strongly disagree with this. Even in C, I don't like this approach. Opaque structs are more typesafe and avoid the need for casts. I can't think of any advantage of using void* instead.

@rtbo
Copy link
Author

rtbo commented Feb 13, 2017

I strongly disagree with this. Even in C, I don't like this approach. Opaque structs are more typesafe and avoid the need for casts. I can't think of any advantage of using void* instead.

And so? You'd prefer to pull-in dependencies? We'd need one version per bindings project. Not so many after all, to my knowledge, there is one binding project for wayland and 2 for X11.

@mdparker
Copy link
Member

No. Opaque structs shouldn't pull in external dependencies if they're declared properly as extern(C) everywhere (which they currently aren't in DerelictGLES). Then they simply become forward references.

That extern(C) bit is key, though. If a given binding doesn't use it, then D mangling makes them different types. This is one of the things I'll have to evaluate when I get around to getting this package up to speed. My preference is to eliminate the void* where practical.

@rtbo
Copy link
Author

rtbo commented Feb 13, 2017

I can't find any reference specifying that extern(C) is making a struct the same type when declared in different modules. I know it works for function, but I'm not aware of any mangling for structs. AFAIK the struct name only exists in compilers memory.

In fact it doesn't work.
I've declared wl_display as extern extern(C) struct wl_display; (also tried other patterns) in both derelict.gles.eglplatform and wayland.native.client and I still get the same error:

Error: function pointer eglGetDisplay (wl_display*) is not callable using argument types (wl_
display*)

@mdparker
Copy link
Member

You don't need the extra extern. That's only used for instances of a type, not type declarations. extern(C) struct wl_display should be all that's needed.

extern(C) affects any symbol, not just functions. D symbols are mangled with the module and package name. extern(C) turns that off. With functions, it also affects the calling convention.

Finally, I just did a test to show you that it does indeed work only to discover that it doesn't. That, IMO, is a bug. I'll take it to the forums later to see if others agree with me.

@mdparker
Copy link
Member

OK, the first example I put together was flawed. This one works.

module ec1;
extern(C):
    struct Foo { int x; }
    Foo* newFoo(int x) { return new Foo(x); }
    void printFoo(Foo* f) {
        import std.stdio;
        writeln(f.x);
    }
module ec3;
extern(C) {
    struct Foo;
    Foo* newFoo(int);
    void printFoo(Foo*);
}
void main() {
    Foo* f = newFoo(10);
    printFoo(f);
}

Compiling with dmd ec3 ec1 and executing will print 10 as expected.

@rtbo
Copy link
Author

rtbo commented Feb 14, 2017

Your example works because module ec3 doesn't know anything about ec1.
The problem happens when having a app importing both (i.e. an app using derelict-gles and wayland-d). In such case it is not possible to make the two modules work together.
To elaborate on your example:

module ec1;
extern(C):
    struct Foo { int x; }
    Foo* newFoo(int x) { return new Foo(x); }
    void printFoo(Foo* f) {
        import std.stdio;
        writeln(f.x);
    }
module ec3;
extern(C) {
    struct Foo;
    void printFoo(Foo*);
}
void callPrintFoo(Foo* foo)
{
    printFoo(foo);
}
module app;
import ec1;
import ec3;

void main()
{
    auto foo = newFoo(5);
    printFoo(foo);
    callPrintFoo(foo);
}
dmd app ec1 ec3
app.d(9): Error: function ec3.callPrintFoo (Foo* foo) is not callable using argument types (Foo*)

read: ec3.callPrintFoo (ec3.Foo* foo) is not callable using argument types (ec1.Foo*)

@mdparker
Copy link
Member

Alright. I was in a rush the other day and didn't put much time into this, but now I've been playing around with it. My first "flawed" test used three modules before I rewrote it (which is why there's no ec2 in the test I posted). Turns out the flaw wasn't what I thought it was. Had I spent more time with it, I would have recognized it.

TL;DR extern(C) is not turning off mangling of type declarations. As shown here:

extern(C):
    struct Foo {};
    void bar() {}
    int x;

pragma(msg, Foo.mangleof);
pragma(msg, bar.mangleof);
pragma(msg, x.mangleof);

Obviously, types have no linkage, but even so I know I'm not the only one who understood that extern(C) would turn off mangling of Foo.

My original "flawed" test:

module ec1;
extern(C) struct Foo {};
module ec2;
extern(C) struct Foo { int x; }
Foo* newFoo(int x) { return new Foo(x); }
void printFoo(Foo* f) { import std.stdio; writeln(f.x); }
module ec3;
void main() {
    import ec1;
    import ec2;
    Foo* f = newFoo(30);
    printFoo(f);
}

This produces the following errors:

ec3.d(5): Error: ec1.Foo at ec1.d(2) conflicts with ec2.Foo at ec2.d(2)
ec3.d(5): Error: cannot implicitly convert expression (newFoo(30)) of type Foo* to Foo*
ec3.d(6): Error: function ec2.printFoo (Foo* f) is not callable using argument types (Foo*)

Notice the braces on Foo in ec1. It's a full type, not a forward declaration. When I finally noticed it, I believed that to be the cause. I would expect a conflict error in that case, even when mangling is turned off. And we see that in the first line. After I rewrote it, I noticed the braces and thought that had been the cause of the error.

What I failed to make note of was that the error message explicitly shows the mangled names (ec1.Foo and ec2.Foo). And if I had taken the time to revert back to this test and try again without the braces, I would have found the same exact error messages.

I really believe extern(C) should be turning off mangling even for type declarations. Otherwise, forward references aren't useful outside of the specific case of binding to a C library (like GLFW). I'll take this to the forums and see what others think.

@mdparker
Copy link
Member

@mdparker
Copy link
Member

Alright. So in that thread Walter helped shed light on my misunderstanding of the compiler internals. void* it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants