Comment 1 Piotr Kubaj 2016-07-18 16:07:12 UTC

Created attachment 172665 [details]
2.6.5 patch

This new version adds LICENSE.

Comment 2 Jan Beich 2016-07-20 14:58:15 UTC

2.6.4 changelog: https://sourceforge.net/projects/freetype/files/freetype2/2.6.4/README/view
2.6.5 changelog: https://sourceforge.net/projects/freetype/files/freetype2/2.6.5/README/view

As 2.6.4 removed symbols one may need to bump PORTREVISION in consumers.

http://abi-laboratory.pro/tracker/timeline/freetype/

Comment 3 Jan Beich 2016-07-20 15:14:19 UTC

Comment on attachment 172665 [details]
2.6.5 patch

(In reply to Piotr Kubaj from comment #1)
> This new version adds LICENSE.
[...]
> +LICENSE=	GPLv2

According to docs/LICENSE.TXT it's a dual-licensed project: FreeType License or GNU General Public License version 2 (or any later). For example,

  LICENSE=	FTL GPLv2+
  LICENSE_COMB=	dual
  LICENSE_NAME_FTL=	FreeType License
  LICENSE_FILE_FTL=	${WRKSRC}/docs/FTL.TXT
  LICENSE_PERMS_FTL=	dist-mirror dist-sell pkg-mirror pkg-sell auto-accept
  LICENSE_FILE_GPLv2+ =	${WRKSRC}/docs/GPLv2.TXT

Comment 4 Piotr Kubaj 2016-07-20 16:48:10 UTC

Thanks for the link to http://abi-laboratory.pro, I didn't know about this website.

As for your LICENSE section, is:
  LICENSE_FILE_GPLv2+ =	${WRKSRC}/docs/GPLv2.TXT
necessary?

GPLv2 and GPLv3 are present in /usr/ports/Templates/Licenses

Other than that, it certainly looks better than my diff.

Comment 5 Jan Beich 2016-07-20 19:56:53 UTC

(In reply to Piotr Kubaj from comment #4)
> As for your LICENSE section, is:
>   LICENSE_FILE_GPLv2+ =	${WRKSRC}/docs/GPLv2.TXT
> necessary?

It is a workaround for an (unfiled) issue with + (plus) variants of (L)GPL looking for wrong license file. Try selecting VIEW_GPLv2+ after

  $ LICENSES_ASK=1 make clean patch

Comment 6 Piotr Kubaj 2016-07-21 18:55:11 UTC

(In reply to Jan Beich from comment #5)
Maybe I don't understand something but it works for me - I can read GPL2 just fine.

Comment 7 Jan Beich 2016-07-21 19:38:20 UTC

Do you see actual license text (same as Templates/Licenses/GPLv2) or the following stub? It happens with GPLv2+, LGPL21+, etc. but not with GPLv2, LGPL21, etc.

  The license: GPLv2+ (GNU General Public License version 2 (or later))
  is standard, please read from the web.

If you still don't see the issue then ignore LICENSE_FILE_GPLv2+ line.

Comment 8 Piotr Kubaj 2016-07-21 22:05:30 UTC

(In reply to Jan Beich from comment #7)
Yes, I can definitely see the full GPLv2 license in the dialog menu.

Comment 9 Piotr Kubaj 2016-07-21 22:08:08 UTC

Turns out I had LICENSE_FILE_GPLv2+ set in the Makefile. Sorry for the confusion.

Does it mean that we need to set LICENSE_FILE every time (L)GPL*+ is set?

Created attachment 172884 [details]
Proposed patch (since 412348 revision)

Hello Piotr Kubaj and Jan Beich.

I think, the replacement of "LCD_FILTERING with V40 code" is wrong proposal, because they are different options and defines:
FT_CONFIG_OPTION_SUBPIXEL_RENDERING:
https://www.freetype.org/freetype2/docs/reference/ft2-lcd_filtering.html
https://www.freetype.org/freetype2/docs/reference/ft2-lcd_filtering.html#FT_Library_SetLcdFilter
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?id=VER-2-6-5#n78

TT_CONFIG_OPTION_SUBPIXEL_HINTING:
https://www.freetype.org/freetype2/docs/reference/ft2-tt_driver.html#interpreter-version
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?id=VER-2-6-5#n586

The V40 option should not be enabled by default for v2.3.5 (as it was for v2.3.4), because of the following changes in v2.3.5:
https://www.freetype.org
-8<--
FreeType 2.6.5
2016-07-12

This release is almost identical to the previous version, with two differences.

- It compiles again on Mac OS X, and
- it reverts the activation of subpixel hinting by default; it will be enabled by default in the forthcoming 2.7.x series. Main reason for reverting this feature is the principle of least surprise: a sudden change in appearance of all fonts (even if the rendering improves for almost all recent fonts) should not be expected in a new micro version of a series.
-->8-

In docs/CHANGES file:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/CHANGES?id=VER-2-6-5#n2
-8<--
CHANGES BETWEEN 2.6.4 and 2.6.5

I. IMPORTANT BUG FIXES

- Compilation works again  on Mac OS X (bug introduced  in version
  2.6.4).


I. IMPORTANT CHANGES

- The new  subpixel hinting  mode is now  disabled by  default; it
  will  be enabled  by default  in the  forthcoming 2.7.x  series.
  Main reason for reverting this feature is the principle of least
  surprise: a  sudden change in  appearance of all fonts  (even if
  the rendering improves  for almost all recent  fonts) should not
  be expected in a new micro version of a series.
-->8-

The FreeType 2 font engine is dual licensed:
https://www.freetype.org/license.html
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/LICENSE.TXT?id=VER-2-6-5
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/FTL.TXT?id=VER-2-6-5
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/GPLv2.TXT?id=VER-2-6-5

Therefore, I would like to propose slightly different patch, with following changes (including changes from Piotr Kubaj):
- Add license information [1, 2]
- Add new sub-pixel hinting options [1, 2]
- Regenerate patch [2]
- Adapt pkg-plist [1]

[1] - changes made by Piotr Kubaj.
[2] - changes made by me.

Also, I manually regenerated TIMESTAMP value for distinfo file with using following command:
% make clean extract && find work/* -type f -print0 | xargs -0 stat -f '%m' | sort -u | tail -1
based on "last modification time of the source" files, according to "SOURCE_DATE_EPOCH specification": https://reproducible-builds.org/specs/source-date-epoch/

Created attachment 172885 [details]
Proposed patch (since 412348 revision)

Lowercase "m" in "minimal" word for V40 option, as it was in documentation.

Created attachment 172886 [details]
Proposed patch (since 412348 revision)

Fix for abbreviation of V40 option, which used in news for v2.6.4.

(In reply to comment #7)
> Do you see actual license text (same as Templates/Licenses/GPLv2) or the
> following stub? It happens with GPLv2+, LGPL21+, etc. but not with GPLv2,
> LGPL21, etc.
> 
>  The license: GPLv2+ (GNU General Public License version 2 (or later))
>  is standard, please read from the web.
> 
> If you still don't see the issue then ignore LICENSE_FILE_GPLv2+ line.
I noticed the same issue with "+" for (L)GPL* licenses. In case of GPLv2+, it shows the stub, even if LICENSE_FILE_GPLv2+ (or LICENSE_FILE_GPLv2) defined (in first case, this is addition (+=), right?). So, I think, the usage of "+" is wrong and better to define LICENSE_FILE for it or create another name(s) without arithmetic operators in port's framework, if needed.

I also noticed, that you proposed your own version of licenses in comment #3. I created slightly different proposal.

Created attachment 172887 [details]
Proposed patch (since 412348 revision)

Maybe, usage of FTL instead of BSD3CLAUSE name for licenses is better, proposed by Jan Beich in comment #3.
The overall changes are following:
- Add license information [1, 2, 3]
- Add new sub-pixel hinting options [1, 3]
- Regenerate patch [3]
- Adapt pkg-plist [1]

[1] - changes made by Piotr Kubaj.
[2] - change made by Jan Beich.
[3] - changes made by me.

(In reply to comment #14)
> [2] - change made by Jan Beich.
* changes.

Comment 16 Jan Beich 2016-07-23 13:17:24 UTC

Comment on attachment 172887 [details]
Proposed patch (since 412348 revision)

> +LICENSE_FILE_FREETYPE2=	${WRKSRC}/docs/LICENSE.TXT

The file documents how to use (included) licenses but isn't an actual license. Try running

  $ LICENSES_ASK=1 make clean patch

When a user selects USE_FREETYPE2 what license terms she accepts? If the answer is in either docs/GPLv2.TXT or docs/FTL.TXT then it's invalid because those are already claimed by other licenses.

> +LICENSE_FILE_GPLv2=	${WRKSRC}/docs/GPLv2.TXT

GNU project says

  Please note that GPLv3 is not compatible with GPLv2 by
  itself. However, most software released under GPLv2 allows you to
  use the terms of later versions of the GPL as well. When this is the
  case, you can use the code under GPLv3 to make the desired
  combination. To learn more about compatibility between GNU licenses,
  please see our FAQ.

which is the case here (in docs/LICENSE.TXT)

  - The GNU General Public License version 2, found in  `GPLv2.TXT' (any
    later version can be used  also), for programs which already use the
    GPL.  Note  that the  FTL is  incompatible  with  GPLv2 due  to  its
    advertisement clause.

 ports r405873 added "or any later version" types in order to track such compatibility.

> +OPTIONS_RADIO=		SUBPIXEL_HINTING
> +OPTIONS_RADIO_SUBPIXEL_HINTING=	V38 V40 BOTH

BOTH option may confuse users but can be eliminated via OPTIONS_GROUP at the cost of losing option helpers for the conditional.

(In reply to lightside from comment #13)
> In case of GPLv2+, it shows the stub, even if LICENSE_FILE_GPLv2+
> (or LICENSE_FILE_GPLv2) defined (in first case, this is addition
> (+=), right?).

amdmi3@ documented the issue in Mk/bsd.licenses.db.mk as

  # Note that though plus is allowed in a variable (and this license) name it
  # needs an extra space before following equals sign for them not to be parsed
  # as a single += operator

with comment 3 as another example.

Created attachment 172889 [details]
Proposed patch (since 412348 revision)

(In reply to comment #16)
> The file documents how to use (included) licenses but isn't an actual license.
I included docs/LICENSE.TXT (as FREETYPE2) for informative purposes. It can be removed for purist's purposes. Possible to create DOCS option with PORTDOCS=reference CHANGES formats.txt LICENSE.TXT raster.txt

(In reply to comment #16)
> BOTH option may confuse users but can be eliminated via OPTIONS_GROUP at the
> cost of losing option helpers for the conditional.
The usage of OPTIONS_RADIO is correct (List of radio-choice grouped options: 0 or 1 among N).
Do you think V38 and V40 are more easy to understand? In overall options this is correct and understandable: someone knows Infinality code, someone knows new mininal code (a.k.a. ClearType), someone associates them with concrete versions of TrueType interpreters (v35, v38, v40) as in source code.
Moreover, "Compile both" is used in ftoption.h:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?id=VER-2-6-5#n619

(In reply to comment #16)
> # Note that though plus is allowed in a variable (and this license) name it
> # needs an extra space before following equals sign for them not to be parsed
> # as a single += operator
Yes, it works. Thanks.

The new proposed patch attached, related to license changes.

(In reply to comment #17)
> new mininal
* minimal

Created attachment 172891 [details]
Proposed patch (since 412348 revision)

(In reply to comment #17)
> Possible to create DOCS option with
> PORTDOCS=reference CHANGES formats.txt LICENSE.TXT raster.txt
Added DOCS option, just in case.

Created attachment 172892 [details]
Proposed patch (since 412348 revision)

(In reply to comment #16)
> BOTH option may confuse users but can be eliminated via OPTIONS_GROUP at the
> cost of losing option helpers for the conditional.
Perhaps, I misunderstood you.
I attached new version with V38 and V40 options only as OPTIONS_GROUP.

Created attachment 172893 [details]
Proposed patch (since 412348 revision)

Renamed SUBPIXEL_MODE variable to SUBPIXEL_HINTING_MODE.

Created attachment 172896 [details]
Proposed patch (since 412348 revision)

Possible to include (extra) patch to "Make sub-pixel hinting mode configurable":
https://git.archlinux.org/svntogit/packages.git/tree/trunk/0003-Make-subpixel-hinting-mode-configurable.patch?h=packages/freetype2&id=a86f6a5423659061598b079599970aeb0e13b9a8
As described in Arch Linux wiki for Font configuration:
https://wiki.archlinux.org/index.php/Font_Configuration#Subpixel_rendering

I attached new proposed patch. Possible to revert previous obsoleted patch, if needed.

Comment 23 Jan Beich 2016-07-23 16:00:04 UTC

Comment on attachment 172896 [details]
Proposed patch (since 412348 revision)

> +V38_DESC=	v38 mode (Infinality code)
> +V38_VARS=	SUBPIXEL_HINTING_MODE=1
> +V40_DESC=	v40 mode (minimal code, a.k.a. ClearType hinting)
> +V40_VARS=	SUBPIXEL_HINTING_MODE=2
> +
> +.include <bsd.port.options.mk>
> +
> +.if defined(SUBPIXEL_HINTING_MODE)
> +.if ${PORT_OPTIONS:MV38} && ${PORT_OPTIONS:MV40}
> +SUBPIXEL_HINTING_MODE=3
> +.endif
> +CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=${SUBPIXEL_HINTING_MODE}
> +.endif

SUBPIXEL_HINTING_MODE is unnecessary distraction in your case. The variable can still be kept for bmake simplification.

  V38_DESC=	v38 mode (Infinality code)
  V40_DESC=	v40 mode (minimal code, a.k.a. ClearType hinting)

  .if defined(.PARSEDIR)

  V38_VARS=	SUBPIXEL_HINTING_MODE+=1
  V38_CFLAGS=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=$$$((${SUBPIXEL_HINTING_MODE:ts|}))
  V40_VARS=	SUBPIXEL_HINTING_MODE+=2
  V40_CFLAGS=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=$$$((${SUBPIXEL_HINTING_MODE:ts|}))

  .else # XXX Drop once /stable/9 hits EOL
  .include <bsd.port.options.mk>

  .if ${PORT_OPTIONS:MV38} && ${PORT_OPTIONS:MV40}
  CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=3
  .elif ${PORT_OPTIONS:MV40}
  CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=2
  .elif ${PORT_OPTIONS:MV38}
  CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=1
  .endif

  .endif # .PARSEDIR

Comment 24 Jan Beich 2016-07-23 16:27:42 UTC

Comment on attachment 172896 [details]
Proposed patch (since 412348 revision)

> +CONFIGURABLE_DESC=	Make sub-pixel hinting mode configurable
[...]
> ++    envval = getenv( "FT2_SUBPIXEL_HINTING" );

freetype2 is mainly configured via fontconfig, not via environment variables. With V38 and V40 both OFF by default CONFIGURABLE maybe of debugging value only.

Created attachment 172900 [details]
Proposed patch (since 412348 revision)

(In reply to comment #23)
> SUBPIXEL_HINTING_MODE is unnecessary distraction in your case.
> The variable can still be kept for bmake simplification.
Looks like, I was right about OPTIONS_RADIO instead of OPTIONS_GROUP. I renamed BOTH option to V38_V40. The (1 | 2) = 3, by the way.
If you not agree, better to propose your version, I think. It may be usable in case of 3 or more such modes (excluding overall mode).

(In reply to comment #24)
> freetype2 is mainly configured via fontconfig, not via environment variables.
> With V38 and V40 both OFF by default CONFIGURABLE maybe of debugging value only.
Ok, I removed CONFIGURABLE option.

Thanks for reviews.

Created attachment 172904 [details]
Proposed patch (since 412348 revision)

Ok, I attached the updated version with V38 and V40 only options.
Thanks Jan Beich for ideas in comment #23.

Sorry about so many patches.

Comment 27 Jan Beich 2016-07-23 18:55:54 UTC

Comment on attachment 172904 [details]
Proposed patch (since 412348 revision)

> +.include <bsd.port.pre.mk>
> +
> +.if defined(SUBPIXEL_HINTING_MODE)
> +CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=${SUBPIXEL_HINTING_MODE:ts|:Q}

bsd.port.pre.mk can also be dropped via :D modifier.

  CFLAGS+=	${SUBPIXEL_HINTING_MODE:D-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=${SUBPIXEL_HINTING_MODE:ts|:Q}}

As the line abused horizontal space comment 23 went with option helpers instead and a side effect of duplicated CFLAGS. Those are harmless but can still be fixed by :u modifier.

  .include <bsd.port.options.mk>

  CFLAGS:=	${CFLAGS:u}

I'm sure there're better ways to utilize bmake features in ports but let's not forget that FreeBSD 9.x still use fmake where :ts and :D are not supported.

  $ make
  Unknown modifier 't'

One way to plan for the future EOL (circa 2017-01-01) is to introduce .if defined(.PARSEDIR) conditionals.

Ok, let's revert the attachment #172900 [details], which is more terse, without the need to select 2 options, if all modes needed. If there are more sub-pixel hinting modes will be available, then the method might be changed to other available at that time.

Created attachment 172906 [details]
Proposed patch (since 412348 revision)

I found solution, which works for FreeBSD 9 with fmake, by using shell.

Created attachment 172907 [details]
The poudriere testport log (FreeBSD 9.3 amd64, with V38=on and V40=on)

Created attachment 172919 [details]
Proposed patch (since 412348 revision)

Sorted LICENSE_FILE_* defines alphabetically.

To Piotr Kubaj:
Do you still insist about replacement of LCD_FILTERING with V40 code as in summary?
If no, could you please correct the summary and obsolete your patch (attachment #172665 [details])?
I proposed patch (current attachment #172919 [details]), which includes your proposal in correct form (in my opinion) as optional feature(s) (according to upstream statement in v2.6.5 changelog).

Comment 33 Piotr Kubaj 2016-08-03 19:35:35 UTC

(In reply to lightside from comment #32)
Done.
As I build my own packages I don't insist on it, but I still believe it should be on by default.

The reason is that LCD_FILTERING is also ON now, even though upstream keeps it disabled.

(In reply to comment #33)
> Done.
Thanks, but I think it might be just:
print/freetype2: Update to 2.6.5

(In reply to comment #33)
> As I build my own packages I don't insist on it, but I still believe it should
> be on by default.
My question in comment #32 was about the exact words in the (previous) summary: "replace LCD_FILTERING with V40 code".
The LCD_FILTERING and V40 options are related to different defines:
FT_CONFIG_OPTION_SUBPIXEL_RENDERING:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?id=VER-2-6-5#n78
TT_CONFIG_OPTION_SUBPIXEL_HINTING=2:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?id=VER-2-6-5#n586

While you proposed to change FT_CONFIG_OPTION_SUBPIXEL_RENDERING to FT_CONFIG_OPTION_SUBPIXEL_RENDERING=2 in your patch (attachment 172665 [details]):
-LCD_FILTERING_CFLAGS=	-DFT_CONFIG_OPTION_SUBPIXEL_RENDERING
+V40_CFLAGS=		-DFT_CONFIG_OPTION_SUBPIXEL_RENDERING=2

This is the same as current LCD_FILTERING option.

The exaplanation about why subpixel hinting mode was disabled for 2.6.5 version is in CHANGES file:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/CHANGES?id=VER-2-6-5#n2

(In reply to comment #33)
> The reason is that LCD_FILTERING is also ON now, even though upstream keeps
> it disabled.
The possible reason is patent(s) issues:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?id=VER-2-6-5#n83
https://freetype.org/patents.html

Created attachment 173436 [details]
Proposed patch (since 412348 revision)

I think, the include/freetype/config/ftoption.h file should be patched directly for FT_CONFIG_OPTION_SUBPIXEL_RENDERING and TT_CONFIG_OPTION_SUBPIXEL_HINTING defines. The same does:
- Arch Linux:
https://git.archlinux.org/svntogit/packages.git/tree/trunk/0002-Enable-subpixel-rendering.patch?h=packages/freetype2&id=a86f6a5423659061598b079599970aeb0e13b9a8
https://git.archlinux.org/svntogit/packages.git/tree/trunk/0003-Make-subpixel-hinting-mode-configurable.patch?h=packages/freetype2&id=a86f6a5423659061598b079599970aeb0e13b9a8
- Fedora Linux:
http://pkgs.fedoraproject.org/cgit/rpms/freetype.git/tree/freetype-2.3.0-enable-spr.patch?id=9a81147af83b1166a5f301e379f85927cc610990
- Gentoo Linux:
https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/freetype/freetype-2.6.5.ebuild?id=c97bdd42e73641b235a38eb9db7e8bfd0c58ddc5#n53

Also possible to add an option to enable table validation modules. The same does:
- Arch Linux:
https://git.archlinux.org/svntogit/packages.git/tree/trunk/0001-Enable-table-validation-modules.patch?h=packages/freetype2&id=a86f6a5423659061598b079599970aeb0e13b9a8
- Fedora Linux:
http://pkgs.fedoraproject.org/cgit/rpms/freetype.git/tree/freetype-2.2.1-enable-valid.patch?id=9a81147af83b1166a5f301e379f85927cc610990
- Gentoo Linux:
https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/freetype/files/freetype-2.3.2-enable-valid.patch?id=c97bdd42e73641b235a38eb9db7e8bfd0c58ddc5

Created attachment 173438 [details]
Proposed patch (since 412348 revision)

Clarified sed patch for TT_CONFIG_OPTION_SUBPIXEL_HINTING.

Comment 37 Piotr Kubaj 2016-08-29 19:17:23 UTC

(In reply to lightside from comment #36)
Fine for me, I must have mistaken the ifdef variables :)

Created attachment 174580 [details]
Proposed patch (since 412348 revision)

Updated to 2.7 version:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/CHANGES?id=VER-2-7#n2

Added V40 option to OPTIONS_DEFAULT (as upstream did).
Adapted sed patches and pkg-plist.

To Piotr Kubaj:
Please, update the summary of PR again.

By the way, do you think this is good idea to use V40 as default, even if upstream used this?
Unfortunately, there was no commit for 2.6.5 version without this, but selectable options to try it before default. Looks like, developer(s) created a way to use FREETYPE_PROPERTIES environment variable to select other TrueType interpreter version (e.g. 35).

Comment 40 Piotr Kubaj 2016-09-13 21:50:31 UTC

(In reply to lightside from comment #39)
I believe FreeBSD should provide vanilla software if possible. If the users want to change the default settings, they are free to recompile or change FREETYPE_PROPERTIES.

> By the way, do you think this is good idea to use V40 as default, even if upstream used this?

I think it is a good idea to build V40 by default.
Hinting can be disabled at runtime (via FREETYPE_PROPERTIES).
But it is better to document how to get old behavior (in /usr/ports/UPDATING?)

> If the users want to change the default settings, they are free to recompile

Port users are free to change options and recompile. Growing number of binary package users have no such freedom. So default set of options should be most useful for some average pkg user.

Created attachment 175459 [details]
Proposed patch (since 412348 revision) with pkg-message file

(In reply to comment #41)
> But it is better to document how to get old behavior (in /usr/ports/UPDATING?)
I think, possible to use port's pkg-message file for this. I attached some variant of it, based on information from following files:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/CHANGES?id=VER-2-7#n2
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?id=VER-2-7#n80
-8<--
The 2.7.x series now uses the new subpixel hinting mode (V40 port's option) as
the default, emulating a modern version of ClearType. This change inevitably
leads to different rendering results, and you might change port's options to
adapt it to your taste (or use the new "FREETYPE_PROPERTIES" environment
variable).

An environment variable "FREETYPE_PROPERTIES" can be used to control driver
properties. Example:

	FREETYPE_PROPERTIES=truetype:interpreter-version=35 \
		cff:no-stem-darkening=1 \
		autofitter:warping=1

This allows to select, say, the subpixel hinting mode at runtime for a given
application.

The controllable properties are listed in the section "Controlling FreeType
Modules" in the reference's table of contents
(%%DOCSDIR%%/reference/ft2-toc.html, if DOCS port's option was enabled).
-->8-

While your are here please add DEBUG port option (default off) to allow build freetype with FT_DEBUG_LEVEL_TRACE defined in ftoptions.h

DEBUG build will allow to use FT2_DEBUG environment variable.

Debug support is useful not only for freetype developers, but also for troubleshooting font rendering problems in applications which use freetype.

> TABLE_VALIDATION_DESC=	Enable table validation modules

This descriptions is not informative.

The better one is:

Enable TrueType GX/AAT and OpenType table validation

Created attachment 175471 [details]
Proposed patch (since 412348 revision)

(In reply to comment #43)
> please add DEBUG port option (default off) to allow build freetype with
> FT_DEBUG_LEVEL_TRACE defined in ftoptions.h
Ok. I also added FT_DEBUG_MEMORY, as on Gentoo Linux:
https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/freetype/freetype-2.7-r2.ebuild?id=74b4be648e9329e9ab2e3c9174baa8626cd657ff#n87

(In reply to comment #44)
>> TABLE_VALIDATION_DESC=	Enable table validation modules
> This descriptions is not informative.
I used description from following link:
https://git.archlinux.org/svntogit/packages.git/tree/trunk/0001-Enable-table-validation-modules.patch?h=packages/freetype2&id=a86f6a5423659061598b079599970aeb0e13b9a8

(In reply to comment #44)
> The better one is:
> Enable TrueType GX/AAT and OpenType table validation
Possible to use "TrueType GX/AAT and OpenType table validation" description, without "Enable" word, because of some characters limit, when displaying options.

I obsoleted previous patch without pkg-message file. The maintainer may clarify it, if needed.

Created attachment 175507 [details]
Proposed patch (since 412348 revision)

Changed:
(%%DOCSDIR%%/reference/ft2-toc.html, if DOCS port's option was enabled).
To:
(%%DOCSDIR%%/reference/ft2-toc.html, if documentation was installed).
for files/pkg-message.in.

Comment 47 Antoine Brodin 2016-10-27 08:29:11 UTC

1 new failure on 11.0 amd64: graphics/inventor

http://package22.nyi.freebsd.org/data/110amd64-default-PR211201/2016-10-26_08h08m09s/logs/errors/inventor-2.1.5.p10_12.log

Created attachment 176248 [details]
Proposed patch (since 412348 revision, first variant for FT_Done_GlyphSlot)

Created attachment 176249 [details]
Proposed patch (since 412348 revision, second variant for FT_Done_GlyphSlot)

(In reply to comment #47)
> 1 new failure on 11.0 amd64: graphics/inventor
> http://package22.nyi.freebsd.org/data/110amd64-default-PR211201/2016-10-26_08h08m09s/logs/errors/inventor-2.1.5.p10_12.log
I investigated mentioned issue.
The 2.6.3 version used commented "-export-symbols $(EXPORTS_LIST)" line in builds/unix/unix-cc.in file:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/builds/unix/unix-cc.in?h=VER-2-6-3#n112

The 2.7 version have it uncommented:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/builds/unix/unix-cc.in?h=VER-2-7#n112

The --export-symbols option limits the list of exported symbols, generated by apinames program:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/tools/apinames.c?h=VER-2-7
https://www.gnu.org/software/gnulib/manual/html_node/Exported-Symbols-of-Shared-Libraries.html

For freetype2 v2.6.3:
-8<--
% readelf -Ds libfreetype.so.6.12.3 | grep FT_Done_GlyphSlot || echo not found
  533 247: 0000000000013b70   377    FUNC GLOBAL DEFAULT  11 FT_Done_GlyphSlot
-->8-

For freetype2 v2.7 (before patch):
-8<--
% readelf -Ds libfreetype.so.6.12.6 | grep FT_Done_GlyphSlot || echo not found
not found
-->8-

I created two variants of patches, which fixes this:
1) First variant in attachment #176248 [details], which comments "-export-symbols" option in builds/unix/unix-cc.in file:
-8<--
	@${REINPLACE_CMD} -e '/-export-symbols/s|^|#|' \
		${WRKSRC}/builds/unix/unix-cc.in
-->8-
-8<--
% readelf -Ds libfreetype.so.6.12.6 | grep FT_Done_GlyphSlot || echo not found
  555 247: 0000000000014490   377    FUNC GLOBAL DEFAULT  11 FT_Done_GlyphSlot
-->8-

2) Second variant in attachment #176249 [details], which adds FT_Done_GlyphSlot as exported function to include/freetype/ftglyph.h file:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/ftglyph.h?h=VER-2-7#n47
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/internal/ftobjs.h?h=VER-2-7#n601
-8<--
	@${REINPLACE_CMD} -e \
	'/FT_BEGIN_HEADER/s|$$|\${.newline}FT_EXPORT(void) FT_Done_GlyphSlot(FT_GlyphSlot slot);|' \
		${WRKSRC}/include/freetype/ftglyph.h
-->8-
-8<--
% readelf -Ds libfreetype.so.6.12.6 | grep FT_Done_GlyphSlot || echo not found
  161 157: 000000000000ebb0   377    FUNC GLOBAL DEFAULT  11 FT_Done_GlyphSlot
-->8-

The second variant is more correct, in my opinion, while first variant is more simple, but may create issues with other exported symbols.

Also, there is a possibility, that FT_Done_GlyphSlot is not required to use, according to documentation:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/internal/ftobjs.h?h=VER-2-7#n601
-8<--
Destroys a given glyph slot. Remember however that all slots are automatically destroyed with its parent. Using this function is not always mandatory.
-->8-
But I didn't check this for graphics/inventor port.

Created attachment 176250 [details]
Proposed patch for graphics/inventor (since 415499 revision)

(In reply to comment #51)
> But I didn't check this for graphics/inventor port.
I created a patch for graphics/inventor port (for attachment #175507 [details]), which comments FT_Done_GlyphSlot function usage.

Maybe devel/valgrind or other memory analysers may check how it works. Personally, I tried to run SceneViewer and it opened/closed without errors.

(In reply to comment #52)
> memory analysers
or memory analyzers.

Comment 54 Antoine Brodin 2016-10-29 06:26:26 UTC

Exp-run looks fine except graphics/inventor.
If graphics/inventor error is handled in graphics/inventor, there is no need for a new exp-run.

Comment 55 Jan Beich 2016-10-29 07:50:42 UTC

Comment on attachment 175507 [details]
Proposed patch (since 412348 revision)

Resuming review. I'm uneasy to land this per timeout due to the following style issues:

> +SUBPIXEL_HINTING_DESC=	Sub-pixel hinting support
> +V38_DESC=	v38 mode (Infinality code)
> +V38_VARS=	SUBPIXEL_HINTING_MODE+=1
> +V40_DESC=	v40 mode (minimal code, a.k.a. ClearType hinting)
> +V40_VARS=	SUBPIXEL_HINTING_MODE+=2
> +
> +.include <bsd.port.pre.mk>
> +
> +SELECTED_MODE=	r=0; for m in ${SUBPIXEL_HINTING_MODE};\
> +	do r=$$(($$r | $$m)); done; ${ECHO_CMD} $$r
> [...]
> +post-patch:
> +.if defined(SUBPIXEL_HINTING_MODE)
> +	@${REINPLACE_CMD} -e \
> +		's|^\(#define TT_CONFIG_OPTION_SUBPIXEL_HINTING\).*|\1 \
> +		${SELECTED_MODE:sh}|' \
> +		${WRKSRC}/include/freetype/config/ftoption.h
> +.else
> +	@${REINPLACE_CMD} -e \
> +		's|^\(#define TT_CONFIG_OPTION_SUBPIXEL_HINTING.*\)|/* \1 */|' \
> +		${WRKSRC}/include/freetype/config/ftoption.h
> +.endif

Can we avoid the complexity in this case? It'd even be 4 lines shorter.

  SUBPIXEL_HINTING_DESC=	Sub-pixel hinting support
  V38_DESC=	v38 mode (Infinality code)
  V40_DESC=	v40 mode (minimal code, a.k.a. ClearType hinting)

  .include <bsd.port.options.mk>

  .if ${PORT_OPTIONS:MV38} && ${PORT_OPTIONS:MV40}
  CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=3
  .elif ${PORT_OPTIONS:MV40}
  CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=2
  .elif ${PORT_OPTIONS:MV38}
  CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=1
  .endif

  post-patch:
  # warning: 'TT_CONFIG_OPTION_SUBPIXEL_HINTING' macro redefined [-Wmacro-redefined]
	  @${REINPLACE_CMD} -e '/^#define TT_CONFIG_OPTION_SUBPIXEL_HINTING/d' \
		  ${WRKSRC}/include/freetype/config/ftoption.h

> +post-patch-DEBUG-on:
> +	@${SED} -i '.d.bak' -e 's|.*\(#define FT_DEBUG_LEVEL_TRACE\).*|\1| ; \
> +		s|.*\(#define FT_DEBUG_MEMORY\).*|\1|' \
> +		${WRKSRC}/include/freetype/config/ftoption.h
> +

- Use "-e" option to split commands rather than ";" to avoid tracking open quotes
- Don't use SED when modifying in-place
- Make backup suffix a bit more verbose than just one confusing letter

Here's what I have in mind:

  post-patch-DEBUG-on:
	  @${REINPLACE_CMD} -i '.bak.DEBUG' \
		  -e 's/.*\(#define FT_DEBUG_LEVEL_TRACE\).*/\1/' \
		  -e 's/.*\(#define FT_DEBUG_MEMORY\).*/\1/' \
		  ${WRKSRC}/include/freetype/config/ftoption.h

(In reply to comment #55)
> Can we avoid the complexity in this case?
> It'd even be 4 lines shorter.
What you proposed is the same complexity, but with static variant, which may require to extend number of checks (2^n - 1, where n is 2 currently) for options, in case of more modes.
For three options (1, 2, 4), for example: (1; 2; 4; 1, 2; 1, 4; 2, 4; 1, 2, 4)
-8<--
.if ${PORT_OPTIONS:MV38} && ${PORT_OPTIONS:MV40} && ${PORT_OPTIONS:MV4X}
CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=7
.elif ${PORT_OPTIONS:MV40} && ${PORT_OPTIONS:MV4X}
CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=6
.elif ${PORT_OPTIONS:MV38} && ${PORT_OPTIONS:MV4X}
CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=5
.elif ${PORT_OPTIONS:MV4X}
CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=4
.elif ${PORT_OPTIONS:MV38} && ${PORT_OPTIONS:MV40}
CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=3
.elif ${PORT_OPTIONS:MV40}
CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=2
.elif ${PORT_OPTIONS:MV38}
CFLAGS+=	-DTT_CONFIG_OPTION_SUBPIXEL_HINTING=1
.endif
-->8-
where for proposed case in attachment #175507 [details]:
-8<--
V4X_VARS=	SUBPIXEL_HINTING_MODE+=4
-->8-

(In reply to comment #55)
> post-patch:
>  # warning: 'TT_CONFIG_OPTION_SUBPIXEL_HINTING' macro redefined [-Wmacro-redefined]
>	  @${REINPLACE_CMD} -e '/^#define TT_CONFIG_OPTION_SUBPIXEL_HINTING/d' \
>		  ${WRKSRC}/include/freetype/config/ftoption.h
Moreover, you proposed to use CFLAGS and remove related define from ftoption.h file. The warning shows where your approach is wrong. Even on Gentoo Linux, while using some of your proposed methods, they decide to patch ftoption.h file directly:
https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/freetype/freetype-2.7-r2.ebuild?id=838c92297214d6330844df0b15ba61c33c27c430#n69

> - Don't use SED when modifying in-place
> - Make backup suffix a bit more verbose than just one confusing letter
This is not justified.
The reason to use ${SED} instead of ${REINPLACE_CMD} with different backup extension (i.e. -i '.d.bak')  is following:
-8<--
% make -C /usr/ports/print/freetype -V REINPLACE_CMD
/usr/bin/sed -i.bak
% make -C /usr/ports/print/freetype -V SED
/usr/bin/sed
-->8-

(In reply to comment #55)
> - Use "-e" option to split commands rather than ";" to avoid tracking open quotes
This is just your preference, in my opinion. It also works with ";".

(In reply to comment #55)
> Resuming review. I'm uneasy to land this per timeout
I guess, better to ask someone from gnome@ team about this, e.g. kwm@, which is active, based on commit in ports r424830.

CC: kwm.

Created attachment 176263 [details]
Proposed patch (since 412348 revision) with proposals from Jan Beich

To Jan Beich:
I attached some variant with your proposals.

To Koop Mast:
The Jan Beich's proposals about static approach may look reasonable, based on two options for sub-pixel hinting modes, currently. If there will be more modes, the gnome@ team (or other people) may (re)consider to use dynamic approach (some of which I already proposed in attachment #175507 [details]).

(In reply to comment #55)
> - Make backup suffix a bit more verbose than just one confusing letter
If it still confusing, you may change other extensions (for attachment #176263 [details]) on your discretion, in case of commit.

(In reply to comment #56)
> The warning shows where your approach is wrong.
To Jan Beich:
This is not quite true from my side. You just proposed to remove TT_CONFIG_OPTION_SUBPIXEL_HINTING define for ftoption.h file, because of CFLAGS usage. You wrote description of warning, for which the sed patch was used. I apologize.

Created attachment 176284 [details]
Proposed patch (since 412348 revision)

I decided to not propose changes for LCD_FILTERING options, because of possible legal issues with patents (see ports r373492 for approved version by core@).

I obsoleted previous versions, just in case.

Created attachment 176285 [details]
Proposed patch (since 412348 revision) with proposals from Jan Beich

Created attachment 176286 [details]
Proposed patch (since 412348 revision, v2) with proposals from Jan Beich

Comment 63 Jan Beich 2016-10-29 18:43:08 UTC

(In reply to lightside from comment #56)
> (In reply to comment #55)
>> Can we avoid the complexity in this case?
>> It'd even be 4 lines shorter.
> What you proposed is the same complexity, but with static variant, which may
> require to extend number of checks (2^n - 1, where n is 2 currently) for
> options, in case of more modes.

It'd be better to ask upstream as gnome@ (or kwm@) are probably just as clueless whether there'd would be more options in future. I don't expect that due to increased maintenance compared to always compiling in the code. For one, v40 only exist because ...

   If someone finds ways to make older fonts render better without
   introducing lists or overly complex hacks, I'm interested.

https://www.freetype.org/freetype2/docs/subpixel-hinting.html

>> - Don't use SED when modifying in-place
>> - Make backup suffix a bit more verbose than just one confusing letter
> This is not justified.
> The reason to use ${SED} instead of ${REINPLACE_CMD} with different backup
> extension (i.e. -i '.d.bak')  is following:
> -8<--
> % make -C /usr/ports/print/freetype -V REINPLACE_CMD
> /usr/bin/sed -i.bak
> % make -C /usr/ports/print/freetype -V SED
> /usr/bin/sed
> -->8-

I'm arguing about semantic meaning of editing file(s) in-place: REINPLACE_CMD carries it while SED does not. Adjusting (or removing) backup suffix is more popular via REINPLACE_CMD:

  $ cd /usr/ports
  $ git grep 'SED.* -i ' | wc -l
       125
  $ git grep 'REINPLACE_CMD.* -i ' | wc -l
       263

Also, keeping '.bak' component the last avoids confusion with '.d' or '.debug' being a non-backed up file extension.

> (In reply to comment #55)
>> - Use "-e" option to split commands rather than ";" to avoid tracking open quotes
> This is just your preference, in my opinion. It also works with ";".

";" requires tracking open quotes whenever a new substitution is added instead of being just one "+" line in a diff. "-e" is *probably* more popular in ports but I'm biased. Actually measuring usage may require a careful regexp to parse newlines, (single/double) quotes, "s" command syntax.

Comment 64 Jan Beich 2016-10-29 23:41:06 UTC

(In reply to lightside from comment #56)
>(In reply to comment #55)
>> Resuming review. I'm uneasy to land this per timeout
> I guess, better to ask someone from gnome@ team about this,
> e.g. kwm@, which is active, based on commit in ports r424830.

After gnome@ neglected this bug for >60 comments... Did you miss maintainer timeout?

(In reply to lightside from comment #52)
> Maybe devel/valgrind or other memory analyzers may check how it works. 
> Personally, I tried to run SceneViewer and it opened/closed without errors.

Thanks for testing, so attachment 176250 [details] can land under portmgr blanket. Modifying ABI without upstream feedback shouldn't be taken lightly unless the fallout is more than one ancient consumer.

(In reply to comment #64)
> Did you miss maintainer timeout?
Do you want to discuss about this or to know my opinion? I don't think so.

The thing is that you proposed new changes in comment #55 (and I don't say about some style changes). There are other possible interested people who may review them, e.g. maintainer (as a gnome@ team), PR's creator, people who participated in this PR, etc. If you confident, that there is no need to wait more time and what you proposed is right, then you already (was) assigned to decide about this.
Personally, I already proposed working variant(s) many weeks ago. Thanks for asking for exp-run and antoine@, who did it. Looks like, this helped to process this PR.

Created attachment 176307 [details]
Proposed patch for graphics/inventor (since 415499 revision)

- Bump PORTREVISION for graphics/inventor port.

I also tested installed programs (without command line arguments) by graphics/inventor port in /usr/local/bin directory for freetype2 v2.6.3 (with and without patch) and v2.7 (with patch). The results are the same:
SceneViewer: Tested gviewIcons.iv, jackInTheBox.iv, windmill.iv from /usr/local/demos/data/Inventor. Exit without errors.
iv2toiv1: Permission denied.
ivcat: Exit without errors.
ivdowngrade: Exit without errors.
ivfix: Exit without errors.
ivinfo: Exit without errors.
ivview: Segmentation fault (core dumped).

Comment 67 Koop Mast 2016-10-30 18:26:52 UTC

grab and start working on getting this update in the tree. Please give me a bit on getting up to date on all the changes.

Comment 68 Jan Beich 2016-11-26 15:05:26 UTC

Hopefully, this lands before 2017Q1 branches unless you plan to support 2.6.x until April next year.

Since comment 54 print/freetype2 lost 1 consumer

  deskutils/xfce4-notification-daemon

and gained 10 more

  cad/solvespace
  cad/sp2sp
  games/voxelands
  lang/neko
  sysutils/xfce4-mount-plugin
  x11-fonts/fontmatrix
  x11-toolkits/c++-gtk-utils
  x11-toolkits/fox16
  x11-toolkits/fox17
  x11-toolkits/gtk-sharp20

It'd be nice if you check them for build regressions as well.

Created attachment 178524 [details]
Proposed patch (since 412348 revision)

Updated to 2.7.1 version:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/CHANGES?id=VER-2-7-1#n2

- Changed SED to REINPLACE_CMD variable, just in case
- Removed some tabs for files/pkg-message.in file

Created attachment 178525 [details]
Proposed patch (since 412348 revision) with proposals from Jan Beich

Created attachment 178526 [details]
Proposed patch (since 412348 revision, v2) with proposals from Jan Beich

Comment 72 Koop Mast 2017-02-13 15:57:45 UTC

Sorry for leaving this hanging for so long. I don't have any excuses.

I'm currently using the new version, and will poudriere test the ports listed in this PR before committing it.

Regarding the patch, the only thing I changed is that I changed the V38/V40 GROUP to a MULTI. Because looking at the logic in the SUBPIXEL_HINTING_MODE selection there is not a !V38 && !V40 case. With Multi it 1 or more need to be selected.

Also the description of the V38 and V40 options doesn't sit that well with me, maybe improve it in some way? I can't think of something to do it, if nothing can be suggested the current will do.

(In reply to comment #72)
> Regarding the patch, the only thing I changed is that I changed the V38/V40
> GROUP to a MULTI. Because looking at the logic in the SUBPIXEL_HINTING_MODE
> selection there is not a !V38 && !V40 case. With Multi it 1 or more need to
> be selected.
The OPTIONS_GROUP was used, because possible to not define TT_CONFIG_OPTION_SUBPIXEL_HINTING, e.g. when V38=off and V40=off, in which case v35 (no subpixel hinting) mode may be used by default internally:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/ftttdrv.h?h=VER-2-7-1#n192
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/ftttdrv.h?h=VER-2-7-1#n316
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/truetype/ttobjs.c?h=VER-2-7-1#n1278
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/truetype/ttinterp.c?h=VER-2-7-1#n52

The value of TT_CONFIG_OPTION_SUBPIXEL_HINTING defines TT_SUPPORT_SUBPIXEL_HINTING_INFINALITY and/or TT_SUPPORT_SUBPIXEL_HINTING_MINIMAL:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?h=VER-2-7-1#n889
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?h=VER-2-7-1#n893
which used in other areas, e.g.:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/truetype/ttdriver.c?h=VER-2-7-1#n98
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/truetype/ttgload.c?h=VER-2-7-1#n1359

There is some description about this:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?h=VER-2-7-1#n608
in the following part:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?h=VER-2-7-1#n665
-8<--
  /* By undefining these, you get rendering behavior like on Windows       */
  /* without ClearType, i.e., Windows XP without ClearType enabled and     */
  /* Win9x (interpreter version v35).  Or not, depending on how much       */
  /* hinting blood and testing tears the font designer put into a given    */
  /* font.  If you define one or both subpixel hinting options, you can    */
  /* switch between between v35 and the ones you define.
-->8-

Similar approach used for Gentoo Linux, for example:
1. Undefine "TT_CONFIG_OPTION_SUBPIXEL_HINTING  2"
https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/freetype/freetype-2.7.1-r1.ebuild?id=85fe078abbaf6e0f0c5860583c808ead0a19f324#n69
which related to following part:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?h=VER-2-7-1#n678
2. Then define what was enabled:
https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/freetype/freetype-2.7.1-r1.ebuild?id=85fe078abbaf6e0f0c5860583c808ead0a19f324#n72

(In reply to comment #74)
> 1. Undefine "TT_CONFIG_OPTION_SUBPIXEL_HINTING  2"
Not undefine, but comment this part, actually:
https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/freetype/freetype-2.7.1-r1.ebuild?id=85fe078abbaf6e0f0c5860583c808ead0a19f324#n61

Created attachment 179958 [details]
Proposed patch (since 412348 revision)

Added sed patch to check defined TT_CONFIG_OPTION_SUBPIXEL_HINTING, e.g.:
-8<--
#if defined(TT_CONFIG_OPTION_SUBPIXEL_HINTING) && (TT_CONFIG_OPTION_SUBPIXEL_HINTING & 1)
#define  TT_SUPPORT_SUBPIXEL_HINTING_INFINALITY
#endif

#if defined(TT_CONFIG_OPTION_SUBPIXEL_HINTING) && (TT_CONFIG_OPTION_SUBPIXEL_HINTING & 2)
#define  TT_SUPPORT_SUBPIXEL_HINTING_MINIMAL
#endif
#endif
-->8-
instead of:
-8<--
#if TT_CONFIG_OPTION_SUBPIXEL_HINTING & 1
#define  TT_SUPPORT_SUBPIXEL_HINTING_INFINALITY
#endif

#if TT_CONFIG_OPTION_SUBPIXEL_HINTING & 2
#define  TT_SUPPORT_SUBPIXEL_HINTING_MINIMAL
#endif
-->8-
in include/freetype/config/ftoption.h file.

Compiler's warning about which was found, when building multimedia/ffmpeg port.

Other variants of patches was obsoleted (but may be used for reference, if needed).

Created attachment 179969 [details]
Proposed patch (since 412348 revision)

Cosmetic fixes.
Sorted some options.
Added some comments.
Changed "PNG_CONFIGURE_OFF=	--without-png" to "PNG_CONFIGURE_WITH=	png", as Koop Mast suggested.

Created attachment 179970 [details]
Proposed patch (since 412348 revision, v2) with proposals from Jan Beich

Returned some alternative variant, just in case.

(In reply to comment #76)
> Compiler's warning about which was found, when building
> multimedia/ffmpeg port.
There was following compiler's warning, when TT_CONFIG_OPTION_SUBPIXEL_HINTING not defined (V38=off and V40=off):
-8<--
In file included from libavfilter/vf_drawtext.c:69:
In file included from /usr/local/include/freetype2/freetype/freetype.h:33:
In file included from /usr/local/include/freetype2/freetype/config/ftconfig.h:42:
/usr/local/include/freetype2/freetype/config/ftoption.h:889:5: warning: 'TT_CONFIG_OPTION_SUBPIXEL_HINTING' is not
      defined, evaluates to 0 [-Wundef]
#if TT_CONFIG_OPTION_SUBPIXEL_HINTING & 1
    ^
/usr/local/include/freetype2/freetype/config/ftoption.h:893:5: warning: 'TT_CONFIG_OPTION_SUBPIXEL_HINTING' is not
      defined, evaluates to 0 [-Wundef]
#if TT_CONFIG_OPTION_SUBPIXEL_HINTING & 2
    ^
-->8-

The compiler's warning in comment #79 may create impression, that possible to define TT_CONFIG_OPTION_SUBPIXEL_HINTING to 0, instead of comment/undefine it, in case of V38=off and V40=off. 
I created some test.c program to test this:
-8<--
#include <stdio.h>

#ifndef CHECK
#if defined(TT_CONFIG_OPTION_SUBPIXEL_HINTING) && (TT_CONFIG_OPTION_SUBPIXEL_HINTING & 1)
#define  TT_SUPPORT_SUBPIXEL_HINTING_INFINALITY
#endif

#if defined(TT_CONFIG_OPTION_SUBPIXEL_HINTING) && (TT_CONFIG_OPTION_SUBPIXEL_HINTING & 2)
#define  TT_SUPPORT_SUBPIXEL_HINTING_MINIMAL
#endif
#else
#if TT_CONFIG_OPTION_SUBPIXEL_HINTING & 1
#define  TT_SUPPORT_SUBPIXEL_HINTING_INFINALITY
#endif

#if TT_CONFIG_OPTION_SUBPIXEL_HINTING & 2
#define  TT_SUPPORT_SUBPIXEL_HINTING_MINIMAL
#endif
#endif

int main() {
	printf("default\n");
#ifdef TT_SUPPORT_SUBPIXEL_HINTING_INFINALITY
	printf("infinality\n");
#endif
#ifdef TT_SUPPORT_SUBPIXEL_HINTING_MINIMAL
	printf("minimal\n");
#endif
	return 0;
}
-->8-

with following results:
-8<--
% cc -o test test.c && ./test
default
% cc -DTT_CONFIG_OPTION_SUBPIXEL_HINTING=0 -o test test.c && ./test
default
% cc -DTT_CONFIG_OPTION_SUBPIXEL_HINTING=1 -o test test.c && ./test
default
infinality
% cc -DTT_CONFIG_OPTION_SUBPIXEL_HINTING=2 -o test test.c && ./test
default
minimal
% cc -DTT_CONFIG_OPTION_SUBPIXEL_HINTING=3 -o test test.c && ./test
default
infinality
minimal
% cc -DCHECK -Wundef -o test test.c && ./test
test.c:12:5: warning: 'TT_CONFIG_OPTION_SUBPIXEL_HINTING' is not defined, evaluates to 0 [-Wundef]
#if TT_CONFIG_OPTION_SUBPIXEL_HINTING & 1
    ^
test.c:16:5: warning: 'TT_CONFIG_OPTION_SUBPIXEL_HINTING' is not defined, evaluates to 0 [-Wundef]
#if TT_CONFIG_OPTION_SUBPIXEL_HINTING & 2
    ^
2 warnings generated.
default
-->8-

But in case of freetype2 source code, there is some check for defined TT_CONFIG_OPTION_SUBPIXEL_HINTING:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/truetype/ttgload.c?h=VER-2-7-1#n1355
-8<--
#ifdef TT_CONFIG_OPTION_SUBPIXEL_HINTING
    TT_Driver driver = (TT_Driver)FT_FACE_DRIVER( loader->face );
#endif
-->8-

So, defining TT_CONFIG_OPTION_SUBPIXEL_HINTING to 0 may create different result(s).

Moreover, the freetype2's developer(s) commented/undefined TT_CONFIG_OPTION_SUBPIXEL_HINTING for 2.6.5 version:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?h=VER-2-6-5#n633
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/config/ftoption.h?h=VER-2-6-5#n845
in preparation for 2.7.x series:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/docs/CHANGES?h=VER-2-6-5#n10

This is why TT_CONFIG_OPTION_SUBPIXEL_HINTING was commented/undefined for V38=off and V40=off case.

Comment 81 commit-hook 2017-03-08 15:26:54 UTC

A commit references this bug:

Author: kwm
Date: Wed Mar  8 15:26:29 UTC 2017
New revision: 435690
URL: https://svnweb.freebsd.org/changeset/ports/435690

Log:
  Update freetype2 to 2.7.1.

  * List licenses
  * Add DEBUG, DOCS, TABLE_VALIDATION and SUBPIXEL_HINTING options.
  * Sort options lines by name.
  * Add pkg-message describing how certain options can be overwritten
    at run time, like the subpixel hinting engine.
  * Fix the build of graphics/inventor.

  PR:		ports/211201
  Submitted by:	Piotr Kubaj <pkubaj@anongoth.pl>
  Discussed with:	jbeigh@, lightside@gmx.com
  Exp-run:	antoine@ (earlier version)

Changes:
  head/graphics/inventor/Makefile
  head/print/freetype2/Makefile
  head/print/freetype2/distinfo
  head/print/freetype2/files/patch-builds_unix_detect.mk
  head/print/freetype2/files/pkg-message.in
  head/print/freetype2/pkg-descr
  head/print/freetype2/pkg-plist

Comment 82 Koop Mast 2017-03-08 15:28:18 UTC

Committed thanks!

Created attachment 180685 [details]
Patch with cosmetic fixes (since 435690 revision)

(In reply to comment #82)
Thanks for commit.

Nevertheless, please check your changes with ports-mgmt/portlint. There are warnings:
WARN: /usr/ports/print/freetype2/pkg-descr: exceeds 24 lines, make it shorter if possible.(currently 25 lines)
WARN: /usr/ports/print/freetype2/pkg-descr: includes lines that exceed 80 characters.
WARN: Makefile: [53]: whitespace before end of line.

Also, value of V40_DESC doesn't fit in option's "window" for added "faster" word (when using `make config` command), but this may be ignored.

I attached possible patch with cosmetic fixes and pkg-descr changes, based on description from documentation:
https://www.freetype.org/freetype2/docs/index.html

(In reply to comment #81)
> Discussed with:	jbeigh@
Just for note:
I guess, it was meant jbeich@, unless his email address wasn't changed (or has other alternative(s)).