-
Notifications
You must be signed in to change notification settings - Fork 38
Fix anchored position rounding #83
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
Fix anchored position rounding #83
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In testing of your updated code, this resolves the x-position wandering.
However, I observed y-rounding errors. I am attaching proposed code changes (see the comments) to solve the rounding (I was afraid of making pull request for fear of messing something up, but let me know if that is easier for you and I will figure it out.)
Here is the code where I saw y-movement due to rounding:
print("Display is started")
# load all the fonts
print("loading fonts...")
# Setup file locations for BDF font files
font_files = [
"fonts/Helvetica-Bold-16.bdf",
# 'fonts/BitstreamVeraSans-Roman-24.bdf',
# 'fonts/BitstreamVeraSans-Roman-16.bdf',
# 'fonts/Fayette-HandwrittenScript-64.bdf',
]
font_list = []
for i, font_file in enumerate(font_files):
if use_builtinfont:
this_font = (
terminalio.FONT
) # comment this out to switch back to BDF loaded fonts
else:
from adafruit_bitmap_font import bitmap_font
this_font = bitmap_font.load_font(font_file)
font_list.append(this_font)
preload_the_glyphs = (
True # set this to True if you want to preload the font glyphs into memory
)
# preloading the glyphs will help speed up the rendering of text but will use more RAM
if preload_the_glyphs:
# identify the glyphs to load into memory -> increases rendering speed
glyphs = b"0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-,.:?! "
print("loading glyphs...")
for font in font_list:
if font is not terminalio.FONT:
font.load_glyphs(glyphs)
print("Glyphs are loaded.")
print("Fonts completed loading.")
# create group
gc.collect()
print("After creating Group, Memory free: {}".format(gc.mem_free()))
my_string = "Welcome to using displayio on CircuitPython!"
gc.collect()
label_start_memory = gc.mem_free()
start_time = time.monotonic()
bmap_label = label.Label(
font=font_list[0],
text=my_string,
color=0xFFFFFF,
max_glyphs=len(my_string),
background_color=0xFF0000,
padding_bottom=0,
padding_left=0,
padding_right=0,
padding_top=0,
background_tight=False,
x=10,
y=30,
line_spacing=1.25,
scale=my_scale,
anchor_point=(0.5,0.5),
anchored_position=(320/2, 240/2),
)
end_time = time.monotonic()
gc.collect()
label_end_memory = gc.mem_free()
bmap_group = displayio.Group(max_size=1) # Create a group for displaying
bmap_group.append(bmap_label)
print("***")
print("{} memory used: {}".format(version, label_start_memory - label_end_memory))
print("{} time to process: {} seconds".format(version, end_time - start_time))
display.auto_refresh = True
display.show(bmap_group)
while True:
print("anchor_point: {}, anchored_position: {}".format(bmap_label.anchor_point, bmap_label.anchored_position))
bmap_label.text='This is a test string'
time.sleep(0.1)
time.sleep(1)
print("anchor_point: {}, anchored_position: {}".format(bmap_label.anchor_point, bmap_label.anchored_position))
bmap_label.text=my_string
time.sleep(0.1)
time.sleep(1)
pass
@@ -399,7 +402,7 @@ def anchored_position(self, new_position): | |||
return # Note: anchor_point must be set before setting anchored_position | |||
new_x = int( | |||
new_position[0] | |||
- self._anchor_point[0] * (self._boundingbox[2] * self._scale) | |||
- round(self._anchor_point[0] * (self._boundingbox[2] * self._scale)) | |||
) | |||
new_y = int( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is my proposed update for the anchored_position
setter for variable new_y
:
new_y = int(round(
new_position[1]
- (self._anchor_point[1] * self._boundingbox[3] * self._scale)
+ ((self._boundingbox[3] * self._scale) / 2.0)
))
int( | ||
self.x | ||
+ round(self._anchor_point[0] * self._boundingbox[2] * self._scale) | ||
), | ||
int( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are my proposed changes for the anchored_position
y-getter
int(round(
self.y
+ (self._anchor_point[1] * self._boundingbox[3] * self._scale)
- ((self._boundingbox[3] * self._scale) / 2.0)
)),
Thank you @kmatch98! I'm trying out your changes now. |
I was able to use your code and reproduce the y shifting. I tested your changes successfully and it does resolve the shifting. Nice catch! Thank you. Latest commit adds those changes. |
@jepler do these fixes solve the moving text bug you observed on your Black Lives Matter Say Their Names display? |
Reminder: this Rounding bug needs to be corrected on |
No, this doesnb't fix the problem for me. Additionally, it changed the vertical positioning of some labels in my code that were NOT using anchored_point / anchored_position. These labels were shifted up:
These labels "slide slowly left" as their text is changed repeatedly. Reassigning the anchored_position and anchor_point properties keeps them in position, though
The full program is https://github.com/adafruit/Adafruit_Learning_System_Guides/blob/master/CircuitPython_SharpDisplay_Displayio/code.py and the font is also in the same folder on github. |
Here's a reproducer that doesn't actually need the display, but it DOES need the font.
|
Thanks for the update jepler. The vertical position changing for some of the texts not using anchored_position definitely is odd to me. I believe those should have been unaffected since our changes were only inside of Thanks for sharing this code. We will get it tested out and once we have another attempt at a fix we can test against this project. |
I will be putting in a PR to work around this issue in the Learn repo's script, so take care to grab the script at https://github.com/adafruit/Adafruit_Learning_System_Guides/blob/d130bff4eb53d5b25edb808c204988e88eb47557/CircuitPython_SharpDisplay_Displayio/code.py for your testing |
@jepler I ran your reproducer code on my end on a Wio Terminal device and I don't think I am seeing anything out of the ordinary. When I run that code and let it go for several seconds I am seeing the same x,y values printed over and over:
|
Also it's worth noting that when using However that is only if the actual String within the text is different. Resetting to the same text that is already present should not result in any movement ideally. |
One more quick update for now: @jepler are you certain you copied in the edited code from this PR to your device? I reverted back to the "real" display_text library and ran your reproducer and I do see the x position that gets printed changing. But when I run that same code with the modified display_text library from this PR I always see I know you are working on a Sharp display of some sort, I don't have access to try with that, but I am curious what microcontroller are you using? If it is something I have access to I'd like to give this reproducer code a try on my end using the same device if possible. |
I ran @jepler 's code on my ILI9341 display (320x240) using the currently-released code for Without changing the code, I changed the Also, I found a Black Lives Matter font here (Cornerstone): https://fontmeme.com/fonts/cornerstone-free-font/ |
It's quite likely I made a mistake in my testing. I'll test again. Thanks for your patience. Maybe I tested with the wrong code altogether -- I'm sure the behavior CHANGED depending which file I copied, so that points to me testing with the released version and with some unrelated version with other bugs.... |
I tested again and here's what I found:
The first point seems to an incompatible behavior between the last release 2.8.0 on July 24, which may or may not be intentional. By using anchored_position, I can write code that works with both versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I re-tested, I determined it fixes the reported problem; the other behavior I identified is not introduced by this PR.
Since we've got confirmation that this resolved the issue and an approving review I am going to go ahead and merge this so that hopefully we will minimize the number of folks that run across the bug in their projects. With regards to the x,y position changes when not using It is perhaps worth noting the default positioning for label does put the y position at the center of the label vertically. So if you set a label like:
The expected behavior is that top half of the text will be off the screen. You will only see the bottom half of the text shown in the top left corner of the display. If the y position getting used moved by half the height my guess is it's related to this default positioning somehow. |
Updating https://github.com/adafruit/Adafruit_CircuitPython_EPD to 2.7.0 from 2.6.2: > Merge pull request adafruit/Adafruit_CircuitPython_EPD#41 from makermelissa/master > Merge pull request adafruit/Adafruit_CircuitPython_EPD#40 from makermelissa/master Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 2.8.1 from 2.8.0: > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#83 from FoamyGuy/fix_anchored_position_rounding > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#85 from kmatch98/glyph_speedup > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#79 from kmatch98/bitmap_label > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#81 from kmatch98/anchor_fix
This resolves #82. The sample script in that issue and several other variations of it that I tested now result in the label remaining in the same position over time.