Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Glyphs #386

Merged
merged 91 commits into from
May 29, 2014
Merged

Glyphs #386

merged 91 commits into from
May 29, 2014

Conversation

yhahn
Copy link
Member

@yhahn yhahn commented May 28, 2014

Not ready but opening PR for others to start to play with/review. Should be able to pull this down and get a working debug demo.

Will let @mikemorris write his notes here tomorrow.

cc @kkaefer

mikemorris and others added 30 commits February 18, 2014 16:11
Conflicts:
	js/text/placement.js
Conflicts:
	js/ui/source.js
Conflicts:
	debug/site.js
Conflicts:
	js/text/placement.js
Conflicts:
	debug/site.js
Does not yet render the right sdfs.
@yhahn
Copy link
Member Author

yhahn commented May 28, 2014

@mikemorris I pushed a second fontstack (Open Sans Semibold, Arial Unicode MS Regular) to the S3 bucket, same location. It appears to "just work":

webgl

Let's bulletproof your work today and make sure there are no regressions/differences in label density, etc. so that your branch can be merged.

@yhahn
Copy link
Member Author

yhahn commented May 28, 2014

  • sdf.js can this be removed now?
  • workers is set to 1 atm, prob from debugging -- can be bumped back up? Tested with multiple workers?
  • more testing + review from @kkaefer @ansis @mourner

@mikemorris anything else you're seeing before you feel this is ready?

@mikemorris
Copy link
Contributor

sdf.js can this be removed now?

Yes.

workers is set to 1 atm, prob from debugging -- can be bumped back up? Tested with multiple workers?

Yup.

65280, // Specials
65520, // Halfwidth and Fullwidth Forms
65534 // Specials
];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest separating all the glyphs stuff into a separate file, since workertile.js gets too big.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mourner Agreed, at the very least this big array should probably live someplace else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, or the whole parseTextBucket method. It would also be nice to have a couple more comments along the code below.

@yhahn
Copy link
Member Author

yhahn commented May 28, 2014

Let's also resolve #388 before considering a merge.

@ansis
Copy link
Contributor

ansis commented May 28, 2014

Added basic text wrapping that only breaks on regular spaces (charcode 32). Controlled with:

{
    "text-max-width": 2, // em, default 15
    "text-line-height": 1.4, // em, default 1.2
    "text-alignment": "right", // default center
}

ansis and others added 4 commits May 28, 2014 17:57
"text-slant": 0.1  // shift one pixel to the right for every pixel up
"text-slant": 0    // no slant
256 char range glyphs. Refs #388.
@yhahn yhahn changed the title [notready] Glyphs Glyphs May 29, 2014
ansis added a commit that referenced this pull request May 29, 2014
@ansis ansis merged commit 567fda9 into master May 29, 2014
@ansis ansis deleted the glyphsproto branch May 29, 2014 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants