Re: [ft-devel] New Infinality Release
Eric, In your integrated code you use FT_UInt for the rule scaling factor, with 1000 being a unit. This contradicts the general FreeType practice of using FT_Fixed to represent fractional numbers. I would strongly suggest switching to FT_Fixed, with 0x1 representing a unit. Then all arithmetic will follow naturally with FT_MulFix, and FT_DivFix. Thank you, Alexei ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
In your integrated code you use FT_UInt for the rule scaling factor, with 1000 being a unit. This contradicts the general FreeType practice of using FT_Fixed to represent fractional numbers. I would strongly suggest switching to FT_Fixed, with 0x1 representing a unit. Then all arithmetic will follow naturally with FT_MulFix, and FT_DivFix. +1 Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
I had originally been using floating point to handle stretching and emboldening of glyphs, which for obvious reasons Werner didn't want. So, I used 1000 because it was easier to convert to and from percentage. I'm OK converting all this over to FT_Fixed, however I think it will make maintaining it more confusing and tedious. Essentially, it's just a question of representation, not changing the values themselves. Good comments help :-) Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
You can do this:
{ DejaVu Sans, 12, Regular Class, 'm', 0.95 * 0x1L },
I think the compiler will get it. See for example
src/cff/cffload.c:1362: priv-expansion_factor = (FT_Fixed)( 0.06
* 0x1L );
src/type1/t1load.c:2091:priv-expansion_factor = (FT_Fixed)( 0.06
* 0x1L );
src/type1/t1load.c:2092:priv-blue_scale = (FT_Fixed)(
0.039625 * 0x1L * 1000 );
That's a pretty clever trick, since it happens at compile time (I'm
guessing). Should make maintaining it not so bad. ;)
___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
but the latter is rather unlikely, so this needs more investigation and pattern searching in real fonts. If you provide me a script or whatever to do such a search, I can use my collection of old fonts as a testbed to search for hits. BTW, what do you think of replacing SPH_DEBUG with FT_TRACE7(( ... )) to get those information with FT2_DEBUG=ttinterp:7 ftview ... ? In case you are not acquainted with FreeType's tracing mechanism, have a look into docs/DEBUG. Thanks, I will look into this. It's always bothered me that there were special variables for debugging SPH stuff anyway. Don't hold up the release for this though! Erik ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
It seems OK, and it compiles fine.
Erik, please have a look at the attached patch. I've `beautified' the
bytecode opcodes, and I've discovered that most of them appear to be
incorrect, compared to the listings in
http://www.microsoft.com/typography/cleartype/truetypecleartype.aspx
Please check that. Have you actually done binary searches in TTFs to
find signatures? Another possibility is to disassemble the `fpgm'
table with TTX, then doing a textual search (`pcregrep' might help for
multiline searches).
Additionally, ttfautohint's bytecode signature has changed; the new
one is
PUSHB_1,
32,
ADD,
FLOOR,
in function 0.
Given that the new ttfautohint bytecode signature is that short, as
are some other snippets in Greg's whitepaper, I think it is best if
you extend the code so that the function number is also checked to
avoid false hits.
Werner
diff --git a/src/truetype/ttinterp.c b/src/truetype/ttinterp.c
index 505168d..440d385 100644
--- a/src/truetype/ttinterp.c
+++ b/src/truetype/ttinterp.c
@@ -4610,25 +4610,90 @@
FT_ULong n;
TT_DefRecord* rec;
TT_DefRecord* limit;
+
#ifdef TT_CONFIG_OPTION_SUBPIXEL_HINTING
FT_Byteopcode_pattern[7][12] = {
- /* inline delta function 1 */
- {0x4B,0x53,0x23,0x4B,0x51,0x5A,0x58,0x38,0x1B,0x21,0x21,0x59},
- /* inline delta function 2 */
- {0x4B,0x54,0x58,0x38,0x1B,0x5A,0x21,0x21,0x59,},
- /* diagonal stroke function */
- {0x20,0x20,0x40,0x60,0x47,0x40,0x23,0x42,},
- /* VacuFormRound function */
- {0x45,0x23,0x46,0x60,0x20,},
- /* ttfautohinted */
- {0x20,0x64,0xb0,0x60,0x66,0x23,0xb0,},
- /* spacing functions */
- {0x01,0x41,0x43,0x58,},
- {0x01,0x18,0x41,0x43,0x58,},
+ /* #0 inline delta function 1 */
+ {
+ 0x4B, /* PPEM*/
+ 0x53, /* GTEQ*/
+ 0x23, /* SWAP*/
+ 0x4B, /* PPEM*/
+ 0x51, /* LTEQ*/
+ 0x5A, /* AND */
+ 0x58, /* IF */
+ 0x38, /* SHPIX */
+ 0x1B, /* ELSE*/
+ 0x21, /* POP */
+ 0x21, /* POP */
+ 0x59 /* EIF */
+ },
+ /* #1 inline delta function 2 */
+ {
+ 0x4B, /* PPEM*/
+ 0x54, /* EQ */
+ 0x58, /* IF */
+ 0x38, /* SHPIX */
+ 0x1B, /* ELSE*/
+ 0x21, /* POP */
+ 0x21, /* POP */
+ 0x59 /* EIF */
+ },
+ /* #2 diagonal stroke function */
+ {
+ 0x20, /* DUP */
+ 0x20, /* DUP */
+ 0xB0, /* PUSHB_1 */
+ 0x01, /* 1 */
+ 0x60, /* ADD */
+ 0x46, /* GC_cur */
+ 0xB0, /* PUSHB_1 */
+ 0x40, /* 64*/
+ 0x23, /* SWAP*/
+ 0x42 /* WS */
+ },
+ /* #3 VacuFormRound function */
+ {
+ 0x45, /* RCVT*/
+ 0x23, /* SWAP*/
+ 0x46, /* GC_cur */
+ 0x60, /* ADD */
+ 0x20, /* DUP */
+ 0xB0, /* PUSHB_1 */
+ 0x26, /* 38*/
+ },
+ /* #4 TTFautohint bytecode (old) */
+ {
+ 0x20, /* DUP */
+ 0x64, /* ABS */
+ 0xB0, /* PUSHB_1 */
+ 0x20, /* 32*/
+ 0x60, /* ADD */
+ 0x66, /* FLOOR */
+ 0x23, /* SWAP*/
+ 0xB0 /* PUSHB_1 */
+ },
+ /* #5 spacing function 1 */
+ {
+ 0x01, /* SVTCA_x */
+ 0xB0, /* PUSHB_1 */
+ 0x18, /* 24*/
+ 0x43, /* RS */
+ 0x58 /* IF */
+ },
+ /* #6 spacing function 2 */
+ {
+ 0x01, /* SVTCA_x */
+ 0x18, /* RTG */
+ 0xB0, /* PUSHB_1 */
+ 0x18, /* 24*/
+ 0x43, /* RS */
+ 0x58 /* IF */
+ },
};
FT_UShort opcode_patterns = 7;
-FT_UShort opcode_pointer[7] = {0,0,0,0,0,0,0};
-FT_UShort opcode_size[7]= {12,9,8,5,7,4,5};
+FT_UShort opcode_pointer[7] = { 0, 0, 0, 0,
Re: [ft-devel] New Infinality Release
Have you actually done binary searches in TTFs to find signatures?
Well, only using the code here, not with a hex editor. I have seen
hits in some TTF files where I would not have expected them. May
speak to the issues you mentioned below.
OK. So it's definitely worth some tests :-)
[...] I think it is best if you extend the code so that the
function number is also checked to avoid false hits.
Yes, this makes sense for functions where we know the number, such
as ttfautohint and 0.
Well, most of the bytecode snippets are associated with one or more
fixed function numbers.
But, I think the whitepaper states that on (at least some)
functions, the number varies, which is why you need the detection
code in the first place.
Correct. My concern is that we get false hits due to the shortness of
the snippets.
Regarding the problems with the opcode patterns, what did you see
that was incorrect? From scanning them (quickly) it seemed like it
was mostly correct (perhaps missing the end IF, etc.)
Ah, I see that I've done a mistake, below is a corrected patch
(untested). However, I think that the current method is flawed in
general since arguments to opcodes are completely ignored by
`SKIP_Code'. For example, it is not possible to decide whether the
bytecode is
SVTCA_x
PUSHB_1
24
RS
IF
or
SVTCA_x
PUSHB_1
39
RS
IF
It seems to me that we really have to compare the *complete* bytecode
(including the arguments) within `Ins_FDEF'.
Werner
diff --git a/src/truetype/ttinterp.c b/src/truetype/ttinterp.c
index 505168d..ebd433c 100644
--- a/src/truetype/ttinterp.c
+++ b/src/truetype/ttinterp.c
@@ -4610,25 +4610,91 @@
FT_ULong n;
TT_DefRecord* rec;
TT_DefRecord* limit;
+
#ifdef TT_CONFIG_OPTION_SUBPIXEL_HINTING
+/* arguments to opcodes are skipped by `SKIP_Code' */
FT_Byteopcode_pattern[7][12] = {
- /* inline delta function 1 */
- {0x4B,0x53,0x23,0x4B,0x51,0x5A,0x58,0x38,0x1B,0x21,0x21,0x59},
- /* inline delta function 2 */
- {0x4B,0x54,0x58,0x38,0x1B,0x5A,0x21,0x21,0x59,},
- /* diagonal stroke function */
- {0x20,0x20,0x40,0x60,0x47,0x40,0x23,0x42,},
- /* VacuFormRound function */
- {0x45,0x23,0x46,0x60,0x20,},
- /* ttfautohinted */
- {0x20,0x64,0xb0,0x60,0x66,0x23,0xb0,},
- /* spacing functions */
- {0x01,0x41,0x43,0x58,},
- {0x01,0x18,0x41,0x43,0x58,},
+ /* #0 inline delta function 1 */
+ {
+ 0x4B, /* PPEM*/
+ 0x53, /* GTEQ*/
+ 0x23, /* SWAP*/
+ 0x4B, /* PPEM*/
+ 0x51, /* LTEQ*/
+ 0x5A, /* AND */
+ 0x58, /* IF */
+ 0x38, /* SHPIX */
+ 0x1B, /* ELSE*/
+ 0x21, /* POP */
+ 0x21, /* POP */
+ 0x59 /* EIF */
+ },
+ /* #1 inline delta function 2 */
+ {
+ 0x4B, /* PPEM*/
+ 0x54, /* EQ */
+ 0x58, /* IF */
+ 0x38, /* SHPIX */
+ 0x1B, /* ELSE*/
+ 0x21, /* POP */
+ 0x21, /* POP */
+ 0x59 /* EIF */
+ },
+ /* #2 diagonal stroke function */
+ {
+ 0x20, /* DUP */
+ 0x20, /* DUP */
+ 0xB0, /* PUSHB_1 */
+ /* 1 */
+ 0x60, /* ADD */
+ 0x46, /* GC_cur */
+ 0xB0, /* PUSHB_1 */
+ /* 64*/
+ 0x23, /* SWAP*/
+ 0x42 /* WS */
+ },
+ /* #3 VacuFormRound function */
+ {
+ 0x45, /* RCVT*/
+ 0x23, /* SWAP*/
+ 0x46, /* GC_cur */
+ 0x60, /* ADD */
+ 0x20, /* DUP */
+ 0xB0 /* PUSHB_1 */
+ /* 38*/
+ },
+ /* #4 TTFautohint bytecode (old) */
+ {
+ 0x20, /* DUP */
+ 0x64, /* ABS */
+ 0xB0, /* PUSHB_1 */
+ /* 32*/
+ 0x60, /* ADD */
+ 0x66, /* FLOOR */
+ 0x23, /* SWAP*/
+ 0xB0 /* PUSHB_1 */
+ },
+ /* #5 spacing function 1 */
+ {
+ 0x01, /* SVTCA_x */
+ 0xB0, /* PUSHB_1 */
+
Re: [ft-devel] New Infinality Release
Ah, I see that I've done a mistake, below is a corrected patch (untested). However, I think that the current method is flawed in general since arguments to opcodes are completely ignored by `SKIP_Code'. For example, it is not possible to decide whether the bytecode is SVTCA_x PUSHB_1 24 RS IF or SVTCA_x PUSHB_1 39 RS IF It seems to me that we really have to compare the *complete* bytecode (including the arguments) within `Ins_FDEF'. So, I've applied the updated patch you provided. The adjusted opcode sequences for the spacing functions broke Segoe UI Semibold (maybe others too), but I agree that what you did looks correct for those. This, along with the point you mentioned above about not being able to detect the pushed value, makes me think that this piece needs more work before it gets pushed out to a release. I do want to keep one or two of them in there however. I will trim down the code to patterns known to be OK, and push off work on the others until after the release. Does this make sense to you? ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
So, I've applied the updated patch you provided. OK. The adjusted opcode sequences for the spacing functions broke Segoe UI Semibold (maybe others too), but I agree that what you did looks correct for those. I'm not completely sure whether the bytecode sequences are correct. For example, Greg sometimes writes #PUSH[], 1 instead of the expected #PUSH 1 This might indicate that the command is actually using NPUSHB 0x01 0x01 instead of PUSHB_1 0x01, or even NPUSHW 0x01 0x00 0x01, but the latter is rather unlikely, so this needs more investigation and pattern searching in real fonts. If you provide me a script or whatever to do such a search, I can use my collection of old fonts as a testbed to search for hits. BTW, what do you think of replacing SPH_DEBUG with FT_TRACE7(( ... )) to get those information with FT2_DEBUG=ttinterp:7 ftview ... ? In case you are not acquainted with FreeType's tracing mechanism, have a look into docs/DEBUG. This, along with the point you mentioned above about not being able to detect the pushed value, makes me think that this piece needs more work before it gets pushed out to a release. I do want to keep one or two of them in there however. I will trim down the code to patterns known to be OK, and push off work on the others until after the release. Thanks! Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
[I'm sending this to the ft-devel mailing list since `infinality.net' seems to have e-mail problems. If you are not Erik or Alexei, you might ignore it :-)] Erik, Alexei! First of all, thanks for working on the code :-) Interesting... I thought there was a problem because I didn't see anything updating on the Savannah end. Looks like while I was typing my last email, things showed up and appear to be OK. Werner, can you verify that everything is OK? Hope I didn't mess anything up. :) It seems OK, and it compiles fine. To avoid such problems in the future I ask you to always do git pull before saying git commit . And in case there is a long delay between committing and pushing, you should call git pull again right before git push so that you can resolve conflicts easily. This still doesn't completely prevent concurrent commits, but it reduces the time frame within this can happen to a few minutes. Hey! You're breaking my commit! :) Just about to fix that stuff. Can you put it back? No, he can't :-) Note that due to having a ChangeLog file, git's `rebase' functionality will fail usually. Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
If I am unable to connect in the next hour or two, I can send you the patch. The patch is now in the repository. Thanks a lot! Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
On Mon, Jun 18, 2012 at 10:38 AM, Werner LEMBERG w...@gnu.org wrote: If I am unable to connect in the next hour or two, I can send you the patch. The patch is now in the repository. Thanks a lot! Great news! Hope to see rest of the infinality patchset in later on :) Regards, ismail ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
Hope to see rest of the infinality patchset in later on :) This will take a longer time, since we first have to implement a proper infrastructure to control the auto-hinter. Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
On Mon, Jun 18, 2012 at 4:38 AM, Werner LEMBERG w...@gnu.org wrote: The patch is now in the repository. Thanks a lot! I am not sure why Round_To_Grid and its family are modified but not used. I am actually more concerned with the way they are modified. Things like this look dangerous: - val = ~63; + val = ~( 64 / resolution - 1 ); Resolution has to be a power of 2 for this to work. There was so much thought in the original code to avoid division only to add it right back in. To be honest I wish (64/resolution) was the function argument, i.e. do this at a much higher level. Can we remove these modifications for now or clean them up? Thank you. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
I really dislike how you disabled hinting in the x-direction. You essentially replaced '64' in the rounding functions with '64 / 64', aka '1'. This is brutal and takes a performance toll on this part of the code even when your subpixel hinting is not used. I strongly suggest that you just if-condition the rounding calls in the x-direction. This is only a couple of places where you use the 'gridlines_per_pixel' variable where the if-conditions should land. Finally, please do help stupid compilers to handle your B1 % 64 and B2 % 64. Make it B1 63 and B2 63. It's much faster. ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
I really dislike how you disabled hinting in the x-direction. [...] Thanks for your reviews, and please continue. I'm sure Erik will collect them all :-) Note that I won't do any changes right now; instead, I hope that Erik's SSH problems are gone soon so that he can maintain the code by himself. Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
Yep, I welcome reviews and suggestions for improvement. Be gentle; I never claimed to be a C coder and I'm still learning best-practice for these things. On 06/18/2012 08:40 AM, Werner LEMBERG wrote: I really dislike how you disabled hinting in the x-direction. [...] Thanks for your reviews, and please continue. I'm sure Erik will collect them all :-) Note that I won't do any changes right now; instead, I hope that Erik's SSH problems are gone soon so that he can maintain the code by himself. Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
Ok, I have access now. I had a newline in my public key. Doh! On 06/18/2012 08:40 AM, Werner LEMBERG wrote: I really dislike how you disabled hinting in the x-direction. [...] Thanks for your reviews, and please continue. I'm sure Erik will collect them all :-) Note that I won't do any changes right now; instead, I hope that Erik's SSH problems are gone soon so that he can maintain the code by himself. Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
On Mon, Jun 18, 2012 at 9:52 AM, Infinality infinal...@infinality.net wrote: On 06/18/2012 08:40 AM, Werner LEMBERG wrote: I really dislike how you disabled hinting in the x-direction. [...] Thanks for your reviews, and please continue. I'm sure Erik will collect them all :-) Note that I won't do any changes right now; instead, I hope that Erik's SSH problems are gone soon so that he can maintain the code by himself. Yep, I welcome reviews and suggestions for improvement. Be gentle; I never claimed to be a C coder and I'm still learning best-practice for these things. I think the patch is actually pretty clean and well separated from the rest of the code with #ifdef's. The patch only intrudes into the rounding routines to effectively disable them in x-direction. So I suggest that you use if (y-direction) OUTSIDE the rounding functions effectively skipping the functions. Do you see my point? ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
On 06/18/2012 03:50 PM, Alexei Podtelezhnikov wrote: On Mon, Jun 18, 2012 at 9:52 AM, Infinality infinal...@infinality.net wrote: On 06/18/2012 08:40 AM, Werner LEMBERG wrote: I really dislike how you disabled hinting in the x-direction. [...] Thanks for your reviews, and please continue. I'm sure Erik will collect them all :-) Note that I won't do any changes right now; instead, I hope that Erik's SSH problems are gone soon so that he can maintain the code by himself. Yep, I welcome reviews and suggestions for improvement. Be gentle; I never claimed to be a C coder and I'm still learning best-practice for these things. I think the patch is actually pretty clean and well separated from the rest of the code with #ifdef's. The patch only intrudes into the rounding routines to effectively disable them in x-direction. So I suggest that you use if (y-direction) OUTSIDE the rounding functions effectively skipping the functions. Do you see my point? Yes, and in fact I had tried going through the rounding functions recently to do simplification / cleanup. Like you said, I'm really only interested in ignoring x under most circumstances. I had originally left the possibility open to have a variable number of vertical gridlines per pixel, but after some time, I don't see why we would ever need to go to anything less than 64 per pixel, which is the max. I tinkered around with it in an attempt to do essentially what you describe here, but it ended up negatively impacting the rendering, probably due to some error on my part. I'll see if I can pick this up again, because I'd like to touch as little existing freetype code as possible with these enhancements. Thanks, Erik ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
Next patch will have all the issues you brought up resolved. I turned the floats into FT_Ints which use 1000 to represent 1.0 and do a FT_Muldiv instead. Rasterizer version is now 38 by default, and sets bit pair 10/17 accordingly. Wide characters are replaced with their numeric values and comments. Code formatting should now be fixed across all patches (even the non-subpixel ones, which admittedly were not formatted well). Now, if I can just figure out what the deal is with the git access to savannah On 06/16/2012 01:44 AM, Werner LEMBERG wrote: For those interested, the latest release is available here: http://www.infinality.net/blog/infinality-freetype-patches/ Very nice! Some remarks regarding freetype-add-subpixel-hinting-infinality-20120615-01.patch: ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
Next patch will have all the issues you brought up resolved. Great! Thanks a lot. There is a last problem, namely a proper ChangeLog entry. However, if you have a final code, I can prepare this for you before you are getting desperate :-) Now, if I can just figure out what the deal is with the git access to savannah Have you done some google search regarding ssh problems on Savannah? Unfortunately, you have to solve this by yourself... Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
Yep, I have done some google searching as well about ssh and savannah, but nothing helped. I'm going through the setup document again to see if I missed something. If I am unable to connect in the next hour or two, I can send you the patch. Thanks, Erik On 06/17/2012 11:39 AM, Werner LEMBERG wrote: Next patch will have all the issues you brought up resolved. Great! Thanks a lot. There is a last problem, namely a proper ChangeLog entry. However, if you have a final code, I can prepare this for you before you are getting desperate :-) Now, if I can just figure out what the deal is with the git access to savannah Have you done some google search regarding ssh problems on Savannah? Unfortunately, you have to solve this by yourself... Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
For those interested, the latest release is available here:
http://www.infinality.net/blog/infinality-freetype-patches/
Very nice!
Some remarks regarding
freetype-add-subpixel-hinting-infinality-20120615-01.patch:
. In line 1440 I see this:
+#endif /* TT_CONFIG_OPTION_SUBPIXEL_HINTING */
+#ifdef TT_CONFIG_OPTION_SUBPIXEL_HINTING
Just drop these too lines :-)
. To get the distinction between GDI and DW ClearType (the former,
older one doesn't support AA rendering along the y axis), Greg
Hitchcock explained on the OpenType mailing list:
Most likely, though, is the DirectWrite support for sub-pixel
positioned ClearType. This can be queried with GETINFO selector
bit 10, return bit 17. (In order to test this flag, the
TrueType version must be = 38 and = 64.) GDI does not support
sub-pixel positioned ClearType.
Bits 10/17, along with two other pairs, 11/18 and 12/19, lack
complete documentation in the OpenType standard currently; Greg is
going to fix that in the ClearType whitepaper, he told me.
Shall we set bit pair 10/17 and use value 38 (not 37) for the
rasterizer version?
. I suggest to replace
if ( distance FT_MulDiv( minimum_distance_factor,
CUR.GS.minimum_distance, 64 ) )
distance = FT_MulDiv( minimum_distance_factor,
CUR.GS.minimum_distance, 64 );
with
new_distance = FT_MulDiv( minimum_distance_factor,
CUR.GS.minimum_distance, 64 );
if ( distance new_distance )
distance = new_distance;
to help dumb compilers. I think it also makes the code more
readable. This is around lines 1300, 1310, 1404, and 1414 in the
patch.
. I'm not happy that you are using `float'. Right now floating
mathematics is not used in FreeType, and ideally I would like to
stay with that (until someone is going to replace MulDiv
completely). Would it be a great burden to use FT_MulDiv and
friends instead?
. You use wide characters for Cyrillic, e.g.
L'и'
This is a bad idea. First of all, MS compilers require them to be
encoded in UTF-16 (which is only 16bit wide). Second, as
currently written in the source code, those values are encoded in
a different encoding, namely UTF-8. This doesn't fit. Third, for
higher Unicode values, UTF-8 gives three or more bytes which
doesn't fit either. Fourth, the correct type for L'...' would be
wchar_t, but this is not portable. Unicode 4.0 says:
The width of wchar_t is compiler-specific and can be as small
as 8 bits. Consequently, programs that need to be portable
across any C or C++ compiler should not use wchar_t for storing
Unicode text. The wchar_t type is intended for storing
compiler-defined wide characters, which may be Unicode
characters in some compilers.
Please replace them with numeric values (and a proper comment).
You did a lot of search and replace. If possible I ask you to fix
some formatting stuff:
. After the variable declarations within a block, you sometimes use
a single line before the code starts. I ask you to use two empty
lines:
FT_UInt foo;
foo = bar;
And please avoid excessive, unnecessary whitespace before variable
names:
FT_UInt foo;
Two spaces (if not aligning vertically) are fully sufficient for
the weird FreeType formatting :-)
. Please don't write
if ( foo != 0
!( bar baz ) )
but rather
if ( foo != 0
!( bar baz ) )
to align the left character of `' or `||' with the correct level
of the closing parenthesis. If necessary, move the closing brace
to the right to get a nice vertical alignment.
. Please say
foo= x;
foobar = x;
instead of
foo = x;
foobar = x;
. Please always say
if (foo)
x = bar;
else
x = foobar;
instead of
if (foo) x = bar;
else x = foobar;
even for very short IF and IF-ELSE clauses. I like vertical
formatting a lot, and it improves legibility IMHO.
. Please don't use braces for single-line blocks, thus
if (foo)
{
foo = bar;
}
should rather be
if (foo)
foo = bar;
. We have a space before a parenthesis only after a C
keyword, but not after function or macro declarations:
foo ( bar )= foo( bar )
sizeof( foo ) = sizeof ( foo )
Werner
___
Freetype-devel mailing list
Freetype-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
it works! :) much thanks -v On 16 Jun 2012, at 05:00, Infinality wrote: * Fix Oxygen rendering if usint TT hinting, and other ttfautohinted fonts ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel
Re: [ft-devel] New Infinality Release
I'll certainly look into these things before committing the patches. I
know you don't like floats, so I'll try to work around that. Regarding
the formatting, some of those were oversights on my part. The thing
with the way off at the end of the line was something I saw a
precedent for somewhere else in existing Freetype code, so I thought it
was the standard. I too thought it looked odd, but figured you wanted
it that way. ;)
On 06/16/2012 01:44 AM, Werner LEMBERG wrote:
For those interested, the latest release is available here:
http://www.infinality.net/blog/infinality-freetype-patches/
Very nice!
Some remarks regarding
freetype-add-subpixel-hinting-infinality-20120615-01.patch:
. In line 1440 I see this:
+#endif /* TT_CONFIG_OPTION_SUBPIXEL_HINTING */
+#ifdef TT_CONFIG_OPTION_SUBPIXEL_HINTING
Just drop these too lines :-)
. To get the distinction between GDI and DW ClearType (the former,
older one doesn't support AA rendering along the y axis), Greg
Hitchcock explained on the OpenType mailing list:
Most likely, though, is the DirectWrite support for sub-pixel
positioned ClearType. This can be queried with GETINFO selector
bit 10, return bit 17. (In order to test this flag, the
TrueType version must be = 38 and = 64.) GDI does not support
sub-pixel positioned ClearType.
Bits 10/17, along with two other pairs, 11/18 and 12/19, lack
complete documentation in the OpenType standard currently; Greg is
going to fix that in the ClearType whitepaper, he told me.
Shall we set bit pair 10/17 and use value 38 (not 37) for the
rasterizer version?
. I suggest to replace
if ( distance FT_MulDiv( minimum_distance_factor,
CUR.GS.minimum_distance, 64 ) )
distance = FT_MulDiv( minimum_distance_factor,
CUR.GS.minimum_distance, 64 );
with
new_distance = FT_MulDiv( minimum_distance_factor,
CUR.GS.minimum_distance, 64 );
if ( distance new_distance )
distance = new_distance;
to help dumb compilers. I think it also makes the code more
readable. This is around lines 1300, 1310, 1404, and 1414 in the
patch.
. I'm not happy that you are using `float'. Right now floating
mathematics is not used in FreeType, and ideally I would like to
stay with that (until someone is going to replace MulDiv
completely). Would it be a great burden to use FT_MulDiv and
friends instead?
. You use wide characters for Cyrillic, e.g.
L'и'
This is a bad idea. First of all, MS compilers require them to be
encoded in UTF-16 (which is only 16bit wide). Second, as
currently written in the source code, those values are encoded in
a different encoding, namely UTF-8. This doesn't fit. Third, for
higher Unicode values, UTF-8 gives three or more bytes which
doesn't fit either. Fourth, the correct type for L'...' would be
wchar_t, but this is not portable. Unicode 4.0 says:
The width of wchar_t is compiler-specific and can be as small
as 8 bits. Consequently, programs that need to be portable
across any C or C++ compiler should not use wchar_t for storing
Unicode text. The wchar_t type is intended for storing
compiler-defined wide characters, which may be Unicode
characters in some compilers.
Please replace them with numeric values (and a proper comment).
You did a lot of search and replace. If possible I ask you to fix
some formatting stuff:
. After the variable declarations within a block, you sometimes use
a single line before the code starts. I ask you to use two empty
lines:
FT_UInt foo;
foo = bar;
And please avoid excessive, unnecessary whitespace before variable
names:
FT_UInt foo;
Two spaces (if not aligning vertically) are fully sufficient for
the weird FreeType formatting :-)
. Please don't write
if ( foo != 0
!( bar baz ) )
but rather
if ( foo != 0
!( bar baz ) )
to align the left character of `' or `||' with the correct level
of the closing parenthesis. If necessary, move the closing brace
to the right to get a nice vertical alignment.
. Please say
foo= x;
foobar = x;
instead of
foo = x;
foobar = x;
. Please always say
if (foo)
x = bar;
else
x = foobar;
instead of
if (foo) x = bar;
else x = foobar;
even for very short IF and IF-ELSE clauses. I like vertical
formatting a lot, and it improves legibility IMHO.
. Please don't use braces for single-line blocks, thus
if (foo)
{
foo = bar;
}
should
