[Buildroot] [PATCH] freetype: Add config option for bzip2 compressed font support

[Buildroot] [PATCH] freetype: Add config option for bzip2 compressed font support

[Maarten ter Huurne]

If bzip2 is available, it might still be preferable to not include
support for bzip2 compressed fonts, for example to reduce the number
of libraries loaded when running a GUI application.

Signed-off-by: Maarten ter Huurne <maarten@treewalker.org>
---
 package/freetype/Config.in   | 9 +++++++++
 package/freetype/freetype.mk | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/package/freetype/Config.in b/package/freetype/Config.in
index ded8738..f245b14 100644
--- a/package/freetype/Config.in
+++ b/package/freetype/Config.in
@@ -4,3 +4,12 @@ config BR2_PACKAGE_FREETYPE
 	  a free, high-quality and portable font engine.
 
 	  http://www.freetype.org/
+
+if BR2_PACKAGE_FREETYPE
+
+config BR2_PACKAGE_FREETYPE_BZIP2
+	bool "support bzip2 compressed fonts"
+	depends on BR2_PACKAGE_BZIP2
+	default y
+
+endif
diff --git a/package/freetype/freetype.mk b/package/freetype/freetype.mk
index b85d0ec..b2d999f 100644
--- a/package/freetype/freetype.mk
+++ b/package/freetype/freetype.mk
@@ -37,7 +37,7 @@ else
 FREETYPE_CONF_OPTS += --without-zlib
 endif
 
-ifeq ($(BR2_PACKAGE_BZIP2),y)
+ifeq ($(BR2_PACKAGE_FREETYPE_BZIP2),y)
 FREETYPE_DEPENDENCIES += bzip2
 FREETYPE_CONF_OPTS += --with-bzip2
 else
-- 
1.8.4.5

[Buildroot] [PATCH] freetype: Add config option for bzip2 compressed font support

[Maxime Hadjinlian]

Hi Maarten, all

On Sat, Oct 18, 2014 at 4:39 AM, Maarten ter Huurne
<maarten@treewalker.org> wrote:
> If bzip2 is available, it might still be preferable to not include
> support for bzip2 compressed fonts, for example to reduce the number
> of libraries loaded when running a GUI application.
>
> Signed-off-by: Maarten ter Huurne <maarten@treewalker.org>
> ---
>  package/freetype/Config.in   | 9 +++++++++
>  package/freetype/freetype.mk | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/package/freetype/Config.in b/package/freetype/Config.in
> index ded8738..f245b14 100644
> --- a/package/freetype/Config.in
> +++ b/package/freetype/Config.in
> @@ -4,3 +4,12 @@ config BR2_PACKAGE_FREETYPE
>           a free, high-quality and portable font engine.
>
>           http://www.freetype.org/
> +
> +if BR2_PACKAGE_FREETYPE
> +
> +config BR2_PACKAGE_FREETYPE_BZIP2
> +       bool "support bzip2 compressed fonts"
I would have had something like:
bool "Enable compressed fonts"
help
  Enable supports for bzip2 compressed fonts

> +       depends on BR2_PACKAGE_BZIP2
> +       default y
Why would we want this to 'default y' ? It kind of goes against what
you state in the commit log, you don't want to have compressed fonts
by default even if bzip2 is selected, right ?
Also, if the users selects this options, it should select the package bzip2
> +
> +endif
> diff --git a/package/freetype/freetype.mk b/package/freetype/freetype.mk
> index b85d0ec..b2d999f 100644
> --- a/package/freetype/freetype.mk
> +++ b/package/freetype/freetype.mk
> @@ -37,7 +37,7 @@ else
>  FREETYPE_CONF_OPTS += --without-zlib
>  endif
>
> -ifeq ($(BR2_PACKAGE_BZIP2),y)
> +ifeq ($(BR2_PACKAGE_FREETYPE_BZIP2),y)
>  FREETYPE_DEPENDENCIES += bzip2
>  FREETYPE_CONF_OPTS += --with-bzip2
>  else
> --
> 1.8.4.5
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH] freetype: Add config option for bzip2 compressed font support

[Arnout Vandecappelle]

On 18/10/14 04:39, Maarten ter Huurne wrote:
> If bzip2 is available, it might still be preferable to not include
> support for bzip2 compressed fonts, for example to reduce the number
> of libraries loaded when running a GUI application.

 I don't agree with this. The impact will be really small, both for memory size
and for load time - especially because any application using freetype will most
likely load much larger GUI libraries. If you're working at such details, you'll
unavoidably have to do some manual hacks to get it even better. So I don't think
it makes sense to add a config option for it - we try to keep the number of
config options in check.

 So unless you can show me a use case where it makes a real impact, it's a NACK.


