Skip to content

Conversation

devongovett
Copy link
Contributor

This adds support for correctly breaking lines of unicode text in languages where there are no spaces between the words, such as CJK languages. It uses the linebreak module, which is an implementation of the Unicode line breaking algorithm. The module is also used by my PDFKit project.

Also adds support for force breaks (e.g. newlines) in the text as well, as a side benefit. 😄

Tested by adding some Chinese text to the timeline demo:

screen shot 2015-02-10 at 4 02 12 pm

@mjohnston
Copy link

This is a much needed addition! I'm curious - have you profiled measureText to see if there is a perf improvement or regression? I know there was much room for improvement there.

My biggest concern is with the added build dependency required to enable the unicode tries import here: https://github.com/devongovett/linebreak/blob/master/src/linebreaker.coffee#L3

@devongovett
Copy link
Contributor Author

i.e. the brfs addition to the webpack config? I'm not too familiar with webpack, but with browserify you can specify transforms needed by a module in the package.json (see here). Webpack didn't seem to support that when I first tried building, so I had to add it to the webpack config here. Is there a better way to do that? As a last resort, I suppose I could publish a prebuilt version to npm but I'd prefer not to as you lose debugging info that way.

Support Unicode 7.0.0 and make builds much smaller
@devongovett
Copy link
Contributor Author

Just updated the linebreak module to the latest version which adds Unicode 7.0 support, and makes builds much smaller (48KB -> 15KB minified).

Also did some benchmarking of the measureText function, and while the new version supporting unicode is slower, it is not by that much. This was expected since the unicode linebreaking algorithm is non-trivial compared to the simple word splitting function used before. Here's the output of the benchmarks I ran, using one of the test strings in the example app:

old x 1,507 ops/sec ±4.86% (62 runs sampled)
new x 1,339 ops/sec ±5.63% (63 runs sampled)
old is the fastest
new is 11% slower

I think other improvements to the actual line layout algorithm are probably possible, but as for line breaking itself, I'm not sure it can get much faster while remaining correct according to the unicode standard. I did a lot of work on this for PDFKit to try to get it to be as fast as possible, including by implementing a custom data structure to represent the unicode properties as efficiently in terms of file size and lookup performance as possible.

As for the build issue, see my previous comment and let me know what you'd prefer.

@devongovett
Copy link
Contributor Author

I should also note that the benchmarks above were without the cache enabled. 😄

@qdsang
Copy link

qdsang commented Mar 13, 2015

Look forward to be able to merge

@mjohnston
Copy link

Sorry - totally dropped the ball on this one. I'd like to get this merged, but am having trouble running the PR branch.

@devongovett are you missing a brfs dependency in the package.json?

I'd like to be able to remove the dependency on webpack and/or browserify to build react-canvas but that doesn't have to happen in this PR.

@mjohnston
Copy link

Just to be clear, it's not just the brfs loader that I'm talking about re: webpack/browserify dependency. It's also that ImageCache relies on webpack's events mock for node.

@devongovett
Copy link
Contributor Author

Sorry for the delay. Just merged in master and added the brfs dependency. Apparently I had brfs installed in the parent directory to react-canvas so it was working for me locally. 😉

@mjohnston
Copy link

Thanks @devongovett for updating. Landing in master now.

mjohnston pushed a commit that referenced this pull request Apr 5, 2015
@mjohnston mjohnston merged commit 4b31658 into Flipboard:master Apr 5, 2015
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.

3 participants