freetype-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [ft-devel] Review commits


From: Parth Wazurkar
Subject: Re: [ft-devel] Review commits
Date: Sat, 2 Jun 2018 14:23:50 +0530

>Here they are.  Note that I did a quick look only, and everything
>looks fine.

>* Please replace all tabs with spaces.  It seems that you are using an
>  editor that represents a tab with two spaces: This is another reason
>  to use spaces only since this is non-standard (the default is
>  8 spaces = 1 tab).

Yes I'll do. I also had this doubt about why did the code lost proper indentation when I looked it on cgit.
 
>  [I know that this is your branch where you can do whatever you like,
>   but you asked for review, and good indentation similar to the rest
>   of the FreeType code makes a review simpler for me :-)]

I'll take care about it in future :)
 
>* There are some comments of the form

>    /*foo*/

>  Please replace this eventually with

>    /* foo */
>  (And please don't use `//' comments in the final code.)

Yes will do!
 
>* Avoid trailing spaces, which stick out in ugly red if I use `git
>  diff' :-) Many editors can be configured to automatically remove
>  trailing spaces as soon as you are saving the file.

I'll take care of it. btw which editor you suggest to use?
 
>* gf.h: `INT1', `UINT1', etc. should be removed.  The code should then
>  use `FT_Int', `FT_UInt', etc.  This is a minor issue and not urgent.

>* Maybe you can replace `READ_INT1' and friends with similar macros
>  and functions already defined in FreeType?  This is another minor
>  issue.

>* It might make sense to replace `int' and friends with smaller data
>  types like `FT_UShort' in structure arrays where possible to get a
> smaller memory footprint.  (The stress is on `might' – maybe this is
>  overkill.)

>* Please avoid the use of `float' and `double'.  Right now, FreeType
>  uses integer arithmetic only; `double' is only used for defining
>  compile-time constants and in debug mode for displaying tracing
>  messages.

I'll do it eventually.
 
>  Theoretically, this shouldn't be very difficult since all parts of
>  TeX are using integer arithmetic only – GF, PK, and TFM are no
>  exception.
 
Yes!

Thank you

--
Parth

reply via email to

[Prev in Thread] Current Thread [Next in Thread]