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

Transfer the scale attribute down to the inner local_group and keep the scale of Label at 1 #116

Merged
merged 2 commits into from
Feb 11, 2021
Merged

Conversation

lesamouraipourpre
Copy link
Contributor

This is a fix for #113 .

It keeps the scale of this outer Label at 1 and passes any scale setting
through to the local_group inside as described in the comments already in
the init method.

It keeps the scale of this outer Label at 1 and passes any scale setting
through to the local_group inside as described in the comments already in
the __init__ method.
Copy link
Contributor

@jposada202020 jposada202020 left a comment

Choose a reason for hiding this comment

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

I verify the changes for the proposal using a WIO terminal in CircuitPython 6.1.0 with the following code adapted from @lesamouraipourpre


"""
This examples shows the use of scale parameter.
"""

import board
import displayio
from adafruit_bitmap_font import bitmap_font
from adafruit_display_text import label

display = board.DISPLAY

# Font definition. You can choose any two fonts available in your system
FONT = bitmap_font.load_font("/fonts/Helvetica-Bold-16.bdf")

# Test parameters
SCREEN_BACKGROUND_COLOR = 0x990099
TEXT_BACKGROUND_COLOR = 0x440044
TEXT_COLOR = 0x00FF00
X_COL_POS = [10]  # X position of the Label boxes

TEST_TEXT = "pqQñúÇß"  # text for testing

LOCALISATION_Y = [50, 100, 150]  # Y coordinate of the text labels
TEST_SCALES = [1, 2, 3]


main_group = displayio.Group(max_size=len(LOCALISATION_Y)+1)
bg_bitmap = displayio.Bitmap(display.width, display.height , 1)
bg_palette = displayio.Palette(1)
bg_palette[0] = SCREEN_BACKGROUND_COLOR
main_group.append(displayio.TileGrid(bg_bitmap, pixel_shader=bg_palette))

# Test
for i, numerto_test in enumerate(TEST_SCALES):
    text = label.Label(
        font=FONT, 
        text=TEST_TEXT, 
        color=TEXT_COLOR, 
        background_color=TEXT_BACKGROUND_COLOR,
        scale=TEST_SCALES[i], 
        x=X_COL_POS[0], 
        y=LOCALISATION_Y[i]
    )

    main_group.append(text)

display.show(main_group)
display.refresh()

while True:
    pass

The solution works well in the WIO Terrminal

@kmatch98
Copy link
Contributor

kmatch98 commented Feb 8, 2021

This PR change causes an issue with the anchor_point and anchored_position calculations that rely on the self._scale value.

