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

Improve API usage of BitmapFont.from #6640

Merged
merged 8 commits into from
May 24, 2020
Merged

Conversation

bigtimebuddy
Copy link
Member

@bigtimebuddy bigtimebuddy commented May 22, 2020

Overview

This change is mostly about improving and streamlining ergonomics using the new BitmapFont.from API (#6600).

Also, did some minor code organization and documentation fixes.

Before
Previously you optionally defined the font name and no chars were set by default.

const font = BitmapFont.from({ chars: BitmapFont.ALPHANUMERIC }, {
    fill: "#808080",
    fontFamily: "Roboto-Black",
    fontSize: 33,
});
BitmapFont.available.TitleFont = font;

After
Now, font is installed, name is required, and chars are optional (defaults to ALPHANUMERIC).

BitmapFont.from("TitleFont", {
    fill: "#808080",
    fontFamily: "Roboto-Black",
    fontSize: 33,
});

Now, if you want to change something on the fly, regenerating the font is as easy as calling BitmapFont.from again with some new style options.

const style = new TextStyle({
    fill: "#808080",
    fontFamily: "Roboto-Black",
    fontSize: 33,
});
BitmapFont.from("TitleFont", style);
style.fontSize = 14;
BitmapFont.from("TitleFont", style); // rerenders font!

Demo

https://jsfiddle.net/bigtimebuddy/wt9szqp6/

Changed

  • Both these signatures are now supported BitmapFont.from(name, style) and BitmapFont.from(options, style) Changed API signature to be BitmapFont.from(name, style, options)
  • Destroying a BitmapFont now also destroys the page textures, not just the glyph textures
  • Field name is required for BitmapFont.from for ease of use.
  • When calling BitmapFont.from, it will override any existing font by default with the same name, making it easier to regenerate the font.
  • Exposed default options via BitmapFont.defaultOptions in case users want to change this globally.
  • Moved utilities drawGlyph and processCharData to their own utilities. Still need a cleanup on generateFillStyle and drawGlyph with Text. Probably better for a follow-up PR.

@codecov-commenter
Copy link

codecov-commenter commented May 22, 2020

Codecov Report

Merging #6640 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #6640   +/-   ##
=======================================
  Coverage   82.34%   82.34%           
=======================================
  Files          37       37           
  Lines        1897     1897           
=======================================
  Hits         1562     1562           
  Misses        335      335           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef8f397...653b392. Read the comment docs.

Copy link
Member

@ShukantPal ShukantPal left a comment

Choose a reason for hiding this comment

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

Looks much better than before!

packages/text-bitmap/test/BitmapFont.js Show resolved Hide resolved
packages/text-bitmap/src/BitmapFont.ts Outdated Show resolved Hide resolved
Copy link
Member

@ShukantPal ShukantPal left a comment

Choose a reason for hiding this comment

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

🥇 🥇

@bigtimebuddy
Copy link
Member Author

One other thing I discovered was the use of Array.prototype.find which is not supported in IE will need to replace that with something more compatible.

Copy link
Member

@GoodBoyDigital GoodBoyDigital left a comment

Choose a reason for hiding this comment

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

nice tidy up @bigtimebuddy ! 👍

import { hex2rgb, string2hex } from '@pixi/utils';
import type { TextMetrics, TextStyle } from '@pixi/text';

// TODO: Prevent code duplication b/w drawGlyph & Text#updateText
Copy link
Member

Choose a reason for hiding this comment

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

oh yes! this is great idea!
we could create a cache that is mapped by style :D
love the idea of storing standard fonts as B/W and then tinting them! 👍

Copy link
Member

@themoonrat themoonrat left a comment

Choose a reason for hiding this comment

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

"
Both these signatures are now supported BitmapFont.from(name, style) and BitmapFont.from(options, style)
"

Like the changes! But this signature style doesn't make sense to me. Surely you just have

BitmapFont.from(name, style, options) 

And then you've one signature to rule them all?

@bigtimebuddy
Copy link
Member Author

bigtimebuddy commented May 23, 2020

Good shout @themoonrat. Fixed.

@bigtimebuddy bigtimebuddy added the ✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t label May 23, 2020
@bigtimebuddy bigtimebuddy merged commit c097ffa into dev May 24, 2020
@bigtimebuddy bigtimebuddy deleted the dev-bitmap-font-cleanup branch May 24, 2020 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants