freetype-devel
[Top][All Lists]
Advanced

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

Re: Prototype adjustment database is ready for your review


From: Werner LEMBERG
Subject: Re: Prototype adjustment database is ready for your review
Date: Wed, 07 Jun 2023 03:46:38 +0000 (UTC)

>> The branch gsoc-craig-2023 has the prototype adjustment database.

Looks good, thanks!  Please provide some images that demonstrate what
your code does.

A basic question is whether a vertical shift of exactly one full pixel
is always the correct action.  I can imagine that for very small sizes
a smaller value looks better (at the cost of a more smeared result)
because a full pixel might be too large.  Have you done experiments
into that direction?  In particular, it would be interesting to know
what smaller distance is still acceptable for a given size.

BTW, I was recently in Spain and I've seen on the highway that the
tilde gets often replaced a horizontal bar on direction signs!  For
example, 'Peñaflor' was shown as 'Pen̄aflor'.

And here is my list of remarks that I saw while doing a first sweep
over your code :-)

* Please try to follow the coding style FreeType uses (line length,
  indentation, position of braces, etc., etc.).  Right now, this is
  not of importance – you are free to do whatever you want in your
  development branch.  However, (a) it makes it easier for me to read
  your code, and (b) it will be less work for you to create the
  'cleaned-up' branch representing your GSoC work at the end of the
  project (which is a necessity to complete it).

* The value of `AF_ADJUSTMENT_DATABASE_LENGTH` should be directly
  derived from the `adjustment_database` array, using the 'sizeof'
  operator.

* s/seperation/separation/

* Please decorate functions and structures with comments, explaining
  shortly what they do.  This should happen in the `.c` file, not in
  the `.h` file.  And please use full sentences (including full stops
  at the end) whenever possible or useful.

* I recommend that you add `FT_TRACE5` (or probably `FT_TRACE4`)
  macros wherever useful so that you get a good logging and tracing
  output if the `FT2_DEBUG` environment variable is set accordingly.
  This also means that you need to add proper `FT_COMPONENT` macros,
  with their values properly registered in `internal/fttrace.h`.

* I think the code is easier to understand if you use high-level
  FreeType functions instead of low-level direct calls (except for
  time-critical code).  For example, don't call
  `unicode_charmap->clazz->char_index` but rather `FT_Get_Char_Index`.

* s/umlats/umlauts/

* You should adjust your editor (a) to always remove trailing spaces,
  and (b) to always insert a newline character at the end of a source
  code file.


     Werner

reply via email to

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