AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.74k stars 430 forks source link

Adsk Contrib - Add cmake option to turn off half-float lookup table in Imath #1982

Closed cozdas closed 1 month ago

cozdas commented 1 month ago

Adding OCIO_USE_HALF_LOOKUP_TABLE cmake option. If set to OFF, it adds IMATH_HALF_NO_LOOKUP_TABLE compiler definition which in return turns off the look up table that's used for half-float to float conversion, shaving off 256kB of binary size and run time footprint.

Please see half.h for the details of the half->single conversion table: https://github.com/AcademySoftwareFoundation/Imath/blob/main/src/Imath/half.h#L116-L152

KevinJW commented 1 month ago

My comments:

  1. The positive vs negative wording of the option when passed to IMATH
  2. Should the OCIO option name also indicate IMATH in the name
  3. Do we see changing the default to False at some point?

I'm thinking that if we see keeping the table as on by default and then asking to disable it when they want to, we may be better with it being something more like OCIO_DISABLE_IMATH_HALF_LOOKUP_TABLE set to false.

If conversely in the longer term we think we'd disable it by default, then the name is probably better phrased in the positive (as proposed).

doug-walker commented 1 month ago

@KevinJW , thanks for the feedback. I had thought that this option may have some general utility, but after further discussion, Cuneyt and I decided we will keep this change confined to the nanoColor branch for now. So I'm closing this PR.