OpenColorIO icon indicating copy to clipboard operation
OpenColorIO copied to clipboard

ociodisplay standalone: incorporate more of OCIO's user-facing functionality

Open zachlewis opened this issue 7 years ago • 8 comments

I know, I know -- ociodisplay is meant to be a demo of OCIO capabilities for developers to further build upon. Problem is -- and I can't say this is for sure the case for everyone, but it's something I deeply suspect -- software vendors see ociodisplay, and think all they have to do is port the functionality within to their own applications and call it a day. Documentation improvements will definitely help, but it's one thing to read about it, and another to have a functional example. Not to mention, color science / pipelines are pretty arcane, and there's every reason to believe that the developer in charge of integrating OCIO might not be familiar with common use cases.

I'm led to believe this because when vendors implement OCIO, there seems to be a tendency to omit things because they don't know to support them.

For instance, check out this thread by one of the sidefx developers in response to OCIO integration from when they initially rolled it out: https://www.sidefx.com/forum/topic/48742/#post-220323

"Unfortunately the OpenColorIO.org samples don't include any Looks, so we completely missed this one.... H16 OpenColorIO support remains in beta, until we're satisfied that it's meeting users' needs."

Likewise, I recall there being a mess with RV 3 / 4 regarding Looks. And in other instances that I can't remember, we found that the safest thing to do was avoid Looks entirely and just use display-referred ColorSpaces, for the sake of eliminating possible room for error across DCC apps that had varying support for all of OCIO.

Anyway, all this is to say, I think if there were a more feature-complete demo direct from the horse's mouth, it would help developers do a better and more consistent job of implementing OCIO in their applications.

Reasonable or unnecessary?

zachlewis avatar Jun 12 '17 20:06 zachlewis

err... upon closer inspection, I see that the sidefx dev quoted above is talking about the available example configs... so, that quote probably belongs in a separate ticket re: including more examples of different workflows and styles based on pipeline needs. I'd be happy to contribute example files / environment scripts / directory structures.

But I still think the missing components in the demo are leading to inconsistent implementations across applications. (It is getting better, now that OCIO has been out for... what, five years or so... but still...)

zachlewis avatar Jun 12 '17 20:06 zachlewis

Agreed, having a well defined reference implementation would be very useful, and ociodisplay is a bit limited in scope. The documentation of course needs some love as well.

scoopxyz avatar Jun 20 '17 21:06 scoopxyz

Also link this to #99

scoopxyz avatar Jun 20 '17 21:06 scoopxyz

I also experienced source code being a copy & paste of ociodisplay so revisiting the apps implementation would be a great thing.

hodoulp avatar Mar 03 '20 14:03 hodoulp

Hey @hodoulp. I'd like to work on this. Any tips on how to get started?

rashil2000 avatar Mar 09 '20 06:03 rashil2000

Hi @rashil2000 Happy to see your interest in the OpenColorIO project.

To have a first pull request (to demonstrate your abilities), I would not suggest to start with this defect. As we are currently revisiting feature need to correctly refactor the app code.

You should think about about something small and localized. A suggestion could be to have a look at #536, and validate the ColorSpace parsing where to_reference and from_reference must be unique (in fact, all the attributes must be unique).

hodoulp avatar Mar 09 '20 17:03 hodoulp

@hodoulp Is now a good time to work on this defect? If so, I am only familiar with the OCIO integration in renderers (e.g. Blender, appleseed), and would like some suggestions on what other application that uses OCIO library I can reference from.

ChinYing-Li avatar Jul 09 '20 09:07 ChinYing-Li

Proposal

To add DynamicProperty, ociodisplay will be a state machine, with state g_ocioDisplayState. g_ocioDisplayState is of type OCIODisplayState, and

enum OCIODisplayState
{
    FullPipeline = 1,
    Dynamic
}
  • The last few lines inociodisplay's int main(), the try block will call UpdatedBasedOnState(). UpdateBasedOnState() will then call UpdateOCIOGLState() if g_ocioDisplayState == FullPipeline and call another (yet to be written) function UpdateDynamicState() if g_ocioDisplayState == Dynamic

  • The user switch the OCIODisplayState by pressing a certain keys (say, SHIFT + CTRL). Of course, we probably need to guard against user switching the state when they have OCIOmenu visible. That OCIOmenu will only be in use when in state FullPipeline.

  • When switching state, the final value of g_exposure_fstop, g_display_gamma and (currently non existing)g_contrast in one state will be the starting value in the other state.

ChinYing-Li avatar Jul 15 '20 20:07 ChinYing-Li