Skip to content

Fix anchor_position getter/setter, anchor_point setter and display_text_anchored_position example #43

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

Merged
merged 22 commits into from
May 27, 2020

Conversation

kmatch98
Copy link
Contributor

@kmatch98 kmatch98 commented May 23, 2020

  • I found an error in the calculations for the anchor_position getter and setter, that incorrectly placed the x,y locations based on the specified anchor_point and anchor_position. I updated these calculations to be correct.

  • I updated the anchor_point setter so that the x,y positions are recalculated upon a change to anchor_point.

  • I updated the example/display_text_anchored_position.py so that the tested anchor positions are directly at the edge and corners of the display

  • Updated the font and text setters such that anchored_position is properly updated.

Verification:
I verified on an ItsyBitsy NRF52840 running an ILI9341 display 320x240 pixels running the Adafruit display driver.

Other impacts:
This update may "break" existing projects using label since the positions of the labels will likely change versus with the current version. I will understand if this change will cause more trouble than help. Any feedback or suggestion is welcome.

Another note:
Unfortunately, I made these changes on the my local branch that also has an outstanding pull request for the font updates (see pull request #42). I recommend cancelling PR #42 and focusing on this one.

However, let me know if you would like me break this out into two separate pull requests all back to the adafruit/master branch.

Margaret Matocha added 20 commits May 22, 2020 07:44
@kmatch98
Copy link
Contributor Author

Yay, this one finally passed the checks. This combines the changes from PR#42.

Sorry for not doing it the simple way! Let me know if you want to take this one at a time or if I need to cancel one of these. Thank y'all!

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.

I lean toward closing the other PR and just using this one even though it has both changes. But we can wait to see what @kattni or another member of the team thinks. If we accept this one we can make sure to mention both changes in the release notes.

The anchor positioning is good and the example code now correctly aligns on 0s.

I used this code to test at the font issue and fix:

import board
import terminalio
from adafruit_display_text import label
from adafruit_bitmap_font import bitmap_font
import time

text = "Hello world"
text_area = label.Label(terminalio.FONT, text=text)
text_area.x = 10
text_area.y = 10
board.DISPLAY.show(text_area)
# any 2 fonts
font1 = bitmap_font.load_font("/fonts/Arial-16.bdf")
font2 = bitmap_font.load_font("/fonts/Helvetica-Bold-16.bdf")
time.sleep(5)
while True:
    text_area.font = font1
    # uncomment to observe current library attempt to change text:
    #text_area.text = "Hello Fonts"
    time.sleep(5)
    text_area.font = font2
    # uncomment to observe current library attempt to change text:
    #text_area.text = "Hello Jello"
    time.sleep(5)

With the current library this code doesn't change the screen at all. If you un-comment the .text setter lines the screen does change but the text gets weirdly shaped and positioned.

With the code from this PR the text will cycle through the fonts, both with and without the .text setters.

@kmatch98
Copy link
Contributor Author

Thanks for the testing @FoamyGuy. Let me know if you find anything else weird, such as repositioning an already created instance.

@tannewt
Copy link
Member

tannewt commented May 26, 2020

@FoamyGuy Go ahead and finish the review and merge. I closed the other PR. Thanks @kmatch98 !

@kmatch98
Copy link
Contributor Author

Thank y’all for all the support!

@FoamyGuy FoamyGuy self-requested a review May 27, 2020 22:42
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. Thank you for both of these fixes! @kmatch98

@FoamyGuy FoamyGuy merged commit e191c8a into adafruit:master May 27, 2020
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request May 28, 2020
Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 2.6.0 from 2.5.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#43 from kmatch98/fix_anchoring
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#40 from adafruit/setup-py-disabled

Updating https://github.com/adafruit/Adafruit_CircuitPython_LED_Animation to 2.1.2 from 2.1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_LED_Animation#31 from rhooper/setup-py-package
  > Merge pull request adafruit/Adafruit_CircuitPython_LED_Animation#30 from rhooper/fix-package-name
  > Merge pull request adafruit/Adafruit_CircuitPython_LED_Animation#29 from rhooper/pulse_rgbw_fix
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.

3 participants