TFT_eSPI_Button ABI compliance

[tobozo]

Just an idea I'm throwing here but TFT_eSPI's Button class (similar to the actual TouchButton) handles the button rendering too.

So I'm wondering if extending the TouchButton model with TFT_eSPI_Button's drawing methods would make sense, and I can eventually do some research+PR as I have a couple of test cases.

@ropg your feeback on this would be a bliss

[ropg]

I think it makes most sense to create an overarching class that inherits TouchButton and draws the button, possibly using TFT_eSPI's Button. That way it's only one call, the events work for it, etc. Could call it "VisualButton" ?

Shall I whip something up?

[ropg]

Alright. Have a play with the "visual" branch on my fork. No documentation yet, but the example from Examples directory should tell you what's going on.

I ended up sticking it all in TouchButton. Negligible overhead and completely backwards compatible with the version that doesn't show on the display. One can even create a user draw function per button, etc etc etc.

I'd be happy to hear what you think.

[tobozo]

this is great !

The example works just fine and is self-explanatory on the syntax indeed.

There are many great projects out there using TFT_eSPI syntax and this would make a great legacy to have them work with very little or no change in their code.

With that in mind, I've tried to compile a sketch based on this TFT_eSPI example code, and make a quick inventory of the differences with the TFT_eSPI 'legacy' approach (without handlers and gestures).

The compiler only yields minor complaints about a few missing things:

• TouchButton::contains( x, y ) -> easily solved/aliased
• TouchButton::press() and TouchButton::release() -> easily solved, setState() already does both
• TouchButton::justPressed() and TouchButton::justReleased() -> easily aliased
• M5.Lcd.getTouch( &x,&y ) i.e. for older sketches that don't use gestures -> easily added
• TouchButton::drawButton( state, label ) -> a bit more tricky
• TouchButton::TouchButton(), TouchButton::initButton(), and TouchButton::setLabelDatum() -> needs a legacy support

Now should I modifiy the TFT_eSPI example sketch (mostly with #ifdefs) or overload TouchButton with aliases in order to see if full compatibility can be achieved between both implementations of touch buttons ?

I may try both :-)

[ropg]

I wanted to make something nicer and took the TFT_eSPI thing more as inspiration than as something to be compatible with. I would first try making a child class that just overloads what you need to change.

[tobozo]

Thanks for that, as per your suggestion I went with modifying the existing code and everything went well and smooth, so I applied this to the M5Stack-SD-Updater, and the resulting code is much shorter than its TFT_eSPI counterpart :+1:

Here's a couple of things I observed:

• setTextFont isn't implemented (throws a weird error at compilation)
• a free font is set event though setFont() wasn't explicitely called

[ropg]

Bear with me and see if I follow everything correctly, I had never really looked at touch or buttons in TFT_eSPI before, but now I have a little bit.

First your observations: I have a .setFont() with takes either a number for a text font or a pointer for a freeFont, and my default is indeed a FreeFont because prettier. I don't see the TFT_eSPI_Button object having a .setTextFont(), so I'm puzzled as to why you expect it.

