graphicore / ufoJS

Javascript API for the Unified Font Object
lib.ufojs.org
GNU General Public License v3.0
52 stars 10 forks source link

This is a port to javascript of the folowing robofab python modules #56

Closed felipesanches closed 9 years ago

felipesanches commented 9 years ago

(just the portions that are really needed for now):

These modules are all used to calculate the bounding box of glyphs during OTF export in Metapolator.

graphicore commented 9 years ago

Thanks! This looks good at a first glance. I will look deeper into it tomorrow.

I have some questions and requests:

  1. please write in a comment at the top of the file what the source-file of the port is (github url + revision hash)
  2. "just the portions that are really needed for now"
    • What exactly is missing?
    • Is it a lot?
    • Is it useful?
    • Why not port it now?
  3. Please use 4 spaces as indentation, I spotted some tabs.
felipesanches commented 9 years ago

OK. The tabs are probably reminescent of the copy&pasted python code. I'll fix that.

I will be glad to completely port all the other methods.

davelab6 commented 9 years ago

Bit of a side question, but, wouldn't porting defcon make more sense than robofab?

graphicore commented 9 years ago

We are not porting all of robofab. These are just utilities like pens and some math functions (look at the names of it).

graphicore commented 9 years ago

I will be glad to completely port all the other methods.

I'm not sure. Let's rather decide which ones we are going to use from these libs. For something like vectorLength of arrayTools.js I'd prefer to have more complete set of complex-number-math operations around. In metapolator we use math/Vector which in turn uses https://github.com/graphicore/Complex and maybe we can have a better structured solution one day. So I wouldn't bother right now to port this.

graphicore commented 9 years ago

I will be glad to completely port all the other methods.

Also, I think in the long run it's much better to use http://pomax.github.io/bezierjs/

graphicore commented 9 years ago

please write in a comment at the top of the file what the source-file of the port is (github url + revision hash)

I checked you ports with several! versions of the source files. For bezierTools and arrayTools I'd prefer as a source: https://github.com/behdad/fonttools/blob/master/Lib/fontTools/misc/arrayTools.py and https://github.com/behdad/fonttools/blob/master/Lib/fontTools/misc/bezierTools.py as the projects are more actively maintained.

The pens should be from robofab (I don't understand how or when that temporary fork is supposed to end, but it is mentioned in the docstring): https://github.com/robofab-developers/robofab/blob/ufo3k/Lib/robofab/pens/boundsPen.py but with a hint to also check https://github.com/robofab-developers/robofab/blob/master/Lib/robofab/pens/boundsPen.py (as the first is the primary source of ufoJS but the second is more actively maintained)

graphicore commented 9 years ago

Ok, I think we don't have to port all the missing stuff. But a short comment at the top of the files, near to the source statement about what functions are missing from the port would be good.

Pomax commented 9 years ago

once you do consider bezierjs, I'll be happy to work in as many improvements as possible for fast (and efficient) computation as make sense for font purposes.

davelab6 commented 9 years ago

:dancer:

The .outline() method is great, would love to have that :)

graphicore commented 9 years ago

@Pomax awesome!

felipesanches commented 9 years ago

@Pomax Thanks! Your help offer is greatly appreciated :-) We have indeed some performance issues in Metapolator's current implementation (which uses ufoJS modules). We better deal with these improvements after we settle the OTF & UFO Export features there. Then, we'll be happy to have you help on profiling and performance improvements if you have those skills to offer.

graphicore commented 9 years ago

The performance issues we have are far away from bezier math.

graphicore commented 9 years ago

But we can use the outline stuff to make an alternative form of centerline-expansion :-)

felipesanches commented 9 years ago

@graphicore yeah, that's what I thought.

felipesanches commented 9 years ago

@graphicore Do you see any pending issue here, or can we already merge this?

graphicore commented 9 years ago

Do you see any pending issue here

both pens have still tabs

A list of missing functions that are exported by the source modules but not by us would be really nice to have. At least we should acknowledge in the header comment that the port is not complete.

Anything else looks pretty good.

felipesanches commented 9 years ago

oh, yeah, sorry! That one slipped. I'll add these comments there.

graphicore commented 9 years ago

no problem