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

Implement IDL based generators #447

Merged
merged 20 commits into from Apr 1, 2018

Conversation

@Diggsey
Copy link
Collaborator

commented Jan 25, 2018

Example generated bindings:
https://gist.github.com/Diggsey/c8a4cb93c7d5643d20203401ead178a7

This can't be merged until:

  1. I've tested it beyond the bindings compiling
  2. The next version of stdweb is released, which will contain the CanvasElement type
@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 28, 2018

I've just implemented a massive rework of the type-generation - it's now really powerful, and will generate borrowed versions of types for arguments and will generate generic methods when appropriate.

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 28, 2018

image

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Jan 29, 2018

Woo! This is looking great!

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Jan 29, 2018

I'm wondering if it might actually make sense to create a new crate instead under this repository, called webgl_generator?

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 29, 2018

Yeah, I don't really mind, is that what you'd prefer?

@tomaka

This comment has been minimized.

Copy link
Collaborator

commented Jan 29, 2018

I personally don't think it's worth creating a new repo.

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Jan 29, 2018

Wasn't talking about creating a new repo, just a new crate as a directory in this repo. If I'm not mistaken it seems like gl_generator and webgl_generator are orthogonal in their implementations and their dependencies?

@tomaka

This comment has been minimized.

Copy link
Collaborator

commented Jan 29, 2018

Oh sorry, yes that would make sense!

@edwin0cheng

This comment has been minimized.

Copy link

commented Jan 31, 2018

@Diggsey As i mentioned in koute/stdweb#92, will it works in Firefox ?

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 31, 2018

@edwin0cheng not right now - at least not if you try do anything complex. I think this can (and should) be fixed from the stdweb side though.

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Feb 2, 2018

Looking great so far! Let me know when this can progress forward!

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 4, 2018

@brendanzab I've realised that the IDL files are missing information on extensions - these are described via separate IDL embedded in XML in the Khronos WebGL repo.

I would like to expose a mapping from "<extension name>" => "<IDL contents>" in the khronos_api crate - this will involve a build step to extract the IDL from the XML and will need to be stored as a static array of type &'static [(&'static str, &'static str)]. Does that sound reasonable to you?

@Diggsey Diggsey force-pushed the Diggsey:webgl-gen branch from 0b42c8c to 643c5ac Feb 4, 2018

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 4, 2018

I've opened a separate PR to add extension support to khronos_api, and I've rebased this PR on top.

The generated bindings are now up to 10,000 lines, and include support for all accepted WebGL extensions. There's also a "version-less" GLContext type to make it easier to write code which can work with multiple versions.

I've uploaded the generated documentation as I think it's quite interesting to read:
http://awesome-davinci-d3197a.bitballoon.com/webgl_stdweb/index.html

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Feb 5, 2018

I would like to expose a mapping from "" => "" in the khronos_api crate - this will involve a build step to extract the IDL from the XML and will need to be stored as a static array of type &'static [(&'static str, &'static str)]. Does that sound reasonable to you?

Oh, missed this comment! Would have been helpful if you had have linked to it in the PR of #451! Yes, that sounds great.

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 5, 2018

One thing that might be cool would be to pull out the "summary" and "function" sections from the XML files and attach them as doc-comments to the generated code.

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Feb 5, 2018

Ooooh! Wondering if we could do that for all the generators 😮

@Diggsey Diggsey force-pushed the Diggsey:webgl-gen branch from 643c5ac to 43294b0 Feb 5, 2018

Diggsey added some commits Feb 26, 2018

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 26, 2018

@brendanzab stdweb 0.4.0 was released, and I've updated the PR to use that. It should be ready for you to review now!

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Feb 27, 2018

Woo, looking now!

&TypeKind::Primitive(ref p) => ProcessedResult::simple(p.name()),
&TypeKind::String => ProcessedResult::simple("String"),
&TypeKind::ArrayBuffer | &TypeKind::ArrayBufferView => ProcessedResult::simple("ArrayBuffer"),
&TypeKind::BufferSource => unimplemented!("BufferSource not supported in output"),

This comment has been minimized.

Copy link
@brendanzab

brendanzab Feb 27, 2018

Owner

Should we be using unimplemented! here? Are you planning to add support for this later?

This comment has been minimized.

Copy link
@Diggsey

Diggsey Mar 3, 2018

Author Collaborator

BufferSource is not currently used in the WebGL IDL files, so I haven't bothered implementing it.

}).collect();

let rust_args = args.iter().map(|a| a.arg.clone()).collect::<Vec<_>>().join(", ");
let js_args = args.iter().map(|a| a.js_arg.clone()).collect::<Vec<_>>().join(", ");

This comment has been minimized.

Copy link
@brendanzab

brendanzab Feb 27, 2018

Owner

itertools has join if you wanted to use that to skip the intermediate collect!

This comment has been minimized.

Copy link
@Diggsey

Diggsey Mar 3, 2018

Author Collaborator

Didn't seem worth it - this only happens at build time, and the cost of the extra dependency would outweigh any minor performance improvements.


// Strings
DOMString | USVString => TypeKind::String,
ByteString => unimplemented!(),

This comment has been minimized.

Copy link
@brendanzab

brendanzab Feb 27, 2018

Owner

Has this been forgotten?

This comment has been minimized.

Copy link
@Diggsey

Diggsey Mar 3, 2018

Author Collaborator

Just not needed for WebGL


Some((field.name, Field {
type_
}))

This comment has been minimized.

Copy link
@brendanzab

brendanzab Feb 27, 2018

Owner

Why does this need to be an Option?

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Feb 27, 2018

Would be nice to Run rustfmt on this if you don't mind!

Diggsey added some commits Mar 4, 2018

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 4, 2018

@Diggsey Diggsey referenced this pull request Mar 4, 2018

Closed

WebIDL binding generator #42

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 20, 2018

@brendanzab hey is this good to go?

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2018

Ack - left this one sit around in my notifications making me feel guilty for too long! Merging!

@brendanzab brendanzab merged commit 4f081d2 into brendanzab:master Apr 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Diggsey Diggsey deleted the Diggsey:webgl-gen branch Apr 1, 2018

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 1, 2018

Thanks! Are you happy to publish the new crate(s) to crates.io?

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2018

Yup, will do. I'll add you to the crate owners too.

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2018

What needs to be published - just the new crates?

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2018

Hum, when publishing I get:

error[E0425]: cannot find value `WEBGL_EXT_XML` in module `khronos_api`
  --> webgl_registry/mod.rs:69:38
   |
69 |         for &ext_xml in khronos_api::WEBGL_EXT_XML {
   |                                      ^^^^^^^^^^^^^ not found in `khronos_api`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0425`.
error: failed to verify package tarball

Caused by:
  Could not compile `webgl_generator`.
@brendanzab

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2018

Oh, I think I need to bump the khronos_api version 🤔

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2018

Oh, and probably also publish those changes

@Diggsey

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 1, 2018

Yes, I think so?

@brendanzab

This comment has been minimized.

Copy link
Owner

commented Apr 1, 2018

Done!

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