As for the broader issue: The TFT_eSPI repository, in the extension directory (which the M5Stack doesn't have compiled in or even present) contains touch and button functionality, the touch as extra functions and data for the root display object, the button as an object. Their touch implementation appears geared towards resistive touch screens that are popular with these small displays (calibration, etc, etc).

The M5Stack people have already inherited from and overloaded the TFT_eSPI class in their M5Display class. I think that would be the proper place to put (behind #ifdef ARDUINO_M5STACK_Core2) the short set of stubs such as .getTouch () and translate to our touch object. We'll just ignore and fake the calibration stuff and all. TFT_eSPI_Button could be defined in touch.h and would be a descendant from TouchButton that converts and passes things on. That way we can create something that "just works" with all code written for TFT_eSPI, simply by using M5.Display.

(I changed the way the datum is done bc that was braindead in TFT_eSPI_Button, but I'll make a compatibility flag.)

This way the example code you pointed to earlier could compile by just adding #include <M5Core.h> and by replacing TFT_eSPI tft = TFT_eSPI(); with M5Display& tft = M5.Lcd;

Happy to hear your thoughts before I start typing, I'll just start documenting what I did yesterday first.

[tobozo]

:bear: thanks for your great feedback, very instructive, motivating and I'm sure I'm not the only one to appreciate :+1:

setTextFont : TFT_eSPI's initButton() method has an extra argument for the font size and I was wrongly assuming this value was a font, this explains the weird error at compilation :man_facepalming:

I have two lame arguments about not loading freefont by default:

• FreeFont have an significant impact on flash space
• Choosing a Latin alphabet is probably arbirary

And also one questionable argument:

• Choosing freefont over the default font makes TouchButton::draw invasive as it requires a text style freeze/restore before and after every draw.

I found out that freezing/restoring text style is limited, for some reason (maybe clumsy code?) this crashes:

  // capture text style
  uint8_t textsize = M5.Lcd.textsize,  // Current font size multiplier
         textdatum = M5.Lcd.textdatum, // Text reference datum
          textfont = M5.Lcd.textfont;  // Reliable value ?
  ButtonColors ColorOn = {RED, WHITE, WHITE};
  ButtonColors ColorLoadOff = { TFT_RED, TFT_GREY, TFT_ORANGE };
  TouchButton LoadBtn(0, 100,  160, 80, "Load", ColorLoadOff, ColorOn, MC_DATUM);
  LoadBtn.draw();
  LoadBtn.addHandler(LoadBtnTapped, TE_TAP + TE_BTNONLY); // will update "tapped" boolean
  auto msectouch = millis();
  unsigned long waitdelay = 5000; // wait 5 seconds for a tap
  do {
    M5.update();
  } while( ! tapped && millis() - msectouch < waitdelay );
  // restore initial text style
  M5.Lcd.setFont( textfont ); // <<< this makes the M5Stack crash
  M5.Lcd.setTextSize( textsize );
  M5.Lcd.setTextDatum( textdatum );

So I ended up forcing M5.Lcd.setFont( nullptr ) instead of M5.Lcd.setFont( textfont ) as a temporary workaround until I figure out why this crashes.

[ropg]

I would like to stick with what M5Stack already has, and they have freefonts compiled in. While not compiling them in is an option, it is an option for the display driver (#define LOAD_GFXFF in In_eSPI_Setup.h, not my stuff, right? If you turn that off, I don't see anything that would crash except having to change the default font to a textFont.

(There's plenty that I might do differently, but I am not signing up to re-architecting the M5Stack library, just to gve it kick-ass touch, buttons, gestures and all.)

Speaking of default fonts: I changed how that works now. Get the latest from the visual fork. It has a global M5.Touch.SetFont() and the buttons have the text size set to 0 be default. That is illegal, so my draw code knows to use the global font, same for textsize. The new version also has rotation. If you call M5.Touch.setRotation(3) both the screen and the touch will be upside down. That means we now have buttons with possible negative coordinates for the area outside of the screen, which it now supports. (And it moves BtnA-BtnC to the right place in the new coordinate system...)

As for your code, I don't know why it crashes, shouldn't as far as I can see. Am curious what value it is trying to restore.

In M5Display.h, setFont() is just an alias for setFreeFont(). And then in In_eSPI.h there's:

#ifdef LOAD_GFXFF
           setFreeFont(const GFXfont *f = NULL),
           setTextFont(uint8_t font),
#else
           setFreeFont(uint8_t font),
           setTextFont(uint8_t font),
#endif

Then in In_eSPI.cpp there's (essentially):

#ifdef LOAD_GFXFF

void TFT_eSPI::setFreeFont(const GFXfont *f) {
  if (f == nullptr) // Fix issue #400 (ESP32 crash)
  {
    setTextFont(1); // Use GLCD font
    return;
  }

  textfont = 1;
  gfxFont = (GFXfont *)f;

  ....blabla
}

void TFT_eSPI::setTextFont(uint8_t f) {
  textfont = (f > 0) ? f : 1; // Don't allow font 0
  gfxFont = NULL;
}

#else

void TFT_eSPI::setFreeFont(uint8_t font) {
  setTextFont(font);
}

void TFT_eSPI::setTextFont(uint8_t f) {
  textfont = (f > 0) ? f : 1; // Don't allow font 0
}

#endif

I was trying to restore the font setting, only to find that that's not easily possible because gfxFont is protected. That means like private but accessible to descendant classes, so at least we can screw with M5Display and not the underlying driver. I was thinking of adding statePush() and statePop() to the display driver. And then just have a struct for all the settings and a std:vector of these structs. That way we solve the problem for everyone that wants to non-invasively do things on the display.

[tobozo]

Thanks for this analysis, now I have a better understanding of why lovyan03 rewrote the whole font stuff in LovyanGFX :-)

I've pulled the last changes, so far so good :+1:

[ropg]

Just pushed new version to visual fork. New M5Display functions M5.Lcd.pushState() and M5.Lcd.popState() are now called from the draw routine. Everything completely non-invasive, as proven by:

#include <M5Core2.h>

TouchButton br(165, 125, 155, 115, "push me", {BLACK, WHITE, WHITE}, {RED, WHITE, WHITE});

void setup() {
  M5.begin();
  M5.Lcd.setTextColor(GREEN, BLACK);
  M5.Lcd.setFreeFont(FM18);
  br.addHandler(printIt, TE_TOUCH);
  M5.Touch.setFont(FSS12);
  M5.Touch.drawButtons();
}

void loop() {
  M5.update();
}

void printIt(TouchEvent& e) {
  M5.Lcd.println("Test");
}

Note that it starts printing too high, but it does that independent of my code, it appears set for bottom left datum at 0, 0. It should probably be set to be TL_DATUM by default, but that breaks many people's code, and is outside my immediate scope.

[ropg]

TFT_eSPI_Button support is here !!

With the current version of the library this compiles if I replace

#include <TFT_eSPI.h>

TFT_eSPI tft = TFT_eSPI();

with

#include <M5Core2.h>

M5Display& tft = M5.Lcd;

and then in setup() replace Serial.begin(115200); with M5.begin();

• The result is quite ugly, but I don't think that's my code's fault. (Please verify...) It looks marginally prettier if you take #define KEY_H 22 up to 30. I hope you know some worthwhile code written with TFT_eSPI_Button so we can test with something a bit more appealing.

• First time it takes a while as it goes and formats SPIFFS to write its calibration data, which I happily leave at all zeroes.

• TouchButton is damn nice in comparison to TFT_eSPI_Button, most of the work was in dumbing it down to be compatible.

Now on to documenting, testing and issuing PR

[tobozo]

that's great news !

The ESP32Marauder seems a good candidate as it makes a strong use of TFT_eSPI and Touch, with some extra complexity as it handles both portrait and landscape modes.

I have a fork of this project which will require very little adaptation to work with M5Core2.h, I'll give it a shot during the weekend.

[edit] Success !!

Possible issue using TFT_eSPI_Button: TouchZones events are still fired even when the buttons aren't displayed, I'm trying to figure out if this is a bug in the ESP32Marauder or in the library.

[ropg]

Sure. Remember that if you call M5.Touch.setRotation() instead of M5.Lcd.setRotation(), it will rotate screen and touch. i'll make it so that you can rotate either to rotate both, so that's just for now.

[tobozo]

Longer ESP32Marauder demo some buttons are still ghosted though, could it be a missing ~TFT_eSPI_Button(); destructor ?

[edit] this has been working flawlessly with every other sketch I've tested so far.

Does it need more extended tests ?

[ropg]

Made some changes to underlying Button object. Please get my latest version and re-test. Also let's use the issues there until new revisions are ready for a PR here.

[tobozo]

Hey Rop

So I just git-pulled the master branch of the library and have issues with TFT_eSPI emulation, it looks like one of getTouch(), justPressed() and justReleased() always return true.

I'm trying to figure out where this comes from but I suspect BtnA, BtnB and BtnC are interferring when 1) not used in the sketch 2) zones overlap with custom buttons 3) had initButton() called more than once

Does this ring a bell ? If not I can eventually try to reproduce that nested menu situation in an example sketch, just let me know.

[ropg]

Very plausible...

I'm a bit busy today, and in the grand scheme of things the resistive-touch emulation is not the highest priority right now. I will get to it, but other things come first...

[tobozo]

Hey Rop,

I can understand you're busy and have higher priorities :+1:

As a followup, this PR takes care of the getTouch(), justPressed() and justReleased() always return true issues with TFT_eSPI_Button emulation:

https://github.com/ropg/M5Core2/pull/73

Is there something else I can do until your priorities let you spend time on this project?

take care ^^

[bump]

[Tinyu-Zhao]

Merged.