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

Allow for multiple OLED font sizes (#6650) #13233

Closed
wants to merge 2 commits into from

Conversation

BifbofII
Copy link

Description

This is my attempt of enabling different font sizes in the OLED driver. The main idea of the implementation is taken from the referenced issue and was extended by me. The features can be summarized as follows:

  • Default configuration has the exact same bahaviour as before.
  • By defining the OLED_FONT_SIZE parameter, the font size can be changed statically. I am planning to include font sizes 1 (default, 8px high), 2 (16px high) and 3 (24px high), even though 3 might already require too much memory for most boards.
  • By setting OLED_MULTIPLE_FONT_SIZES a new mode is selected, which includes the full font with all symbols in size 1 and additionally only alphanumerical characters for larger font sizes. To select the font size dynamically, all methods based on the font or characters are extended with a size parameter. This allows for dynamically printing text in different font sizes.

The reasoning for only including alphanumerical characters in the larger fonts in multi size mode is to reduce the required space.

The larger fonts are stored in a special format (each char being stored in height / 8 chunks of width bytes) that allows to only slightly modify the memcpy based approach used for writing to the render buffer at the moment, but at the cost of being a custom font format. I am planning to look into modifying this conversion tool to fit that custom format, to provide a tool for creating new fonts. For now, only the original font and a version naively scaled by a factor of 2 is included.

Types of Changes

  • Core
  • New feature
  • Enhancement/optimization
  • Documentation (will be updated once the feature is complete)

Issues Fixed or Closed by This PR

BifbofII added 2 commits June 17, 2021 23:37
* Modify OLED char printing to work with fonts of height multiples of 8
* Add font with height 16 that is naively scaled
* Add configuration parameter to choose font size
Add multi font size mode where font size can be selected at runtime
For all fonts except size 1, only the ASCII characters 32 to 126 are
available to save flash space.
@github-actions github-actions bot added the core label Jun 17, 2021
@XScorpion2
Copy link
Contributor

OLED_FONT_SIZE with fixed font sizes make sense, and from looking at the code it looks reasonable.

OLED_MULTIPLE_FONT_SIZES I would argue has no place in this driver and should be a separate driver (quantum painter based?) for a couple of reasons. First being the API contract changes with that define and now require a size parameter. Second, the OLED DRIVER was intended to be optimized for simple display cases for AVR MCUs primarily, with the intent that more complex cases, or more powerful MCUs using a different, more feature rich, driver possibly based on some of the core ideas from OLED DRIVER. Thus my comment about possibly using quantum painter for this when it's more complete.

Would like to get some of the QMK Contributors view on this as well.

@@ -3,6 +3,7 @@
// Helidox 8x6 font with QMK Firmware Logo
// Online editor: http://teripom.x0.com/

#if OLED_FONT_SIZE == 1
Copy link
Member

Choose a reason for hiding this comment

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

This I'm not a huge fan of.

It might be worth renaming to font_6x8 or what not.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think I quite get what you mean.
Do you mean removing the hard coded 1 and having something like #if OLED_FONT_SIZE == OLED_FONT_6x8 with OLED_FONT_6x8 being defined somewhere in the header?
Or do you mean removing the preprocessor directive altogether, name all of the fonts differently and have the compiler remove the unused fonts?

@BifbofII
Copy link
Author

@XScorpion2 Thanks for your feedback. I hadn't even seen Quantum painter before, but it looks very interesting indeed. The problem sort of is, that the memory requirements for the larger fonts explode quite fast, with a size 4 font (32x24) being basically as large as the ATmega32u4s flash that I'm testing with. That's why I came up with the idea of supporting multiple font sizes and removing symbols from the larger fonts. But I do get if you think that's not a feature that belongs in this driver and the API change isn't that nice indeed.
Do you think it would be an idea (for the fixed one size mode) to provide some reduced font sets that can be optionally chosen where some of the icons etc are removed. I don't know how many people are actually using them and they make up almost halve of the font.

@stale
Copy link

stale bot commented Aug 4, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stale
Copy link

stale bot commented Sep 7, 2021

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants