ampas / aces-vwg-output-transforms

Other
0 stars 2 forks source link

Investigate near zero M behaviour in Gamut mapper. #25

Open KevinJW opened 3 months ago

KevinJW commented 3 months ago

https://github.com/ampas/aces-dev/blob/v2-dev-release/lib/Lib.Academy.OutputTransform.ctl#L919

This conditional sets M to zero for two distinct reasons:

  1. Values of J above the limit, which was an explicitly chosen option
  2. When M is below 0.0001

I think the second case could be limiting us. The original blink mentions something about NaN for the inverse, so there maybe a reasonable need for something here, but perhaps instead of setting to zero, M should be left untouched? This would entail splitting the conditional into its two parts and adding an additional return path. Otherwise I think this creates a 'hole' in the JMh space, which I think is a bug.

We should pay close attention to the compression along the vector does not push values near this limit to below it as this would then not invert back to the same location.

KevinJW commented 3 months ago

Relates to #19 ?

nick-shaw commented 3 months ago

I'm guessing this may be down to precision issues when normalising to very small M, gamut compressing and then denormalising. The question is then whether the better thing to do is to pass M through unmodified (which potentially leaves some pixels outside the gamut) or to clamp it to the gamut. I suppose there is a clamp of the final output anyway, so passing through unmodified may be safe.

KevinJW commented 3 months ago

So my quick test involved feeding this through diagnostic mode 104

set cut_paste_input [stack 0]
version 14.1 v3
push $cut_paste_input
Reformat {
 type "to box"
 box_width 720
 box_fixed true
 name Reformat4
 selected true
 xpos 351
 ypos 221
}
Expression {
 expr0 10
 expr1 0.005*y/(height-1)
 expr2 2*pi*x/(width-1)
 name Expression5
 selected true
 xpos 351
 ypos 245
}

And using a sampler to plot the result, not quite the bug I was looking for

KevinJW commented 3 months ago

image

KevinJW commented 3 months ago

To be clear that is the Blink not the CTL, also the fact I was using radians not degrees was a cut and paste from another use case, doesn't matter if you pass in a degree scaled range it just moves the discontinuity.

priikone commented 3 months ago

I recall that it was just a temporary workaround and wasn't meant to be there in the end. That check wasn't there originally and was added at some point because of NaNs or some such. IMO it should've been removed, and still should be.

nick-shaw commented 3 months ago

Is that expression supposed to be 2*pi? As is it does not work for me with 2pi.

KevinJW commented 3 months ago

yes sorry can't cut-n-paste from work machines very easily! Edited

KevinJW commented 3 months ago

Ah it looks like that discontinuity might come from me not having merged the updated v60... so now my original test shows the missing cylinder near M == 0

image