Fix a few typo

[NCBM]

The "length" field seems to have a wrong name and type.

On my linux with freetype2 installed provides this struct like this:

typedef struct  FT_Data_
{
  const FT_Byte*  pointer;
  FT_UInt         length;

} FT_Data;

I can confirm the name y is wrong, but I'm not sure why use FT_Int in ft_structs.py.

I searched in this project and found FT_Data as a struct interface merely.

[HinTak]

For your questions, naming differences could be historical (likely the other side updated), and unsigned length requires validation elsewhere, while signed length is a check on itself for overflows and wrong data.

I think this needs research on the history on both sides.

That said, it would also be nice for you to supply a example code where this is used.

[NCBM]

For the name y, I believe it is an error - I downloaded the source tarball of freetype 2.2.1 (released in 2009, on sourceforge) and found that the name was length, which also corresponds with code comments. In my view I can believe it was not changed.

For the type FT_Int, I searched through the source files and found that FT_Int was used until freetype 2.11.1 (released in 2021, on sourceforge). In 2.11.1 it was changed into FT_UInt.

Changelog of freetype 2.11.1:

CHANGES BETWEEN 2.11.0 and 2.11.1

  I. IMPORTANT CHANGES

    - Some  fields  in  the  `CID_FaceDictRec`, `CID_FaceInfoRec`, and
      `FT_Data` structures  have been changed  from signed to unsigned
      type,  which  better reflects  the actual usage.  It is also  an
      additional means to protect against malformed input.

FT_Data structures have been changed from signed to unsigned type, which better reflects the actual usage.

I searched the github and found a few projects which directly includes the source code of freetype-py, as well as some unique freetype bindings with correct field name with FT_Int type.

I'm sorry that I don't have code in practice of this type, instead I'm browsing the source (to make a .pyi stub for private use at least) and found this suspicious definition.

[rougier]

For the name y, It might be typically an error on my part. When I search/replace text in emacs, I type y to confirm change and from time to time I hit y too much. In any case, it does not make sense to keep the y.

[HinTak]

@NCBM just a dev tips - you can do "git blame ..." on a file on the command line to see when a specific line was last changed, and why. There are GUI equivalent (a "blame" button) showing the same on github. Some changes are just formatting, so you "git blame" earlier, until you find what you want.

I think I have C code that uses it, but client pointer is a strange thing in freetype.

[NCBM]

Oh, a git function I have never used. Acknowledged. This struct seems to be less used, at least in python. It's not easy to consider.

…

---- Replied Message ---- | From | ***@***.***> | | Date | 04/29/2024 22:56 | | To | ***@***.***> | | Cc | ***@***.***>***@***.***> | | Subject | Re: [rougier/freetype-py] Fix wrong field in FT_Data (PR #190) | @NCBM just a dev tips - you can do "git blame ..." on a file on the command line to see when a specific line was last changed, and why. There are GUI equivalent (a "blame" button) showing the same on github. Some changes are just formatting, so you "git blame" earlier, until you find what you want. I think I have C code that uses it, but client pointer is a strange thing in freetype. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[NCBM]

Maybe changing the field name is reasonable and acceptable, while it needs more consideration to change the type for now?

[HinTak]

Sorry for the slow response. Yes, the field name change is the more important one; the type change only affect data larger than 2GB and wrapping around, which is quite unlikely. For my part, I am pausing for a suitable example usage.

[HinTak]

It is possible that there is no usage with current freetype-py: freetype-py only covers about 70% of freetype API the last time I counted. We basically adds bindings to the remaining as usages are found. (Recent addition coming to mind is color layer related routines for colour fonts). There does not seem to be anything among the 70% which uses this structure.

[NCBM]

I'm sorry that I changed other code (although it's a typo).

For the original typo, it only sets a wrong attribute and makes the restype not working as expected, and I'm not sure the effect of this function.

[HinTak]

Hmm, you shouldn't force-push... the 2nd typo can go in first, as a separate pull. Anyway, as for your question, restype defaults to int (and same as FT_Bool on most platforms), so it isn't very different from not setting it. I think I wrote that line myself and the example that goes with it, possibly.

[HinTak]

I did: 4b7f93b

[HinTak]

I have cherry-picked your 2nd commit: d06cd73 - you don't need to do anything (git can deal with simultaneous identical changes across branches, as long as they are exactly the same in the surrounding code); but ideally this should just go back to how it was - the first commit (before you changed it) was waiting for accompanying example code and possibly API addition to actually use that structure, that's all.

[HinTak]

As a rule, force push to already-published branches are frowned upon, and should only be done if you genuinely want to erase the history of it ever happened. In this case, the change was okay (at least on first glance), just waiting for additions, so shouldn't be withdrawn.

[HinTak]

If you want to experiment with restoring deleted/overwritten/rebased work, the command to use is "git reflog" to look up how your repo was. Typically you can do that within about 2 weeks of a "mistake". I think your old commit was 9f8440d (should be confirmed by "git reflog"), and you can recover the old work with "git checkout-b new-branch-name 9f8440d" to recover it, then force-push from the new branch with renaming, something like "git push -f origin new-branch-name:old-branch-name" to update the pull. Read about "git reflog" and the syntax of "git push" about renaming while pushing.

[HinTak]

I have recovered your old commit and put it - 2d5b6d0 , with a comment following a3f915b about the lack of examples.

Looked through the history - the sign change was recent (2021) on FreeType's side. The field has always been 'length' on FreeType side since 2003 (beginning of FreeType 2) and has always been wrong in freetype-py in the initial commit in 2011.

As for examples, while it is used internally, the only public APIs which use this is in incremental glyph loading, and it is an interface negotiated between the freetype and ghostscript, and AFAIK, only ghostscript uses it (for pcl/pxl/postscript, which have partial embedded font bits, not quite complete fonts). So it is probably quite difficult to come up with an example, likely involving reimplementing some of ghostscript's functionality in python. Not even mupdf uses it. Hence the added comment in case somebody is brave enough to try ;-).