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

Made the demo thingy properly center text. #7

Merged
merged 3 commits into from
Jun 10, 2017

Conversation

Peetz0r
Copy link

@Peetz0r Peetz0r commented Jun 10, 2017

Also moved all of the text a bit to the right to allow for longer names.

http://imgur.com/a/UCyBV :)

… a bit to the right to allow for longer names.
@Peetz0r
Copy link
Author

Peetz0r commented Jun 10, 2017

Oh, and I also added some ifndefs because the simulator won't compile with the input stuff right now.

@Peetz0r
Copy link
Author

Peetz0r commented Jun 10, 2017

screenshot

Depends on SHA2017-badge/Firmware#23

We also lose ugfx.print_funts() which was somehow broken. See micropython#23
@Peetz0r
Copy link
Author

Peetz0r commented Jun 10, 2017

Note, commit message refers not to #23 in here, but SHA2017-badge/Firmware#23 (review) :)

Copy link

@raboof raboof left a comment

Choose a reason for hiding this comment

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

LGTM but would appreciate a second set of eyes on whether we're free to remove things from ugfx_module_globals_table


gdispClear(White);
gdispDrawString(150, 25, "STILL", robotoBlackItalic, Black);
gdispDrawString(130, 50, mp_obj_str_get_str(hacking), permanentMarker, Black);
gdispDrawStringBox(0, 6, 296, 40, "STILL", robotoBlackItalic, Black, justifyCenter);
Copy link

Choose a reason for hiding this comment

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

much nicer thx :)

@@ -655,8 +638,6 @@ STATIC const mp_rom_map_elem_t ugfx_module_globals_table[] = {
{MP_OBJ_NEW_QSTR(MP_QSTR_polygon), (mp_obj_t)&ugfx_polygon_obj},
{MP_OBJ_NEW_QSTR(MP_QSTR_fill_polygon), (mp_obj_t)&ugfx_fill_polygon_obj},

{MP_OBJ_NEW_QSTR(MP_QSTR_print_fonts), (mp_obj_t)&ugfx_print_fonts_obj},
Copy link

Choose a reason for hiding this comment

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

are we implementing some API here or do we decide for ourselves what needs to be exposed?

in any case the current implementation is ugly so removing the function altogether seems like an improvement to me, would appreciate a second set of eyes :)

@@ -0,0 +1 @@
../../components/ugfx/PermanentMarker36.c
Copy link

Choose a reason for hiding this comment

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

hmm it'd be nice to avoid this but it's consistent with how we handle other fonts ;)

Copy link
Author

Choose a reason for hiding this comment

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

I did this only because that's how the others were setup. It seems to just work when I remove the symlinks and just move the fonts from components/ugfx/ to micropython/esp32/. I could commit that to both PR's if desired.

Copy link

Choose a reason for hiding this comment

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

No please don't move them, then it won't work with pure ugfx (without python) anymore :).

it'd be nice if upy could use the ugfx fonts without these symlinks, but that might be hard for some reason. Anyway this is OK for now.

Copy link
Author

Choose a reason for hiding this comment

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

OK then :)

@raboof
Copy link

raboof commented Jun 10, 2017

Refs SHA2017-badge/Firmware#23

@raboof
Copy link

raboof commented Jun 10, 2017

Actually let's just go for it, can always add something later ;)

@raboof raboof merged commit 385ff98 into SHA2017-badge:esp32 Jun 10, 2017
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.

2 participants