By changing all current references to scale._scale to self.scale will ensure that the overridden scale setter will be used. (Note: I think this will resolve the differences observed in Blinka, since the Blika's definition of displayio.Group contains a conflicting self._scale that is causing conflicts.) I also reviewed bitmap_label.py and it may have similar issues as observed with label.py and Blinka. Can you please check this?

Here's the error I got when running on a pyPortal:

test_label3.bounding_box: (0, -7, 30, 14)
Traceback (most recent call last):
  File "code.py", line 53, in <module>
  File "/lib/adafruit_display_text/label.py", line 454, in anchored_position
AttributeError: 'Label' object has no attribute '_scale'

Here's the code I ran, adding a change to the position using anchor_point and anchored_position:

import board
import displayio
from adafruit_pyportal import PyPortal
import terminalio

from adafruit_display_text import label
#from adafruit_display_text import bitmap_label as label

display = board.DISPLAY

# Make the display context
splash = displayio.Group(max_size=10)
#splash._scale=2
display.show(splash)

WIDTH = 320
HEIGHT = 240

# Draw a black background
bg_bitmap = displayio.Bitmap(WIDTH, HEIGHT , 1)
bg_palette = displayio.Palette(1)
bg_palette[0] = 0x000000
splash.append(displayio.TileGrid(bg_bitmap, pixel_shader=bg_palette))

test_label1 = label.Label(terminalio.FONT, text="Scale", scale=1, color=0xFFFFFF, x=8, y=8)
print("Test1 : before append to splash")
print(f"  Scale {test_label1.scale}")
splash.append(test_label1)
print("Test1 : after append to splash")
print(f"  Scale {test_label1.scale}")
print("test_label1.bounding_box: {}".format(test_label1.bounding_box))

print()
test_label2 = label.Label(terminalio.FONT, text="Scale", scale=2, color=0xFFFFFF, x=8, y=38)
print("Test2 : before append to splash")
print(f"  Scale {test_label2.scale}")
splash.append(test_label2)
print("Test2 : after append to splash")
print(f"  Scale {test_label2.scale}")
print("test_label2.bounding_box: {}".format(test_label2.bounding_box))

print()
test_label3 = label.Label(terminalio.FONT, text="Scale", scale=3, color=0xFFFFFF, x=8, y=88)
print("Test3 : before append to splash")
print(f"  Scale {test_label3.scale}")
splash.append(test_label3)
print("Test3 : after append to splash")
print(f"  Scale {test_label3.scale}")
print("test_label3.bounding_box: {}".format(test_label3.bounding_box))



##### Try moving the anchored_position
test_label3.anchor_point=(1,1)
test_label3.anchored_position=(WIDTH,HEIGHT)

@lesamouraipourpre
Copy link
Contributor Author

I will invesigate and test this tomorrow.

I originally came up three fixes:

  1. Remove the inner local_group so that there is only one scale value to deal with. I discounted this because I assume there was a good rationale for using the inner local_group.
  2. Follow the comments in the code and store the scale in the local_group and keep the outer scale at 1.
  3. Force the scale of the local_group to be 1 and store the scale on the outer Label.

This change currently implements option 2 because of the existing comments in the code. My preferred choice would actually be 3 and store the scale in the outer object.

Are there any preferences of whether to stick with option 2 or switch to option 3?

@kmatch98
Copy link
Contributor

kmatch98 commented Feb 8, 2021

Thanks for working on this.

Here’s what I remember about the rationale for the two nested groups in Label and Bitmap_Label.

When changing the scale of a label, we need “intercept” the scale change so we also update the position using anchored_position.

When overriding the Group’s scale setter, I ran into an issue where I could no longer access the Group’s class variable for scale. I think regular Python has a way using __set__ to access the parent’s variables. But I couldn’t get this to work in CircuitPython (that I was running on microcontrollers. It will probably work on Blinka.).

So as a solution, I added this second local_group to handle the scaling while leaving the top-level Group with scale=1 permanently.

Probably doesn’t help you find a solution, but I wanted to highlight a difference I’d seen in Blinka versus on microcontrollers. Also, I observed that Blinka’s displayio.Group definition has a self._scale variable, so that is another confusion that should be avoided in the solution.

To clarify, I think you’ll need to follow some form of 2 but if you can solve the parent __set__ problem then 1 is the way to go, but be sure to debug on a microcontroller since Blinka will likely act totally different.

Let me know if you have more questions.

@kmatch98
Copy link
Contributor

kmatch98 commented Feb 9, 2021

Looks good to me, I tested on a PyPortal. The use of anchor_point and anchored_position work fine now. I think this is ok to merge.

Note: The issue that this PR resolves (of using self._scale that causes problems on Blinka) is still present in bitmap_label. I will add an issue.

Tested with this code on a PyPortal:

import board
import displayio
from adafruit_pyportal import PyPortal
import terminalio

from adafruit_display_text import label
#from adafruit_display_text import bitmap_label as label

display = board.DISPLAY

# Make the display context
splash = displayio.Group(max_size=10)
display.show(splash)

WIDTH = 320
HEIGHT = 240

# Draw a black background
bg_bitmap = displayio.Bitmap(WIDTH, HEIGHT , 1)
bg_palette = displayio.Palette(1)
bg_palette[0] = 0x000000
splash.append(displayio.TileGrid(bg_bitmap, pixel_shader=bg_palette))

test_label1 = label.Label(terminalio.FONT, text="Scale", scale=1, color=0xFFFFFF, x=8, y=8)
print("Test1 : before append to splash")
print(f"  Scale {test_label1.scale}")
splash.append(test_label1)
print("Test1 : after append to splash")
print(f"  Scale {test_label1.scale}")
print("test_label1.bounding_box: {}".format(test_label1.bounding_box))

print()
test_label2 = label.Label(terminalio.FONT, text="Scale", scale=2, color=0xFFFFFF, x=8, y=38)
print("Test2 : before append to splash")
print(f"  Scale {test_label2.scale}")
splash.append(test_label2)
print("Test2 : after append to splash")
print(f"  Scale {test_label2.scale}")
print("test_label2.bounding_box: {}".format(test_label2.bounding_box))

print()
test_label3 = label.Label(terminalio.FONT, text="Scale", scale=3, color=0xFFFFFF, x=8, y=88)
print("Test3 : before append to splash")
print(f"  Scale {test_label3.scale}")
splash.append(test_label3)
print("Test3 : after append to splash")
print(f"  Scale {test_label3.scale}")
print("test_label3.bounding_box: {}".format(test_label3.bounding_box))

test_label3.anchor_point=(1,1)
test_label3.anchored_position=(WIDTH,HEIGHT)


while True:
    pass

@lesamouraipourpre
Copy link
Contributor Author

That's great. I was holding off on the bitmap one as I don't have a microcontroller to test it with, just a Raspberry Pi. I'm going to continue and have a deeper look at the code.

@kmatch98
Copy link
Contributor

kmatch98 commented Feb 9, 2021

Thanks @lesamouraipourpre. I went ahead and submitted a PR on bitmap version. #117

Please review and let me know if you see issues.

@kmatch98
Copy link
Contributor

kmatch98 commented Feb 9, 2021

BTW - your github handle had a few french-speaking folks confused during this week's CircuitPython weekly meeting phonecall. Again thanks for this work!

@lesamouraipourpre
Copy link
Contributor Author

BTW - your github handle had a few french-speaking folks confused during this week's CircuitPython weekly meeting phonecall. Again thanks for this work!

It's meant to be Purple Samurai which I think translates as le samouraï violet, but I like pourpre better than violet. And 'Le samouraï' is one of my favourite films.

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Looks good to me. Tested with Blinka_Displayio and PyPortal. Scaling works consistently across the board now.

Thank you @lesamouraipourpre for this fix!

@FoamyGuy FoamyGuy merged commit 68aad7c into adafruit:master Feb 11, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 12, 2021
Updating https://github.com/adafruit/Adafruit_CircuitPython_BME680 to 3.3.1 from 3.3.0:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_BME680#40 from caternuson/iss39
  > Merge pull request adafruit/Adafruit_CircuitPython_BME680#36 from adafruit/REUSE
  > Merge pull request adafruit/Adafruit_CircuitPython_BME680#38 from caternuson/iss37_spi_example
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_CLUE to 2.2.7 from 2.2.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_CLUE#37 from adafruit/REUSE
  > Hardcoded Black and REUSE versions
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_FONA to 2.1.2 from 2.1.1:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_FONA#12 from adafruit/REUSE
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_HTS221 to 1.1.3 from 1.1.2:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_HTS221#6 from adafruit/REUSE
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_MSA301 to 1.2.5 from 1.2.4:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_MSA301#14 from adafruit/REUSE
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_NeoPixel to 6.0.2 from 6.0.1:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_NeoPixel#103 from adafruit/REUSE
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_RockBlock to 1.3.1 from 1.3.0:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_RockBlock#19 from adafruit/REUSE

Updating https://github.com/adafruit/Adafruit_CircuitPython_SCD30 to 2.0.2 from 2.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_SCD30#9 from caternuson/iss2_mcp2221

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1327 to 1.2.1 from 1.2.0:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1327#9 from adafruit/REUSE
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_ST7789 to 1.4.3 from 1.4.2:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_ST7789#21 from wildestpixel/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_TCA9548A to 0.3.4 from 0.3.3:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_TCA9548A#23 from SAK917/adafruit/Adafruit_CircuitPython_TCA9548A#20-remove-obsolete-code

Updating https://github.com/adafruit/Adafruit_CircuitPython_AWS_IOT to 2.0.5 from 2.0.4:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_AWS_IOT#16 from adafruit/REUSE
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_AzureIoT to 2.3.3 from 2.3.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_AzureIoT#25 from adafruit/REUSE
  > Hardcoded Black and REUSE versions
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bitmap_Font to 1.3.4 from 1.3.3:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_Bitmap_Font#37 from jfurcean/fix_api_docs

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_MIDI to 1.0.3 from 1.0.2:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_MIDI#6 from adafruit/REUSE

Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 1.12.3 from 2.12.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#117 from kmatch98/bitmap_label_scale
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#116 from lesamouraipourpre/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_Hue to 1.1.3 from 1.1.2:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_Hue#14 from adafruit/REUSE

Updating https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad to 0.13.2 from 0.13.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_ImageLoad#43 from adafruit/REUSE
  > Hardcoded Black and REUSE versions
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_LIFX to 1.9.4 from 1.1.2:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_LIFX#10 from adafruit/REUSE

Updating https://github.com/adafruit/Adafruit_CircuitPython_MagTag to 1.6.0 from 1.5.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_MagTag#54 from makermelissa/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 1.9.6 from 1.9.5:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#68 from jfabernathy/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_SimpleIO to 3.0.1 from 3.0.0:
  > Hardcoded Black and REUSE versions
  > Merge pull request adafruit/Adafruit_CircuitPython_SimpleIO#60 from adafruit/REUSE
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

Successfully merging this pull request may close these issues.

4 participants