search

AcademySoftwareFoundation / OpenColorIO

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

Remove Imath and Pystring from nanoColor #2002

Open doug-walker opened 4 months ago

doug-walker commented 4 months ago

One of the requirements for nanoColor is that it have no external dependencies. Most of the dependencies have already been removed, via optional CMake flags. However, code from Imath and Pystring was temporarily moved to the ext folder (i.e. the required code from those projects was copied in, strictly as a temporary prototyping measure).

For Pystring, the proposal is to reimplement any functions currently needed by OCIO (there are not many).

For Imath, the only thing that is used by OCIO is the half class. The proposal is that a half class be created specifically for OCIO, based on the Imath code but with some simplifications. Given that most hardware these days has specific instructions for half-floats, those would be used if available. For other machines, the code from Imath that does not use the LUT would be used.

For simplicity, these changes would apply to both nanoColor and full OCIO. If people would like to continue to be able to use the full Imath or Pystring as an optional dependency with full OCIO, please note that as a comment below.

lgritz commented 4 months ago

It's a PITA for people making apps to have three or four dependencies that each come with their own half type that are basically equivalent but require casting and other hoop jumping as data is passed around in the app. Might I suggest instead "vendoring" the Imath one rather than make a new half class? These days, Imath's is (or can be rigged to be) header only and with no tables.

If you need some support on the Imath side (maybe it's not a truly standalone single header, but could be made so?), we would be more than happy to help you or to accept a PR that fixes it so you can more easily vendor it.

doug-walker commented 4 months ago

Thanks for that feedback @lgritz. OCIO doesn't actually expose the half class to the public API, it's only used internally, so I'm not sure you would run into the problem you mentioned. That said, Nick and I discussed it during the NanoColor meeting today and I think we're all on the same page about preferring to "vendor" the Imath one. It's simple to turn off the LUT, but there were some other things we wound up editing when we pulled it in. I took an action item to report back to you and Nick at a future OpenEXR TSC meeting.

lgritz commented 4 months ago

Aha, thanks for clarifying. In that case, I don't really care at all. I was thinking that you were exposing this class in the public APIs. Carry on.