freetype-devel
[Top][All Lists]
Advanced

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

Re: Contributing COLRv1


From: Werner LEMBERG
Subject: Re: Contributing COLRv1
Date: Mon, 11 Jan 2021 17:32:11 +0100 (CET)

> I took some notes on what formatting details I need to pay attention
> to in the future (in particular with regards to argument lists,
> double spaces after types and between doc sentences, use of tilde in
> docs, and I took some hints on how to phrase commit messages with
> regards to distinction between header and impl files.).

Thanks!

> In short, all changes look good to me - from my point of view,
> they're good to go for merging!

OK.  Merging will probably take some time since I'm busy with other
things.

Some questions.

(1) How can your code be tested?  It should be eventually fuzzed, too.

(2) Is a test font already available?  In this case, could you
    contribute code to Armin's repository to enable fuzzing?

      https://github.com/freetype/freetype2-testing

(3) Is there a simple way to actually see COLR v1 fonts in action?
    For example, would it be possible to have a Qt demo program?

> One really minor nit, and an entirely optional change: In the 4th
> commit in the series, there are reformatting fixes changing code
> that was introduced in code in the third commit of the series.  In
> other words commit 223edbfc0 contains
> 
> @@ -389,7 +684,7 @@
>      p = (FT_Byte*)( colr->base_glyphs_v1 +
>                      base_glyph_v1_record.paint_offset );
> -    if ( p >= (FT_Byte*)( colr->table + colr->table_size ) )
> +    if ( p >= ( (FT_Byte*)colr->table + colr->table_size ) )
>        return 0;
> 
> which may be better placed in cd3c13fa51d5, the previous commit, which
> introduces this code.

I've noticed that, too.  However, Savannah git doesn't allow forced
pushs to branches; I will fix this (and some other minor details) when
merging into 'master'.


    Werner



reply via email to

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