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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix triangle glyphs scaling by drawing them manually #2074

Merged
merged 9 commits into from Oct 18, 2019
Merged

Fix triangle glyphs scaling by drawing them manually #2074

merged 9 commits into from Oct 18, 2019

Conversation

bew
Copy link
Contributor

@bew bew commented Oct 18, 2019

Fixes #2006

It works so well!
馃帀 馃帀 馃帀
Thanks for the help @kovidgoyal


I updated the testing functions to use the new format of the class setup_for_testing (was probably a function before, now it uses a with ...: syntax).

I also changed something in icat, because the testing functions for box drawing make use of the method show from icat, which previously gave an error saying that screen_size is None at this line:

ss = screen_size()

(which is called from show)

@kovidgoyal
Copy link
Owner

Please resolve the conflicts then I will merge.

@bew
Copy link
Contributor Author

bew commented Oct 18, 2019

The builds fail because there is a trailing whitespace somewhere... I'm not on my computer right now, so It's pretty hard for me to find it (github UI is not helpful), could you take a quick look?

As a sidenote, it would be nice to see where the trailing whitespace is in the CI
nevermind I'm blind

kitty/fonts.c Show resolved Hide resolved
@bew
Copy link
Contributor Author

bew commented Oct 18, 2019

Note: I didn't test this last change on glyph ids as I'm not on my computer right now, will do later today if you don't

@bew
Copy link
Contributor Author

bew commented Oct 18, 2019

I get a test failure now:

FAIL: test_box_drawing (kitty_tests.fonts.Rendering)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/circleci/project/kitty/launcher/../../kitty_tests/fonts.py", line 55, in test_box_drawing
    self.assertEqual(len(self.sprites), prerendered + len(box_chars))
    line = '鈹鈹佲攤鈹冣晫鈺嶁攧鈹呪攬鈹夆晭鈺忊攩鈹団攰鈹嬧暣鈺碘暥鈺封暩鈺光暫鈺烩暭鈺解暰鈺縗ue0b0\ue0b2\ue0b4\ue0b6\ue0b1\ue0b3\ue0b8\ue0ba\ue0bc\ue0be鈺愨晳鈺炩暋鈺モ暔鈺暙鈺暊鈺b暒鈺┾暠鈺测暢鈻鈻佲杺鈻冣杽鈻呪枂鈻団枅鈻夆枈鈻嬧枌鈻嶁枎鈻忊枑鈻戔枓鈻撯枖鈻曗枛鈻椻枠鈻欌枤鈻涒枩鈻濃枮鈻熲攲鈹嶁攷鈹忊攼鈹戔敀鈹撯敂鈹曗敄鈹椻敇鈹欌敋鈹涒暛鈺暞鈺扳敿鈹解斁鈹库晙鈺佲晜鈺冣晞鈺呪晢鈺団晥鈺夆晩鈺嬧敎鈹濃敒鈹熲敔鈹♀敘鈹b敜鈹モ敠鈹р敤鈹┾敧鈹敩鈹敭鈹敯鈹扁敳鈹斥敶鈹碘敹鈹封敻鈹光敽鈹烩晵鈺曗晿鈺涒晸鈺栤暀鈺溾晹鈺椻暁鈺濃暉鈺⑩暏鈺'
    prerendered = 9
    s = <fast_data_types.Screen object at 0x564484aad320>
    self = <kitty_tests.fonts.Rendering testMethod=test_box_drawing>
AssertionError: 178 != 179

I'm not sure what to do here..

@kovidgoyal kovidgoyal merged commit 2b39626 into kovidgoyal:master Oct 18, 2019
@bew bew deleted the fix-triangle-symbol-scaling branch October 19, 2019 16:09
display_bitmap(rgb_data, width, height)
finally:
set_send_sprite_to_gpu(None)
with setup_for_testing(family, sz) as (_, width, height):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an advantage in using

with setup_for_testing(family, sz) as (_, width, height):

instead of just

(_, width, height) = setup_for_testing(family, sz)

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, setup_for_testing is actually a class now, not a function (and the previous code hasn't been updated when the change happened, and simply didn't work):

class setup_for_testing:

This syntax with the with ... as ... : and an indented block will do some operation before entering the block (the __enter__ method of the returned obj), then do some more thing at the end of the block (the __exit__ method).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation.

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.

Dynamic font scaling (with font size change keys) isn't perfect
3 participants