code review 6354080: freetype/truetype: vector set/gets, store ops, roll/max...

freetype/truetype: vector set/gets, store ops, roll/max/min ops.

[nigeltao, 2012-07-06 11:25:14]

Hello bsiegert@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/freetype-go

[bsiegert, 2012-07-06 14:08:13]

http://codereview.appspot.com/6354080/diff/1005/freetype/truetype/hint.go
File freetype/truetype/hint.go (right):

http://codereview.appspot.com/6354080/diff/1005/freetype/truetype/hint.go#new...
freetype/truetype/hint.go:17: store [128]int32
Should the size of this depend on the number of storage locations given in the
font file header?

http://codereview.appspot.com/6354080/diff/1005/freetype/truetype/hint.go#new...
freetype/truetype/hint.go:32: // The default vectors are along the X axis.
There is more and more initialization of state here. Should this be done in a
NewHinter function instead? Or is the purpose to start each run with a clean
stack?

http://codereview.appspot.com/6354080/diff/1005/freetype/truetype/hint.go#new...
freetype/truetype/hint.go:359: return errors.New("truetype: hinting: unsupported
IDEF instruction")
Is this really an error, or should this just emit a warning and skip until the
corresponding ENDF?

[nigeltao, 2012-07-08 00:38:10]

http://codereview.appspot.com/6354080/diff/1005/freetype/truetype/hint.go
File freetype/truetype/hint.go (right):

http://codereview.appspot.com/6354080/diff/1005/freetype/truetype/hint.go#new...
freetype/truetype/hint.go:17: store [128]int32
On 2012/07/06 14:08:13, bsiegert wrote:
> Should the size of this depend on the number of storage locations given in the
> font file header?

Maybe. The easiest thing to do for now is a fixed size storage area. I'll add a
TODO.

http://codereview.appspot.com/6354080/diff/1005/freetype/truetype/hint.go#new...
freetype/truetype/hint.go:32: // The default vectors are along the X axis.
On 2012/07/06 14:08:13, bsiegert wrote:
> There is more and more initialization of state here. Should this be done in a
> NewHinter function instead? Or is the purpose to start each run with a clean
> stack?

Each run has to start with a clean slate. This initialization will probably
separate out when I implement function calls, but that's for a future CL.

http://codereview.appspot.com/6354080/diff/1005/freetype/truetype/hint.go#new...
freetype/truetype/hint.go:359: return errors.New("truetype: hinting: unsupported
IDEF instruction")
On 2012/07/06 14:08:13, bsiegert wrote:
> Is this really an error, or should this just emit a warning and skip until the
> corresponding ENDF?

I prefer an error over a warning.

[bsiegert, 2012-07-08 08:49:06]

LGTM

[nigeltao, 2012-07-09 13:19:46]

*** Submitted as
http://code.google.com/p/freetype-go/source/detail?r=33f1aeb76b1a ***

freetype/truetype: vector set/gets, store ops, roll/max/min ops.

R=bsiegert
CC=golang-dev
http://codereview.appspot.com/6354080