Twinklebear / embree-rs

Rust bindings to Embree 3
MIT License
35 stars 14 forks source link

Apply align() to all structures needed #1

Closed beltegeuse closed 6 years ago

beltegeuse commented 6 years ago

Hi Will, I saw that you had updated your Embree wrapper a few days ago. Thanks for your work :) This is a pull request to put align with other Embree structures. Of course, it is a temporary fix; normally this alignment will need to be handled automatically by rust-bindgen.

Note that I have added:

![feature(attr_literals)]

![feature(repr_align)]

To lib.rs as I have used nightly to build your code. Please remove them if you think it is unnecessary. Cheers, Adrien

Twinklebear commented 6 years ago

Looks good, thanks Adrien! It looks like the attr literals and repr align are still unstable, so are needed.

Is there a tracking issue on the bindgen repo for alignment? I found one mentioning supporting #pragma pack but not alignment. As a temporary fix I was planning to add some additional text replacement to generate-sys-bindings.sh to add the alignment fields the next time it's re-generated.

Edit Actually, Rust 1.26 nightly tells me repr_align was stabilized in 1.25, attr_literals are still unstable though.

beltegeuse commented 6 years ago

Is there a tracking issue on the bindgen repo for alignment?

I have opened an issue about "repr_align" on rust-bindgen last week. Indeed, when I have tested rust-bindgen, the current version was not able to generate proper alignment (doesn't add "align" and do not do proper add testing code). The primary maintainer has fixed some of the rust-bindgen code, but I did not tried yet. I plan to investigate more this issue next week. I will keep you updated :)

Cheers, Adrien

Twinklebear commented 6 years ago

It looks like they've added support in https://github.com/rust-lang-nursery/rust-bindgen/commit/fa1245198a0c61c13c0a8e6e02d431efe25ba0c9 , so it may be in now actually. I'm not sure when they'll put out another release, I've just been using it through cargo currently.