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

Why is font_to_py.py modified? #9

Closed
peterhinch opened this issue Apr 11, 2021 · 6 comments
Closed

Why is font_to_py.py modified? #9

peterhinch opened this issue Apr 11, 2021 · 6 comments

Comments

@peterhinch
Copy link

peterhinch commented Apr 11, 2021

This program has had numerous improvements over the last three years thanks to input from a number of users.

I'm baffled as to why you include get_width() in the font file. In my work this function lives in the display driver. The use of a modified version has caused confusion in the forum. Might I suggest you reconsider this decision?

If there is a technical reason for including this method, I will consider updating my format to include it.

@jeffmer
Copy link
Owner

jeffmer commented Apr 11, 2021

Hi Peter, I have not touched this driver for over three years now. The reason why I put getWidth in the font was that as far as I remember it had something to do with efficiency. I am afraid that I am not in a position to test a modified version but would be happy if someone else wants to.

With regards to confusion - it is documented.....

I am quite surprised that it has been used so much as I found it fairly slow.

@peterhinch
Copy link
Author

It is worth noting that the requirement for get_width is more involved than at first sight. This is because some glyphs have trailing whitespace. After feedback from users who noted that widths were sometimes overstated, I implemented a revised algorithm in the Writer class. I don't plan to include this in my font file format. Users might choose to use a simpler, less precise but faster algorithm such as yours. My view remains that this code should be located elsewhere than in the font file.

It seems unfortunate that three years of development is lost. Aside from bug fixes, notable improvements are implementation of sparse fonts, better support for foreign character sets and support for minimal character sets such as a digital clock set. Also, relevant to tiny displays, support for bdf and pcf font files (i.e. hand drawn bitmapped font files). Sooner or later someone is going to try to use these features only to find them fail.

My approach would be to fork font_to_py.py rather than maintaining an independent version. That enables you to include your adaptation while ensuring that merging changes in master is trivially easy.

If you've moved on to other things, fair enough.

@jeffmer
Copy link
Owner

jeffmer commented Apr 20, 2021

I take your point entirely about accessing the latest version. I would prefer to not have a separate version of your excellent font_to_py program. Would there be any chance of you adding an option to output the get_width function so that I can refer to your repository directly. I think that the point of having get_width in the font file is the same as having the get_ch function in the font file. It avoids having to know details of the font file structure in the driver.

@peterhinch
Copy link
Author

A get_width function does not need to know details of the font file structure: it can use the documented fact that get_ch returns the width of the current glyph along with the glyph itself.

As I pointed out above, there is a choice of algorithms for get_width depending on whether you want to prioritise speed or accuracy. You might like to look at my stringlen and the _truelen method it calls to see what is involved in getting an accurate determination of length.

In many applications simply summing the glyph widths returned by get_ch would provide a good enough width, faster and with less code. In my view this choice between two equally legitimate approaches rules out incorporating the function into the font.

Looking at the font in your repo suggests your version of font_to_py.py is modified as your get_ch function returns only two values. Mine returns three, and has done since first posted. It returns the glyph followed by height and width. See the docs. I agree that there is no real need to return height, but I didn't want to foist a breaking change on users. Please bear this difference in mind if you do update.

@jeffmer
Copy link
Owner

jeffmer commented Apr 21, 2021

OK, let’s agree to disagree. Calling get_ch to get the width adds an extra level of function calls which is the efficiency reason I added get_width in the first place.

@jeffmer jeffmer closed this as completed Apr 21, 2021
@CodeClueless
Copy link

By the way, the font_to_py works if you cut and paste the last three functions of the downloadable fonts that come with the ili934Xnew.py and paste them into the font produced by font_to_py.

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

No branches or pull requests

3 participants