Skip to content

Reduce Imath footprint even more #3332

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
Feb 10, 2022

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Feb 10, 2022

There was that one pesky place where we were still including Imath
headers, from simd.h in the case where OIIO_SIMD==0 and we wanted to
fall back on some Imath::Matrix methods.

Ugh, but this backfired because when including this file from Cuda,
OIIO_SIMD==0 and it included the header. But what happens if a
downstream app (a) includes this file from Cuda, and (b) uses a
DIFFERENT Imath (so names clash) or doesn't want to use Imath at all?

So one more change:

  • Remove the include of Imath.h from simd.h entirely.

  • Most places where we fell back on the Imath::Matrix methods, just
    replace it with scalar code, it's not complicated. (For some technical
    reasons, this required some minor surgery on the matrix44 class, but
    not anything that changes anything from a caller's point of view.)

  • The one complicated case is matrix44::inverse(). Punt! Just let that
    only be implemented if either (a) there is SIMD available, or (b)
    there is Imath available (i.e., included by the downstream code
    prior to this header). That's it. If you want to invert a 4x4 matrix
    without SIMD, you need to arrange inclusion of the correct
    ImathMatrix.h yourself before including this header.

There was that one pesky place where we were still including Imath
headers, from simd.h in the case where OIIO_SIMD==0 and we wanted to
fall back on some Imath::Matrix methods.

Ugh, but this backfired because when including this file from Cuda,
OIIO_SIMD==0 and it included the header. But what happens if a
downstream app (a) includes this file from Cuda, and (b) uses a
DIFFERENT Imath (so names clash) or doesn't want to use Imath at all?

So one more change:

* Remove the include of Imath.h from simd.h entirely.

* Most places where we fell back on the Imath::Matrix methods, just
  replace it with scalar code, it's not complicated. (For some technical
  reasons, this required some minor surgery on the matrix44 class, but
  not anything that changes anything from a caller's point of view.)

* The one complicated case is matrix44::inverse(). Punt! Just let that
  only be implemented if either (a) there is SIMD available, or (b)
  there is Imath available (i.e., included by the downstream code
  prior to this header). That's it. If you want to invert a 4x4 matrix
  without SIMD, you need to arrange inclusion of the correct
  ImathMatrix.h yourself before including this header.
@lgritz lgritz merged commit a07cb91 into AcademySoftwareFoundation:master Feb 10, 2022
@lgritz lgritz deleted the lg-imathh branch February 10, 2022 22:10
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