> 
> Signed-off-by: Maarten ter Huurne <maarten@treewalker.org>
> ---
>  package/freetype/Config.in   | 9 +++++++++
>  package/freetype/freetype.mk | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/package/freetype/Config.in b/package/freetype/Config.in
> index ded8738..f245b14 100644
> --- a/package/freetype/Config.in
> +++ b/package/freetype/Config.in
> @@ -4,3 +4,12 @@ config BR2_PACKAGE_FREETYPE
>  	  a free, high-quality and portable font engine.
>  
>  	  http://www.freetype.org/
> +
> +if BR2_PACKAGE_FREETYPE
> +
> +config BR2_PACKAGE_FREETYPE_BZIP2
> +	bool "support bzip2 compressed fonts"
> +	depends on BR2_PACKAGE_BZIP2
> +	default y

 In reply to Maxime's comment: it should indeed default to yes, so the default
keeps the behaviour we had up to now.


 Regards,
 Arnout

> +
> +endif
> diff --git a/package/freetype/freetype.mk b/package/freetype/freetype.mk
> index b85d0ec..b2d999f 100644
> --- a/package/freetype/freetype.mk
> +++ b/package/freetype/freetype.mk
> @@ -37,7 +37,7 @@ else
>  FREETYPE_CONF_OPTS += --without-zlib
>  endif
>  
> -ifeq ($(BR2_PACKAGE_BZIP2),y)
> +ifeq ($(BR2_PACKAGE_FREETYPE_BZIP2),y)
>  FREETYPE_DEPENDENCIES += bzip2
>  FREETYPE_CONF_OPTS += --with-bzip2
>  else
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH] freetype: Add config option for bzip2 compressed font support

[Maarten ter Huurne]

On Saturday 18 October 2014 15:12:11 Arnout Vandecappelle wrote:
> On 18/10/14 04:39, Maarten ter Huurne wrote:
> > If bzip2 is available, it might still be preferable to not include
> > support for bzip2 compressed fonts, for example to reduce the number
> > of libraries loaded when running a GUI application.
> 
>  I don't agree with this. The impact will be really small, both for memory
> size and for load time - especially because any application using
> freetype will most likely load much larger GUI libraries. If you're
> working at such details, you'll unavoidably have to do some manual hacks
> to get it even better. So I don't think it makes sense to add a config
> option for it - we try to keep the number of config options in check.
> 
>  So unless you can show me a use case where it makes a real impact, it's a
> NACK.

The menu we're using (GMenu2X) uses FreeType via SDL_ttf, not via a large 
GUI toolkit. Dropping the libbzip2 dependency reduces the number of 
libraries loaded from 12 to 11. I haven't benchmarked how many milliseconds 
of a difference that makes in startup time.

Note that the only place where bzip2 compression is actually supported by 
FreeType is when opening PCF files. Disabling the feature has no effect on 
support for other font formats such as TTF.

Bye,
		Maarten

[Buildroot] [PATCH] freetype: Add config option for bzip2 compressed font support

[Arnout Vandecappelle]

On 19/10/14 02:49, Maarten ter Huurne wrote:
> On Saturday 18 October 2014 15:12:11 Arnout Vandecappelle wrote:
>> On 18/10/14 04:39, Maarten ter Huurne wrote:
>>> If bzip2 is available, it might still be preferable to not include
>>> support for bzip2 compressed fonts, for example to reduce the number
>>> of libraries loaded when running a GUI application.
>>
>>  I don't agree with this. The impact will be really small, both for memory
>> size and for load time - especially because any application using
>> freetype will most likely load much larger GUI libraries. If you're
>> working at such details, you'll unavoidably have to do some manual hacks
>> to get it even better. So I don't think it makes sense to add a config
>> option for it - we try to keep the number of config options in check.
>>
>>  So unless you can show me a use case where it makes a real impact, it's a
>> NACK.
> 
> The menu we're using (GMenu2X) uses FreeType via SDL_ttf, not via a large 
> GUI toolkit. Dropping the libbzip2 dependency reduces the number of 
> libraries loaded from 12 to 11. I haven't benchmarked how many milliseconds 
> of a difference that makes in startup time.
> 
> Note that the only place where bzip2 compression is actually supported by 
> FreeType is when opening PCF files. Disabling the feature has no effect on 
> support for other font formats such as TTF.

 Okay, let's leave it up to the Benevolent Dictator to make the call. If it's a
go, and the depends on is changed into a select like Maxime requested, it gets
my Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>



 Regards,
 Arnout
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH] freetype: Add config option for bzip2 compressed font support

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

>>> So unless you can show me a use case where it makes a real impact, it's a
 >>> NACK.
 >> 
 >> The menu we're using (GMenu2X) uses FreeType via SDL_ttf, not via a large 
 >> GUI toolkit. Dropping the libbzip2 dependency reduces the number of 
 >> libraries loaded from 12 to 11. I haven't benchmarked how many milliseconds 
 >> of a difference that makes in startup time.
 >> 
 >> Note that the only place where bzip2 compression is actually supported by 
 >> FreeType is when opening PCF files. Disabling the feature has no effect on 
 >> support for other font formats such as TTF.

 >  Okay, let's leave it up to the Benevolent Dictator to make the call. If it's a
 > go, and the depends on is changed into a select like Maxime requested, it gets
 > my Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Sorry - We discussed it at the Buildroot developer days, and agreed that
it it isn't worth the trouble to have an explicit option for this.

-- 
Bye, Peter Korsgaard