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

fix glyph alignment problem #20

Merged
merged 4 commits into from
Feb 12, 2017
Merged

fix glyph alignment problem #20

merged 4 commits into from
Feb 12, 2017

Conversation

yhfudev
Copy link

@yhfudev yhfudev commented Dec 21, 2016

  1. fix glyph alignment problem
  2. update the default charset to 0-255(unicode) to cover more languages by default.

@AnHardt
Copy link

AnHardt commented Dec 21, 2016

That's looking promising.
Will try soon.

@@ -1265,7 +1270,7 @@
// RepRapDiscount FULL GRAPHIC Smart Controller
// http://reprap.org/wiki/RepRapDiscount_Full_Graphic_Smart_Controller
//
//#define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER
#define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER
Copy link
Author

Choose a reason for hiding this comment

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

Please ignore the changes to the file Marlin/Configuration.h

Copy link
Owner

@thinkyhead thinkyhead Dec 24, 2016

Choose a reason for hiding this comment

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

Even if I ignore the changes, git won't.

You may revert these changes in your branch, clean up excess commits with git rebase -i HEAD~5, and use git push -f to update the PR, if you wish. Otherwise I would need to do all of that myself.

@AnHardt
Copy link

AnHardt commented Dec 22, 2016

Tests look good.
Alignment problem is fixed.
The languages using DISPLAY_CHARSET_ISO10646_1 will never need a font refresh.
Using permanently TALL_FONT_CORRECTION is not that beautiful but readable - for now ok.
Char displays seem to work as we are used to.

What to do next:
Update and refresh to current language files.
Especially Turkish.
Do we want to make an exception and provide the binaries for the programs needed for font refreshing (at least for windows?) ?
Merge to RCBugFix.
Cleanup.:
Rework the relevant parts of ISO10646-0-3.bdf to use not more than one pixel descent.
Remove most of the dogm_font_data_ISO10646_*.h files.
We don't need the *.fon files anymore.
Do we still want to support USE_BIG_EDIT_FONT and USE_SMALL_INFOFONT?
...

@@ -318,10 +318,10 @@ FORCE_INLINE void _draw_heater_status(const uint8_t x, const int8_t heater) {
#endif

if (PAGE_UNDER(7))
_draw_centered_temp((isBed ? thermalManager.degTargetBed() : thermalManager.degTargetHotend(heater)) + 0.5, x, 7);
_draw_centered_temp((isBed ? thermalManager.degTargetBed() : thermalManager.degTargetHotend(heater)) + 0.5, x, 7-1);
Copy link
Owner

Choose a reason for hiding this comment

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

@yhfudev Is it possible here for the glyph to extend below pixel row 6 and into pixel row 7?


if (PAGE_CONTAINS(21, 28))
_draw_centered_temp((isBed ? thermalManager.degBed() : thermalManager.degHotend(heater)) + 0.5, x, 28);
_draw_centered_temp((isBed ? thermalManager.degBed() : thermalManager.degHotend(heater)) + 0.5, x, 28-1);
Copy link
Owner

@thinkyhead thinkyhead Dec 24, 2016

Choose a reason for hiding this comment

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

@yhfudev Is it possible here for the glyph to extend below pixel row 27 and into pixel row 28?

I ask because, if the position of the glyph changes we should alter PAGE_CONTAINS(21, 28) too.

Copy link
Author

Choose a reason for hiding this comment

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

The y offset of the glyph is 6, is because of the value yoff of the FONTBOUNDINGBOX in BDF file is set to -2. The height of the glyph is 8, so we have to set the position to (8-2) so that the glyph can be showed in full at the top.

Copy link
Author

Choose a reason for hiding this comment

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

So I don't think you need to change the PAGE_CONTAINS(21, 28).

@@ -174,7 +173,7 @@ static int fontgroup_cb_draw_u8g (void *userdata, const font_t *fnt_current, con
assert(NULL != userdata);
if (pdata->fnt_prev != fnt_current) {
u8g_SetFont(pdata->pu8g, fnt_current);
u8g_SetFontPosBottom(pdata->pu8g);
//u8g_SetFontPosBottom(pdata->pu8g);
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm.

@thinkyhead
Copy link
Owner

Do we want to make an exception and provide the binaries for the programs needed for font refreshing…?

I think that's a good idea. If the tools were implemented in Java, Javascript, Python, or GNU shell scripting, they'd be more portable. Simple C programs are of course also acceptable, if they use common libs and do simple stream-in/stream-out style.

@thinkyhead
Copy link
Owner

Do we still want to support USE_BIG_EDIT_FONT and USE_SMALL_INFOFONT?

Maybe. The "small" font doesn't end up being much smaller than the normal font, so I'm less worried about it. But the big edit font might be useful for some vendors / setups that want to have a more custom appearance for the edit screens.

@thinkyhead thinkyhead merged this pull request into thinkyhead:rc_utf8_diff Feb 12, 2017
@yhfudev yhfudev deleted the rc_utf8_diff branch March 24, 2018 23:57
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