Skip to content

Hide Imath headers and types from most of the public API #3301

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

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Jan 24, 2022

This patch sets out to minimize the extent to which Imath is a
necessary source dependency for using OIIO C++ header files and
APIs. The major changes are:

  • Remove the unconditional #include <OpenImageIO/Imath.h> from
    fmath.h and simd.h. Those forced Imath headers on nearly all all
    code that uses OIIO's public APIs, whether or not they needed the
    very few calls where Imath types are used as parameters.

  • Make sure all public OIIO non-virtual methods and freestanding
    functions that have Imath types as parameters are enclosed in header
    guards like #ifdef INCLUDE_IMATHVECTOR_H. Downstream client code
    needs to include Imath headers before including OIIO headers, if
    they want to call those few particular functions.

This means that client applications who don't need those particular
functions, but call other parts of the OIIO APIs, don't need to
#include any Imath headers (or pay the extra compilation time for
doing so), and don't need any knowledge of which specific version of
Imath is used by the OIIO they are using.

The small number of public API calls that will no longer be visible
downstream unless the client code also include Imath headers (before
the OIIO headers) are:

  • color.h: ColorConfig::createMatrixTransform (needs ImathColor.h)

  • fmath.h: type conversions involving half (needs half.h).

  • imagebufalgo.h: warp(), colormatrixtransform() (needs ImathMatrix.h)

  • simd.h: conversions between our SIMD types and Imath types (Vec3,
    Matrix33, Matrix44) and vtransform() calls involving those types
    (requires ImathVec.h and ImathMatrix.h), and SIMD load/store
    involving half data (requires half.h).

These changes necessitated adding including of Imath.h in a number of
internal .cpp files, the ones where those types are actually used.

The following header files do still unconditionally reference Imath
types or headers:

  • texture.h: Some virtual methods of TextureSystem use Imath types
    as parameters.
  • imagebufalgo_util.h: For half support. We use this a lot
    internally, but presume most client code never uses this header.
  • simd.h, ONLY IF USE_SIMD=0 (which is generally only used for
    development debugging and if porting to obscure platforms that are
    neither x86 nor ARM).

The bottom line is that the vast majority of downstream callers of OIIO
APIs who need ImageInput/ImageOutput, ImageBuf, ImageBufAlgo, and
generally everything in the OpenImageIO_Util library, do not need to
use Imath types, include Imath headers, or even be aware of which Imath
release OIIO is using.

Why would we do all this?

Because this paves the way for optionally building OIIO to link against
static libraries for OpenEXR and Imath, essentially embedding and
completely hiding an OpenEXR/Imath inside libOpenImageIO.

That allows libOpenImageIO to use the very latest Imath (3.1, say,
with the new faster core API), even though that libOpenImageIO is
being consumed by an application that for whatever reason is forced to
use an entirely different (older) OpenEXR or Imath library. In other
words, it lets OIIO use OpenEXR/Imath fully embedded without
transitively passing that OpenEXR version as a downstream dependency
(except for the few use cases that need to call the tiny number of
OIIO API methods that require Imath types.

There is a small chance that client code that use Imath types and
SHOULD have been includeing Imath headers themselves actually omitted
those includes and dependd on the fact that certain OIIO headers
included them. Now that they don't, it may require the client-side
code to add those includes. I'm going to claim that this is a "bug" in
the client code (they didn't include a header that defined a type they
used), and not a breaking incompatibility in OIIO's public calls
themselves. Thus, I feel comfortable with these changes being in OIIO
2.4 (today's master) and not necessitating a 3.0 release (in which
arbitrary loss of backward compatibility would be allowed). So watch
out for the occasional need for OIIO-calling code to need to add an
#include for Imath types the code was always using, but neglected
to explicitly include.

This patch sets out to minimize the extent to which Imath is a
necessary source dependency for using OIIO C++ header files and
APIs. The major changes are:

* Remove the unconditional `#include <OpenImageIO/Imath.h>` from
  fmath.h and simd.h. Those forced Imath headers on nearly all all
  code that uses OIIO's public APIs, whether or not they needed the
  very few calls where Imath types are used as parameters.

* Make sure all public OIIO non-virtual methods and freestanding
  functions that have Imath types as parameters are enclosed in header
  guards like `#ifdef INCLUDE_IMATHVECTOR_H`. Downstream client code
  needs to include Imath headers before including OIIO headers, if
  they want to call those few particular functions.

This means that client applications who don't need those particular
functions, but call other parts of the OIIO APIs, don't need to
`#include` any Imath headers (or pay the extra compilation time for
doing so), and don't need any knowledge of which specific version of
Imath is used by the OIIO they are using.

The small number of public API calls that will no longer be visible
downstream unless the client code also include Imath headers (before
the OIIO headers) are:

  - color.h: ColorConfig::createMatrixTransform (needs ImathColor.h)

  - fmath.h: type conversions involving `half` (needs half.h).

  - imagebufalgo.h: warp(), colormatrixtransform() (needs ImathMatrix.h)

  - simd.h: conversions between our SIMD types and Imath types (Vec3,
    Matrix33, Matrix44) and vtransform() calls involving those types
    (requires ImathVec.h and ImathMatrix.h), and SIMD load/store
    involving `half` data (requires half.h).

These changes necessitated adding including of Imath.h in a number of
internal .cpp files, the ones where those types are actually used.

The following header files do still unconditionally reference Imath
types or headers:

  - texture.h: Some virtual methods of TextureSystem use Imath types
    as parameters.
  - imagebufalgo_util.h: For `half` support. We use this a lot
    internally, but presume most client code never uses this header.
  - simd.h, ONLY IF USE_SIMD=0 (which is generally only used for
    development debugging and if porting to obscure platforms that are
    neither x86 nor ARM).

The bottom line is that the vast majority of downstream callers of OIIO
APIs who need ImageInput/ImageOutput, ImageBuf, ImageBufAlgo, and
generally everything in the OpenImageIO_Util library, do not need to
use Imath types, include Imath headers, or even be aware of which Imath
release OIIO is using.

Why would we do all this?

Because this paves the way for optionally building OIIO to link against
static libraries for OpenEXR and Imath, essentially embedding and
completely hiding an OpenEXR/Imath inside libOpenImageIO.

That allows libOpenImageIO to use the very latest Imath (3.1, say,
with the new faster core API), even though that libOpenImageIO is
being consumed by an application that for whatever reason is forced to
use an entirely different (older) OpenEXR or Imath library.  In other
words, it lets OIIO use OpenEXR/Imath fully embedded without
transitively passing that OpenEXR version as a downstream dependency
(except for the few use cases that need to call the tiny number of
OIIO API methods that require Imath types.

There is a small chance that client code that use Imath types and
SHOULD have been includeing Imath headers themselves actually omitted
those includes and dependd on the fact that certain OIIO headers
included them.  Now that they don't, it may require the client-side
code to add those includes. I'm going to claim that this is a "bug" in
the client code (they didn't include a header that defined a type they
used), and not a breaking incompatibility in OIIO's public calls
themselves. Thus, I feel comfortable with these changes being in OIIO
2.4 (today's master) and not necessitating a 3.0 release (in which
arbitrary loss of backward compatibility would be allowed). So watch
out for the occasional need for OIIO-calling code to need to add an
`#include` for Imath types the code was always using, but neglected
to explicitly include.
@lgritz lgritz merged commit 71f0aa6 into AcademySoftwareFoundation:master Jan 28, 2022
@lgritz lgritz deleted the lg-hideimath branch January 30, 2022 16:28
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.

1 participant