Bug: Type Mismatch in GLES 3.0 Shader Code of _Add_Reach_table() and _Add_Cusp_table()

[ufolyah]

This is the code I generated for ACES 2.0 ODT from ACEScg to SDR 100 nit Rec.709 on Android platform. The compile complains that there are int/float type mismatch:

float _ocio_res_reach_m_table_0_sample(float h)
{
  float i_base = floor(h);
  float i_lo = i_base + 1;
  float i_hi = i_lo + 1;
  float lo = texture(_ocio_res_reach_m_table_0Sampler, vec2((i_lo + 0.5) / 363, 0.0)).r;
  float hi = texture(_ocio_res_reach_m_table_0Sampler, vec2((i_hi + 0.5) / 363, 0.5)).r;
  float t = h - i_base;
  return mix(lo, hi, t);
}

Here, the compiler complains around +1 and /363.

vec3 _ocio_res_gamut_cusp_table_0_sample(float h)
{
  int i = int(h) + 1;
  int i_lo = int(max(float(0), float(i + 0)));
  int i_hi = int(min(float(361), float(i + 2)));
  while (i_lo + 1 < i_hi)
  {
    float hcur = _ocio_res_gamut_cusp_table_0_hues_array[i];
    if (h > hcur)
    {
      i_lo = i;
    }
    else
    {
      i_hi = i;
    }
    i = (i_lo + i_hi) / 2;
  }
  vec3 lo = texture(_ocio_res_gamut_cusp_table_0Sampler, vec2((i_hi - 1 + 0.5) / 363, 0.5)).rgb;
  vec3 hi = texture(_ocio_res_gamut_cusp_table_0Sampler, vec2((i_hi + 0.5) / 363, 0.5)).rgb;
  float t = (h - _ocio_res_gamut_cusp_table_0_hues_array[i_hi - 1]) / (_ocio_res_gamut_cusp_table_0_hues_array[i_hi] - _ocio_res_gamut_cusp_table_0_hues_array[i_hi - 1]);
  return mix(lo, hi, t);
}

Here, the compiler complains around + 0.5.

For now, I can make some simple change to the code to make it work, but I have two concerns:

1. Is there a GLSL ES 3.0 test for this codebase's CI?
2. Is it a correct fix to simply change the constants to float type?

[ufolyah]

For now, we switch to ACES1.3 config and so far it works.

[remia]

Thanks for the report @ufolyah, can you confirm this patch fix the issue for you as well?

Is there a GLSL ES 3.0 test for this codebase's CI?

If I'm not mistaken the only GPU runs in CI we currently have is OpenGL on Linux, we recently had new contributions for Vulkan and DX12 tests so now would probably be a good time to try enabling OpenGL / OpenGL ES / Metal / Vulkan and DX12 tests running in CI if possible. We also need a way to enable this running on-demand (assuming there is a cost) for PRs instead of only after merging to main.

For OpenGL ES support, some changes are required to move away from the immediate mode legacy GL, I tested the above patch with some (AI-assisted) code adjustments to have OpenGL ES support and this seem straightforward.

[ufolyah]

@@remia I tried the patch on my device (Snapdragon 8 elite 5). It compiles and looks good.
I did not test the exact rgb value but it looks decent with 709 -> acescg -> rgc -> displayviewtransform to 709 display.

[ufolyah]

I am an android developer and I need to say that the opengl compatibility on android devices could be a nightmare if anyone uses some fancy techniques in shader and texture formats. It works on 8e5 does not mean it works on exynos, kirin or powervr gpu. So if anyone would like to build the CI, I recommend to cover each brand at least.

[remia]

Thanks for testing! I don't know your exact workflow but note that ACES Reference Gamut Compression is no longer strictly needed in general case with ACES 2. For the CI, we will try to add OpenGL ES tests on the Desktop to at least catch simple compilation errors like this, but I don't see us testing any phone platform any time soon unfortunately (you can see from my previous comment we are already quite far from even covering desktop platforms which represent the majority of our users).

[doug-walker]

The fix will be included in OCIO 2.5.2.