There was a bug in the existing implementation where the glyph for a variation form was not being retrieved. This patch fixes the issue.
Bug details
In the current implementation, the default UVS is checked first, followed by the non-default UVS. However, if the variation selector is not the default form in the font, the result of the default UVS lookup (which is -1) is assigned to i. The non-default UVS lookup should then be performed, but since i is now -1, the code does not enter the branch for the non-default UVS check, and the process terminates without performing the lookup.
Additionally, even when the default UVS lookup is successful, the code incorrectly proceeds to the non-default UVS lookup without considering the result.
Fix
The variable i represents the result of the initial selector lookup, but instead of relying on this result for further logic, the code will be rewritten to use an early exit approach. Once a hit is found in either the default UVS or non-default UVS, the result will immediately be returned. This simplifies the flow and makes it clearer.
Additional request for release
I would like to request that this patch be applied not only to the 2.0.x master branch but also to the 1.9.x branch, and that a patch release be distributed.
I am not using fontkit directly but as a dependency through pdfkit. I am using the latest release of pdfkit, version 0.15.0, which declares a dependency on fontkit^1.8.1. Therefore, in order to benefit from this fix, an update to the 1.9.x branch is necessary.
Adding test code
I was unable to add test code because the 1.9.x codebase seems to have build issues, possibly due to its age, and the existing test cases are not passing. There are multiple issues, and fixing one causes another problem to arise. Since this is complex, I will report it as a separate issue at a later time.
omochi
commented
1 month ago
There was a bug in the existing implementation where the glyph for a variation form was not being retrieved. This patch fixes the issue.
Bug details
In the current implementation, the default UVS is checked first, followed by the non-default UVS. However, if the variation selector is not the default form in the font, the result of the default UVS lookup (which is
-1) is assigned toi. The non-default UVS lookup should then be performed, but sinceiis now-1, the code does not enter the branch for the non-default UVS check, and the process terminates without performing the lookup.Additionally, even when the default UVS lookup is successful, the code incorrectly proceeds to the non-default UVS lookup without considering the result.
Fix
The variable
irepresents the result of the initial selector lookup, but instead of relying on this result for further logic, the code will be rewritten to use an early exit approach. Once a hit is found in either the default UVS or non-default UVS, the result will immediately be returned. This simplifies the flow and makes it clearer.Additional request for release
I would like to request that this patch be applied not only to the
2.0.xmaster branch but also to the1.9.xbranch, and that a patch release be distributed.I am not using
fontkitdirectly but as a dependency throughpdfkit. I am using the latest release ofpdfkit, version0.15.0, which declares a dependency onfontkit^1.8.1. Therefore, in order to benefit from this fix, an update to the1.9.xbranch is necessary.Adding test code
I was unable to add test code because the
1.9.xcodebase seems to have build issues, possibly due to its age, and the existing test cases are not passing. There are multiple issues, and fixing one causes another problem to arise. Since this is complex, I will report it as a separate issue at a later time.