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

Modify TinySDF to return only alpha channel #3

Merged
merged 6 commits into from
Jun 28, 2017

Conversation

ChrisLoer
Copy link
Contributor

It's convenient for the demo to use an RGBA ImageData for building the texture, but in general I think we'd expect users of TinySDF to work with an alpha-channel-only result since that's the only actual information in the SDF we generate.

I updated the demo to transform the alpha channel into an ImageData, and didn't see any noticeable change in performance.

@ChrisLoer ChrisLoer requested a review from mourner June 27, 2017 17:43
@ChrisLoer
Copy link
Contributor Author

@mourner I tacked on an additional 'font-weight' parameter (motivated by Mapbox GL JS use case of using a single font family for Chinese characters but changing weights based on the style).

index.js Outdated
this.radius = radius || 8;
var size = this.size = this.fontSize + this.buffer * 2;

this.canvas = document.createElement('canvas');
this.canvas.width = this.canvas.height = size;

this.ctx = this.canvas.getContext('2d');
this.ctx.font = fontSize + 'px ' + this.fontFamily;
/* style | variant | weight | size/line-height | family */
this.ctx.font = `normal normal ${this.fontWeight} ${fontSize}px/normal ${this.fontFamily}`;
Copy link
Member

Choose a reason for hiding this comment

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

I don't yet use ES6 in this module so let's keep the concatenation. I think the shorthand form can be simplified too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched to the format 'font-weight font-size font-family'. I read the "Shorthand font property: the 'font' property" section of the CSS spec, and it seems to me to be a little ambiguous about how it distinguishes 'font-weight'/'font-variant'/'font-style'. But I guess it can disambiguate based on the value (except for 'normal' vs 'inherited').

index.js Outdated
@@ -37,10 +39,10 @@ TinySDF.prototype.draw = function (char) {
this.ctx.fillText(char, this.buffer, this.middle);

var imgData = this.ctx.getImageData(0, 0, this.size, this.size);
var data = imgData.data;
var alphaChannel = new Uint8Array(this.size * this.size);
Copy link
Member

Choose a reason for hiding this comment

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

Should we use Uint8ClampedArray here instead of Uint8Array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@ChrisLoer ChrisLoer force-pushed the cloer_return_alpha_channel branch from f73e27f to 52871d7 Compare June 27, 2017 20:09
@mourner mourner merged commit 97a071f into master Jun 28, 2017
@mourner mourner deleted the cloer_return_alpha_channel branch June 28, 2017 20:24
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.

2 participants