L4 fixes #3179
L4 fixes #3179
Conversation
|
Err. Needs more work. |
|
This still seems to produce wrong results with avar present. Sigh. |
I think I fixed it. |
behdad
force-pushed
the
L4-fixes
branch
2 times, most recently
from
fbc7541
to
0199946
Compare
|
Tests fixed. |
|
I think this is good now. |
outGain is always zero in this branch.
Imagine a font with current min/default/max of 100,700,1000. And new setting of 100,400,1000. The current normalizeLocation will calculate the new location for 700 to be +.33, whereas it should calculate +.5! This is because 400 translates to -.5, so 700 will be normalized to -1,-.5,+1 and get +.33... We need a special normalizeLocation that is aware of the "distance" between min/default/max, ie. the non-normalized values. Then it will be clear that the distance from 400 to 700 is equal to 700 to 1000, and as such 700 should be normalized to .5, not .33... I'm still trying to figure out the case where avar is present. Store this distance in NormalizeAxisLimit and reach it out in the solver. Fixes #3177
|
With the latest changes the test coverage is complete for the change. |
There was a problem hiding this comment.
I find it strange that we didn't catch this before, when L4 was not implemented yet. Can you confirm that nothing basically has/will change after this PR for the pinning-axis and restrict-axis-but-keep-default cases of partial instancing, and that these changes only affect when move-axis-default is involved?
| if lower >= 0: | ||
| return (v - default) / (default - lower) | ||
|
|
||
| # lower < 0 and v < default |
There was a problem hiding this comment.
I see that your example make sense but I'm still not 100% clear why this situation in which default >=0 and v < default and lower < 0 needs to take into account the positive/negative distances, whereas the other cases do not. Is it because in this particular case the value v falls in a segment that may span both the negative and positive sides and thus the ratio between these two matters?
There was a problem hiding this comment.
Is it because in this particular case the value v falls in a segment that may span both the negative and positive sides and thus the ratio between these two matters?
Correct.
Correct. Because as you noted, this only affects cases where the new default moves, and the new range for min-side or max-side spans over the previous default, which only happens with L4. And yes I'm also surprised we didn't catch this before. |
|
Thanks @anthrotype. I think I addressed all your feedback. |
Fixes #3177
See #3177 (comment)