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

Initial support for C wrappers #56

Closed

Conversation

scott-wilson
Copy link

@scott-wilson scott-wilson commented Oct 25, 2020

Description

This is the intial C wrapper/reimplementation of Imath (where it makes sense). The general idea for this project is to create a C and eventually a Rust wrapper. At the moment, this is a minimal commit to start a discussion about how best to implement this, maintaining the C bindings, etc.

The current goals

  • Follow style standards set by Imath
  • Follow C style standards set by the OpenTimelineIO and OpenImageIO C wrappers (where it makes sense)
  • Make the C and the C++ types the same, byte for byte, and all functions produce the same result.
  • Create a minimal set of wrappers. For example, if there's helper functions and methods, then don't implement them unless they make sense to be implemented.

Reference C wrappers

Todo

  • Sign the CLA (Shouldn't be any issues with this, but double checking with company)
  • Finish implementing half
  • Implement the rest of Imath

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 25, 2020

CLA Check
The committers are authorized under a signed CLA.

Comment on lines 21 to 22
imath_half imath_half_createFromFloat (float f)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this not use the C++ version (casting on the way out or forcing a type pun)? It seems to me that we want to wrap the C++ functionality rather than repeat it. It's going to be much harder to maintain multiple copies of the same code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true. I want to try to balance speed with maintainability, so the plan was to reimplement some things where they were trivial. However, if we want to go the type punning route, then I can go for that.

src/CImath/chalf.h Outdated Show resolved Hide resolved
Copy link
Contributor

@lgritz lgritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest that the test program be C, not C++. Otherwise we aren't really verifying that it compiles and runs properly in a pure C context.

@scott-wilson
Copy link
Author

I would suggest that the test program be C, not C++. Otherwise we aren't really verifying that it compiles and runs properly in a pure C context.

That's a good point. I'll see what Anders did in oiio.

@cary-ilm
Copy link
Member

Closing this PR, as the plans for Rust have progressed separately.

@cary-ilm cary-ilm closed this Feb 12, 2022
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.

3 participants