<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Sep 14, 2017 at 2:54 PM, Adrian Johnson <span dir="ltr"><<a href="mailto:ajohnson@redneon.com" target="_blank">ajohnson@redneon.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On 15/09/17 05:52, Matthias Clasen wrote:<br>
> On Thu, Sep 14, 2017 at 12:37 PM, Behdad Esfahbod<br>
</span><span class="gmail-">> <<a href="mailto:behdad.esfahbod@gmail.com">behdad.esfahbod@gmail.com</a> <mailto:<a href="mailto:behdad.esfahbod@gmail.com">behdad.esfahbod@gmail.<wbr>com</a>>> wrote:<br>
><br>
>     Thanks Matthias.  I took a look at the patch.  Looks good!  Minor<br>
>     comments:<br>
><br>
>     - Moving the format towards CSS by default (no comma, no equal sign;<br>
>     just spaces!) while accepting both is what I do in HarfBuzz. Maybe<br>
>     advertise the same here,  UPDATE: I was wrong.  CSS uses comma as<br>
>     well.  It's the equal sign that they don't use, they use space.  We<br>
>     should accept both.<br>
><br>
><br>
> I've made the parser a bit more flexible. It now accepts variations like<br>
><br>
> wdth=200,wght=300<br>
> wdth 200, wght=300<br>
> wdth 200 , wght 300<br>
<br>
</span>Does it work with decimal commas?<br>
<div class="gmail-HOEnZb"><div class="gmail-h5"></div></div></blockquote><div><br></div><div>Good point.  HarfBuzz has same problem.  glib has g_ascii_strtod() that always uses '.' as decimal separator.  We need something like that in cairo and HarfBuzz.  Such a pain :(.</div><div><br></div><div>  <a href="https://git.gnome.org/browse/glib/tree/glib/gstrfuncs.c#n687">https://git.gnome.org/browse/glib/tree/glib/gstrfuncs.c#n687</a></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">><br>
><br>
>     - Maybe make the variations member be "const char *" instead of<br>
>     "char *".<br>
><br>
><br>
> But I'm strdup'ing and freeing it. The const will get in the way there.<br>
>  <br>
><br>
>     - In +cairo_font_options_set_<wbr>variations(), typically in these kinds<br>
>     of functions I like copying then freeing the old value, such that<br>
>     the input can be the return value of get_variations() on the same<br>
>     object.<br>
><br>
><br>
> Done<br>
>  <br>
><br>
>     -<br>
>     (((uint8_t)p[0])<<24)|(((<wbr>uint8_t)p[1])<<16)|(((uint8_t)<wbr>p[2])<<8)|((uint8_t)p[3]); <br>
>     FreeType has a macro for that: FT_MAKE_TAG()<br>
><br>
><br>
> Thanks for the reminder. Done.<br>
><br>
>     (Generally I find it easier pushing a tree on github and reviewing<br>
>     there; also because I can just fix stuff and push my own tree.)<br>
><br>
><br>
> You can find the new version here:<br>
><br>
> <a href="https://github.com/matthiasclasen/cairo/commits/wip/matthiasc/font-variations" rel="noreferrer" target="_blank">https://github.com/<wbr>matthiasclasen/cairo/commits/<wbr>wip/matthiasc/font-variations</a><br>
><br>
><br>
><br>
<br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature">behdad<br><a href="http://behdad.org/" target="_blank">http://behdad.org/</a></div>
</div></div>