search

pbnjay / pixfont

A simple, lightweight Pixel Font package for Go that works with the standard image/draw package.
MIT License
105 stars 9 forks source link

minor cleanup and add GetHeight() function #5

Closed cbrake closed 5 years ago

cbrake commented 5 years ago

library is working great -- thanks!

pbnjay commented 5 years ago

re: MeasureString not returning the actual value - do you have an example of that?

Looking at the code I think there may be a bug in the fixed-width code that gives an extra pixel when drawing to characters that use the full "charWith" - the measurement code short-circuits drawing for fixed-width fonts so that would cause this difference. I'd rather fix the bug than cement the behavior in docs!

Let's not add the doc.go to cmd though, it's really just a workaround for godoc's quirks. Since fontgen isn't intended to be a general tool I don't think it is important to expose it in docs.

Thanks for contributing!

cbrake commented 5 years ago

yes, agreed it would be best if MeasureString is accurate, so I'll take another look at that. I was not sure what you intended.

Fine on doc.go.

May be a bit until I get back around to this, but I'll try to get back to you.

pbnjay commented 5 years ago

I just copy-pasta'd DrawRune's head to MeasureRune - does that address it?

cbrake commented 5 years ago

Your change to DrawRune does not seem to fix it. I added example code to this branch:

https://github.com/cbrake/pixfont/tree/tightpixel15-example

You can run the test by:

go run examples/tightpixel15/*

In the example I'm rendering the string: "ai"

It is returning the same width for each character.

(using go 1.11 here)

pbnjay commented 5 years ago

ok thanks for that test case, it definitely helped narrow it down. I believe 3716a538 fixes that issue! source of the problem was actually the initial value of bitMask and not w - whoops.

cbrake commented 5 years ago

that fixes it -- cleaning up pull request now ...

cbrake commented 5 years ago

pull request updated to address concerns in your 1st comment. Thanks for the MeasureString fix!