Coloured fonts #49
Coloured fonts #49
Conversation
|
Cool! I will look closer into this PR when I have fixed #51 which is quite critical. |
|
Lots of nice stuff here. I need to think a bit about the colouring scheme before merging as this can be a personal preference. |
|
I would love to see a screenshot of some sort on the UI changes. |
|
I'd put this on the blog post: https://i.imgur.com/k4wdxAz.jpg |
|
Should I bother rebasing this to current master, or should I just close the PR? |
|
Having a colour parameter for tft_putch() seems like a good addition to me. We can just have it default as white and if someone wants something custom they'll have to tweak it themselves in the source. |
|
Hi @tzarc. No, I definitely don't think you should close this PR. We (as in I and anyone else here interested) should come up with a "style sheet" kind of mechanism where a user can color the UI at build time without dirtying the source code. Some (like me) might prefer the dull monochrome UI which is why I do not want to add colouring. |
|
No problem, I'll knock something up to allow putting the colours in the Makefile -- something like COLOR_VOLTS, COLOR_AMPS, COLOR_INPUTVOLTS, COLOR_CV_CA, COLOR_ICONS? Also, now that there's a newer font generator script, should I just split out the PNG converter stuff to a different PR? I've been using my own font for testing purposes, as well as using FontAwesome for the icon generation -- perhaps the icons should be generated too? |
|
Yes, I think colours in the makefile should do it. Using a font for the icons is a clever idea that has not occurred to me, great thinking! Cool picture of you rig btw. Is it 3D printed? |
|
I too had issues with the latest ImageMagick and had to build it from source which took my VM an hour. Then once I'd grappled with the various files required I got this: I'm going to have a look at your generation of the fonts instead as where as I did get it to 'work' it was a real pain |
|
What are you using to generate your font images @tzarc? I guess the holygrail would be to point the makefile at a .TTF file and it works it all out from there |
|
I was originally using the old ImageMagick to generate fonts -- I use a TTF directly (I dislike the Ubuntu one). Haven't swapped back to using my personal selection, but I've put the test generator here: As for your problem -- make sure the buffer sizes at the top of tft.c match the largest glyph size of any of the fonts. You'll get artifacting if you don't as it'll create a buffer overrun! I doubt it'd work with the repo in its current form, mind you. Will look at consolidating both. |
The repo posted in my previous comment has an example of the icons, including using very condensed CV/CC as icons. Two PR's, or both under this? And yeah, 3D printed! No the "daily driver' though -- was more just mucking about. |
|
I've updated my PR to include just the colour changes for output voltage/current and input voltage. Makefile now accepts arguments COLOR_VOLTAGE, COLOR_AMPERAGE, and Will look at migrating across the image converter at a later stage. |
|
I wonder if we can make a C or python program that will be able to render the files directly from a .ttf. I'll have a look into it when I have some free time. Although then again Ubuntu comes with an easy installer for the older magick version so I'll give yours a spin
Try to make pull requests for one change at a time if you can. This would be a welcome change though as I've had to pull the thermometer out of mine a few times to free up some FLASH space |
Precisely why I ripped them out. :) Font generator-wise... definitely can. This sort of font format was something I used at a previous employer for OpenGL -- I actually wrote the C++ font generator for it for my team. Would be nice to just reproduce it, but alas, corporate ownership etc. etc. That said, Python would most likely be easier than writing it in C, purely from a maintenance perspective. |
The old PNG converter actually has a commented-out mode for storing fonts as 4bpp monochrome, with the intention of testing with a simple RLE applied to it... need to see if it's worth swapping to that storage mechanism in flash, and decoding at runtime. Not sure about RAM availability, need to check datasheets. |
|
I can certainly see font compression being a useful addition |
|
Yikes. Yeah, looks like it'd be a good addition. |
|
Aren't they currently 8bpp? So it would half the size not quarter? |
Nope, they're 16bpp -- they're RGB565, the buffers themselves being DMA'd to the LCD.
Which questions? |
|
Ah of course, thanks for the clarification. Considering we don't want rainbow coloured text I think 4bpp is a no brainer And I started a review yesterday. Github doesn't do a good job of notifying people about them Below is a screenshot of what it looks like on my end: |
|
This is what I see:
They were a copy over from the old branch, but I think they're actually now wrong -- if there's support for the DPS8005 in the mix, 80,000 mV won't fit into a uint16_t. Will swap them all back to uint32_t. |
opendps/uui_number.h
Outdated
| int32_t max; | ||
| int16_t value; | ||
| int16_t min; | ||
| int16_t max; |
There was a problem hiding this comment.
What's the mentality of changing these to int16_t out of interest?
opendps/tft.c
Outdated
| * @param highlight if true, the character will be inverted | ||
| * @retval none | ||
| */ | ||
| void tft_putch(uint8_t size, char ch, uint32_t x, uint32_t y, uint32_t w, uint32_t h, bool highlight) | ||
| void tft_putch(uint8_t size, char ch, uint32_t x, uint32_t y, uint32_t w, uint32_t h, uint32_t color, bool highlight) |
There was a problem hiding this comment.
I believe color is defined as a uint16_t in ili9163c.h not a uint32_t
|
Wow @tzarc, thanks! I've been using this review thing wrong for a while now.... |
opendps/uui_number.c
Outdated
| @@ -129,12 +129,13 @@ static void number_draw(ui_item_t *_item) | |||
| break; | |||
| } | |||
| uint16_t xpos = _item->x - w; | |||
| uint32_t color = item->color; | |||
There was a problem hiding this comment.
This should be a uint16_t I believe. Other than this it looks really good and would be a useful addition
There was a problem hiding this comment.
Done. Fixed up a couple of build warnings in the process, too.
Makefile now accepts arguments COLOR_VOLTAGE, COLOR_AMPERAGE, and COLOR_INPUT. Color names can be found in ili9163c.h.
|
Nice to see someone finally slay the mini-printf.c warning message! I'm all happy with this as it stands now |
|
Thanks @tzarc! |
As per the discussion on the blog post -- added support for different coloured meters on the screen.
Replaced the existing conversion code for fonts/graphics as well -- my build machine had serious problems with the latest ImageMagick, so it seemed like the easiest solution was to DIY it. No piping between multiple programs -- load input PNG and export the corresponding rgb565 data.