Issue with proper character positioning when applying 'just':'centerchar' for character based LCDs

[DennisB66]

When using the 'just':'centerchar' on a fixed character bases 20x4 LCD (hd44780_i2c / pages_lcd_20x4.py) the text is not aligned with a proper character positioning when the widget text contains a uneven number of characters when the available space is for an even number of characters (and vice versa).

The related code found in display.py (occurring two times):

        elif just == u'centerchar':
            # If the number of chars is even, then we should be ok
            if cx % 2 == 0:
                ax = (maxw-cx)/2
            else:
            # If it's odd though we'll get split so add another character worth of space to the calculation
                ax = (maxw-cx-fx)/2

For example when the text contains 8 characters (cx = 40) and the available space is 100 pixels (on a 20x4 LCD) this would lead to: ax = (100 - 40) / 2 = 30. When the text contains 9 characters (cx = 45) it is calculated as ax = (100 - 45 - 5) / 2 = 22.

This leads to garbage on the 20x4 LCD display since the outcome should be ax = 25 (multiple of fx = 5).

This issue can be solved by (twice) replacing the code section with:

        elif just == u'centerchar':
            ax = int( fx * round((( maxw - cx) / 2) / fx))

Where fx as the width of the characters is used to calculate the proper alignment.

This solves the issue for the 20x4 LCD display mentioned, but must work for all fixed character based LCD displays.

I will create a pull request. Please check if you agree with this solution.

In addition: This issue also shows that the code can use some improvements by removing all redundant code (I would expect only a single code section of this calculation of the proper 'centerchar' positioning). Furthermore should the 'center' tag be fixed character sensitive as well, making the explicit 'centerchar' tag obsolete. It took me a while to browse through the code to find why the 'center' tag was not working on my display and there was a 'centerchar' tag as well that was not mentioned in the documentation (but I understand this is still work in progress).

// Dennis

[dhrone]

Your solution is certainly more elegant but I'm confused as to why you were having character positioning errors. Given your example:

For example when the text contains 8 characters (cx = 40) and the available space is 100 pixels (on a 20x4 LCD) this would lead to: ax = (100 - 40) / 2 = 30. When the text contains 9 characters (cx = 45) it is calculated as ax = (100 - 45 - 5) / 2 = 22.

This calculation results in 25, not 22 which should be correct.

In any case, I do not see any reason not to use your calculation so will accept the changes. I'll also look at making the 'center' fixed character sensitive as you suggest.

FYI, I totally agree on the code clean-up comments. In this particular instance I was rapidly trying to add truetype support and just copied the text widget code over, which I know is not a particularly good coding practice. That said, I don't think anyone would ever use the 'centerchar' justification with ttype as those fonts are only appropriate for graphical displays anyway.

I really appreciate the contribution!

[DennisB66]

Sorry for the late reply. My example was indeed not correct, but still considered a bug fix related to a different use case. Apparently I was mixing up two different issues (center vs centerchar).

When the text exceeds the number the size of the display and the length of the text is uneven the value of ax becomes negative (alwas -fx/2), but should be zero.

For example the text contains 21 characters on a 20x4 LCD, then we get the following: maxw = 105 and cx = 105, making ax = (105-105-5)/2 = -3

The proposed solution does not have this flaw. Also not that the initial code was also causing issues with a even fx value (like when fx =4). It was not checking on even/uneven characters, but on even/uneven pixels.

While browsing again through the code I noticed I missed a 3rd spot of the code that needs the same fix. This one I think is related to a multi-line text, which never happens in my setup.

Will create another pull request to have this fixed this as well.