2021-11-01
November 11, 2021
Host: Michael Dolan
Attendees:
Apologies:
Mei Chu
OCIO TSC Meeting Notes
Need for 2.1.1:
Doug: Don't want metal support released in 2.1 until unit test coverage is there. In a later PR.
Michael: Do we need a release before OSL and Metal updates?
Patrick: No emergency right now. Metal will only be enum update. Should not move until unit tests are there. Apple asks if it can be in 2.1. Reason for short term solution is avoiding ABI break. It's not mandatory, but great to have. Long term fix will require ABI change. Highlights new requirement for GPU implementation for more modern languages.
Michael: Is that preventing full Metal implementation?
Patrick: Will know more when we have the test framework.
Doug: Not expecting any limitations.
Patrick: Improving implementation is not related to feature set. Just implementation detail.
Kevin: Are we thinking it's good to get it in so that we can see what we really should have in 2.2?
Patrick: That could be a good approach.
Kevin: We would have to change it to make it more elegant and usable for other modern languages. If we wait, it won't be complete until 2.3. But if we move now with current implementation, it would be complete in 2.2.
Patrick: From a different perspective, if we can push that in 2.1, and people use it, it will help us know the change we have to do later. I can only see based on what I know today. Don't know what will be needed for other languages. Just what is needed for Metal.
Kevin: Gives us experience to improve API in next version.
Patrick: We will fix issues of course, but also grab experiences and plan for when we change the API.
Michael: Not any different than OpenGL ES change in terms of ABI compatibility.
Patrick: We should try to have it, but not make it mandatory.
Michael: Work still needed on OSL for 2.1?
Doug: Update was merged already, so is ready to go.
Patrick: Testing framework is there. Want more feedback from users, but implementation point of view is done. Wanting regular releases, do we release now? I don't think we should wait on Metal for 2.1.1. The PR is only the first one.
Doug: At this point 2.1 is reference platform release. Don't think apps are shipping with it yet, or that it's critical to rush 2.1.1. Not sure it's worth doing these releases every time we have a few bug fixes. Preference is to wait.
Michael: Apple is moving fast, so not concerned about pace if we want to wait till end of year.
Doug: Ok with waiting another month or two.
Patrick: As long as we have 2.1.1 before end of year. OSL improvements also fix some bugs. Also small PRs for Imath updates related to build.
Need for 2.0.3:
Patrick: Nothing for now, but have two PRs fixing problematic bugs. ocioconvert metadata save issue, and CLF fix replacing comma with space to adhere to spec.
Michael: the CLF issue seems like it would be worth fixing, unless it's an edge case.
Doug: If written and read with OCIO, will work, but for another project will not. ACES pushing to promote CLF more. Working on release candidate for implementation guide, releasing today. Potential issue. Could see the case for doing it.
Michael: Will it handle reading file with old syntax? I think we could release 2.0.3 at the same time as 2.1.1.
Doug: The fewer releases the better. Less work for all.
Patrick: Both will work. So delay release for 2.1.1? Also include ocioconvert fix?
Michael: Would do both.
Kevin: Yes, agree with Doug, fewer releases the better. Would do both 2.0.3 and 2.1.1 at same time.
Rémi: Should we have warning for CLF syntax issue? In case previously generated CLF file needs to be read elsewhere?
Doug: We try to be strict with writing, and forgiving when reading. There have been other examples of slight differences and we don't warn. We don't have a linting mechanism.
Rémi: Mostly thinking if OCIO used for conformance test, but not main goal of tool, so fine with not doing that.
Doug: There is a read test, with examples of illegal files, and we try to get into conformance, but at this point it's been bigger issues. Not sure if the thing not accepting the commas is specific to CDL, or for any CLF array.
Patrick: Any CLF array.
Doug: We're allowing it, but technically the spec uses spaces or any whitespace. There is a related thing related to fast_float where CLF allows "+" before numbers for XML data type, and fast_float does not. Another example of syntactic thing and we need to decide how to handle it.
Rémi: Could start to be problematic if XSD layer would fail.
Float parsing:
Summary: Following additional discussion, there are three possible options for solving the float localization issue. 1) fast_float third-part lib, as presented in current PR. 2) OIIO/OSL solution as presented by Larry in PR comment. 3) CLF parser solution as presented by Patrick in PR comment. Using a builtin solution would be nice to avoid introducing a new dependency, but fast_float is a good solution if it has better performance, and it would help transition to the C++17 feature in the future. LUT parsing speed is important, since in particular shot-based LUTs, which are read sequentially in review applications, can have a noticeable performance impact. Patrick will compare the three solutions, all of which are reasonable, for performance and provide numbers in the PR for comparison. Following that analysis, amyspark will complete the current PR with the chosen solution.
Thanks to amyspark for the contribution and continued work on this important fix!
Default branch rename:
Michael: Brought this up at last TAC meeting. Some projects have already made the switch and others will do so soon. Kimbal asked that we document our process ro share with the group. Can rename the branch before 2.1.1. Any objection?
No objections raised.
Patrick: Just mention to everyone, just in case.
Michael: I will post a message in Slack #dev channel for visibility.
Items for next meeting agenda:
TODO: Patrick will test the three proposed float parsing solutions for performance comparison.