Add sextant glyph support to terminal rendering - #51072
Conversation
|
I have verified the changes fix the broken QR code. Steps to verify fix
|
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Please fix this thank you! |
- Introduce SextantGlyph type and painting logic - Extend layout_grid to return sextant_glyphs - Propagate sextant_glyphs through call sites - Implement sextant_char_to_packed and bit-reversal helpers - Render sextant glyphs after text runs in TerminalOutput - Add tests for sextant packing and decorative range boundaries
38c9ae7 to
8922ab7
Compare
|
I believe I understood the changes @/benbrandt made in #57483 to now use a neutral terminal backend and updated the addition of sextant glyphs for Expo SDK 55+ QR codes correctly. Just wanted to mention it for context. |
- Rename SextantGlyph to BlockElementLayoutRect - Update layout to use block element rects - Add collect_block_element_regions and block_element_regions_to_rects - Update tests for merging adjacent full blocks and mixed regions
|
I have updated the PR description to reflect the most recent changes I've made. The first fix left gaps in the seams of the QR code. If this is not the direction the Zed team wants to go, I am happy to close this PR and go a different direction. |
|
@SomeoneToIgnore Thanks for checking. You are right that we no longer see the issue reported in #50158 because Also, it seems this PR fixes other issues with glyphs in the integrated terminal like in #58830 where some fonts render gaps in TUIs like Opencode.
Based on #17904 and #27559, the Zed team seems to prefer these are fixed with "terminal": {
"line_height": {
"custom": 1.3
},
"font_family": "PT Mono"
}If that is the preference and you feel Examples
|
|
In that case, I found it quite odd that we did not support a better range of symbols — pushed that into the PR along with the drive-by fix for the quadratic Can you check that my changes make sense? |
|
Thank you for the changes. It took me a bit to wrap my head around them, but now I think it's much easier to understand how the characters end up mapping to the terminal subcells. I think the added symbols are good and avoiding unnecessary calls to I don't think there is anything I want to add to this PR. I found some other problems with glyphs and powerlines as mentioned #51310, but I think that would be a separate PR if the team decides to implement the suggestions. It does look like this PR addresses some of the initial requests in the feature request, though. I have updated the PR description to reflect the new changes. I asked if the author of #56342 agrees this fixes their main concern or if they were requesting every Unicode in that range be handled. Unless you need anything else, I am happy for you to merge this. |
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
Great, thank you a lot for confirming!
The Zed integrated terminal shows a broken QR code for apps with Expo SDK 55. This PR custom-paints the terminal block and sextant glyphs used for QR codes rather than relying on font glyphs. | SDK 55 Before | SDK 55 After | SDK 55 in Ghostty | | --- | --- | --- | | <img width="581" height="536" alt="Zed Nightly" src="https://github.com/user-attachments/assets/11054401-e947-4628-918a-0088042ccbf9" /> | <img width="563" height="535" alt="Zed dev build" src="https://github.com/user-attachments/assets/22e0bed3-9927-436d-8e0b-1ad0839f7bee" /> | <img width="576" height="502" alt="Ghostty" src="https://github.com/user-attachments/assets/c5f544d8-43e5-4dad-8bcd-8a36c401e9a7" /> | ### Other notes - Expo recently merged expo/expo#46148, which causes Expo SDK 56 to fall back to the old QR code behavior when used in Zed. Once this PR merges, Expo can remove that fallback and use the compact QR code path in Zed again for Expo SDK 56+. | Current Fallback QR Code for SDK 54 and 56 | Fallback with this PR | | -- | -- | | <img width="744" height="684" alt="SCR-20260622-jwco" src="https://github.com/user-attachments/assets/414896aa-52bc-4de7-a6b9-59cd5af08de0" /> | <img width="616" height="752" alt="SCR-20260622-kfjh" src="https://github.com/user-attachments/assets/accbef7a-3365-480f-add3-08910141afa5" /> | This PR now also fixes TUIs in the integrated terminal for users using some fonts. | OpenCode w/ Monaco before change | OpenCode w/ Monaco after change | | --- | --- | | <img width="1512" height="982" alt="image" src="https://github.com/user-attachments/assets/a40f01a6-adf9-4e8b-909a-be53945eb813" /> | <img width="1512" height="982" alt="image" src="https://github.com/user-attachments/assets/b40bb22c-ef7e-466a-9bcf-46eb951087cd" /> | Finally, shade characters are now handled in the integrated terminal. | btop with tty graph before change | btop with tty graph after change | | --- | --- | | <img width="1516" height="863" alt="SCR-20260724-irpr-2" src="https://github.com/user-attachments/assets/afb77f01-6d3d-428a-b1be-7abb3d0e3d4d" /> | <img width="1516" height="876" alt="SCR-20260724-ises-2" src="https://github.com/user-attachments/assets/42262858-70da-4ba2-a5e6-96243fcfaadd" /> | #### This PR - Adds custom painting for terminal bitmap glyphs used by QR renderers: `█`, `▀`, `▄`, `▌`, `▐`, and U+1FB00..U+1FB3B sextants - Adds tests for handling sextants, blocks/half-blocks, and adjacent block merging Closes zed-industries#50158 Closes zed-industries#58830 Before you mark this PR as ready for review, make sure that you have: - [X] Added a solid test coverage and/or screenshots from doing manual testing - [X] Done a self-review taking into account security and performance aspects - [X] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - Fixed QR code rendering for Expo SDK 55 apps - Fixed rendering TUIs in the integrated terminal for some users --------- Co-authored-by: Kirill Bulatov <kirill@zed.dev>
The Zed integrated terminal shows a broken QR code for apps with Expo SDK 55. This PR custom-paints the terminal block and sextant glyphs used for QR codes rather than relying on font glyphs. | SDK 55 Before | SDK 55 After | SDK 55 in Ghostty | | --- | --- | --- | | <img width="581" height="536" alt="Zed Nightly" src="https://github.com/user-attachments/assets/11054401-e947-4628-918a-0088042ccbf9" /> | <img width="563" height="535" alt="Zed dev build" src="https://github.com/user-attachments/assets/22e0bed3-9927-436d-8e0b-1ad0839f7bee" /> | <img width="576" height="502" alt="Ghostty" src="https://github.com/user-attachments/assets/c5f544d8-43e5-4dad-8bcd-8a36c401e9a7" /> | ### Other notes - Expo recently merged expo/expo#46148, which causes Expo SDK 56 to fall back to the old QR code behavior when used in Zed. Once this PR merges, Expo can remove that fallback and use the compact QR code path in Zed again for Expo SDK 56+. | Current Fallback QR Code for SDK 54 and 56 | Fallback with this PR | | -- | -- | | <img width="744" height="684" alt="SCR-20260622-jwco" src="https://github.com/user-attachments/assets/414896aa-52bc-4de7-a6b9-59cd5af08de0" /> | <img width="616" height="752" alt="SCR-20260622-kfjh" src="https://github.com/user-attachments/assets/accbef7a-3365-480f-add3-08910141afa5" /> | This PR now also fixes TUIs in the integrated terminal for users using some fonts. | OpenCode w/ Monaco before change | OpenCode w/ Monaco after change | | --- | --- | | <img width="1512" height="982" alt="image" src="https://github.com/user-attachments/assets/a40f01a6-adf9-4e8b-909a-be53945eb813" /> | <img width="1512" height="982" alt="image" src="https://github.com/user-attachments/assets/b40bb22c-ef7e-466a-9bcf-46eb951087cd" /> | Finally, shade characters are now handled in the integrated terminal. | btop with tty graph before change | btop with tty graph after change | | --- | --- | | <img width="1516" height="863" alt="SCR-20260724-irpr-2" src="https://github.com/user-attachments/assets/afb77f01-6d3d-428a-b1be-7abb3d0e3d4d" /> | <img width="1516" height="876" alt="SCR-20260724-ises-2" src="https://github.com/user-attachments/assets/42262858-70da-4ba2-a5e6-96243fcfaadd" /> | #### This PR - Adds custom painting for terminal bitmap glyphs used by QR renderers: `█`, `▀`, `▄`, `▌`, `▐`, and U+1FB00..U+1FB3B sextants - Adds tests for handling sextants, blocks/half-blocks, and adjacent block merging Closes zed-industries#50158 Closes zed-industries#58830 Before you mark this PR as ready for review, make sure that you have: - [X] Added a solid test coverage and/or screenshots from doing manual testing - [X] Done a self-review taking into account security and performance aspects - [X] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - Fixed QR code rendering for Expo SDK 55 apps - Fixed rendering TUIs in the integrated terminal for some users --------- Co-authored-by: Kirill Bulatov <kirill@zed.dev>
# Why Zed's integrated terminal had malformed QR codes with some fonts that became more obvious with Expo's new sextant based QR code rendering. The sextant QR code rendering was disabled in #46148 but now Zed handles the sextant range needed for the new QR code rendering with the changes from zed-industries/zed#51072. This PR removes the check that gated against Zed rendering the sextant QR code. # How I removed the changes made by @/kitten in https://github.com/expo/expo/pull/46148/changes. # Test Plan Added the following to my `.zshrc` as the docs say ``` # expo custom build alias nexpo="/path/to/project/expo/packages/@expo/cli/bin/cli.js" alias expo-inspect="node --inspect /path/to/project/expo/packages/@expo/cli/bin/cli.js" ``` 1. Build the custom cli based on the instructions in the README.md 2. Open an Expo SDK 55+ app in Zed with Monaco font for the terminal (a font that use to show the regression) 3. Open the Zed integrated terminal 4. Run `npx expo start` 5. Observe the current release falls back to the non-sextant QR code rendering 6. Run `nexpo start` 7. Observe the QR code is now using sextant rendering | Zed Integrated Terminal w/ fallback QR code | Zed Integrated Terminal w/ sextant QR code | | --- | --- | | <img width="1512" height="982" alt="SCR-20260731-kiyk-2" src="https://github.com/user-attachments/assets/b297c93c-24f2-43cd-b5e8-7f36ca19bf88" /> | <img width="1512" height="982" alt="SCR-20260731-kipj-2" src="https://github.com/user-attachments/assets/f913a7e9-d9e6-4b8f-bc76-6f7516d7ce90" /> | # Checklist <!-- Please check the appropriate items below if they apply to your diff. --> - [x] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [x] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [x] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) --------- Co-authored-by: Phil Pluckthun <phil@kitten.sh>
Zed's integrated terminal had malformed QR codes with some fonts that became more obvious with Expo's new sextant based QR code rendering. The sextant QR code rendering was disabled in #46148 but now Zed handles the sextant range needed for the new QR code rendering with the changes from zed-industries/zed#51072. This PR removes the check that gated against Zed rendering the sextant QR code. I removed the changes made by @/kitten in https://github.com/expo/expo/pull/46148/changes. Added the following to my `.zshrc` as the docs say ``` alias nexpo="/path/to/project/expo/packages/@expo/cli/bin/cli.js" alias expo-inspect="node --inspect /path/to/project/expo/packages/@expo/cli/bin/cli.js" ``` 1. Build the custom cli based on the instructions in the README.md 2. Open an Expo SDK 55+ app in Zed with Monaco font for the terminal (a font that use to show the regression) 3. Open the Zed integrated terminal 4. Run `npx expo start` 5. Observe the current release falls back to the non-sextant QR code rendering 6. Run `nexpo start` 7. Observe the QR code is now using sextant rendering | Zed Integrated Terminal w/ fallback QR code | Zed Integrated Terminal w/ sextant QR code | | --- | --- | | <img width="1512" height="982" alt="SCR-20260731-kiyk-2" src="https://github.com/user-attachments/assets/b297c93c-24f2-43cd-b5e8-7f36ca19bf88" /> | <img width="1512" height="982" alt="SCR-20260731-kipj-2" src="https://github.com/user-attachments/assets/f913a7e9-d9e6-4b8f-bc76-6f7516d7ce90" /> | <!-- Please check the appropriate items below if they apply to your diff. --> - [x] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [x] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [x] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) --------- Co-authored-by: Phil Pluckthun <phil@kitten.sh>
Zed's integrated terminal had malformed QR codes with some fonts that became more obvious with Expo's new sextant based QR code rendering. The sextant QR code rendering was disabled in #46148 but now Zed handles the sextant range needed for the new QR code rendering with the changes from zed-industries/zed#51072. This PR removes the check that gated against Zed rendering the sextant QR code. I removed the changes made by @/kitten in https://github.com/expo/expo/pull/46148/changes. Added the following to my `.zshrc` as the docs say ``` alias nexpo="/path/to/project/expo/packages/@expo/cli/bin/cli.js" alias expo-inspect="node --inspect /path/to/project/expo/packages/@expo/cli/bin/cli.js" ``` 1. Build the custom cli based on the instructions in the README.md 2. Open an Expo SDK 55+ app in Zed with Monaco font for the terminal (a font that use to show the regression) 3. Open the Zed integrated terminal 4. Run `npx expo start` 5. Observe the current release falls back to the non-sextant QR code rendering 6. Run `nexpo start` 7. Observe the QR code is now using sextant rendering | Zed Integrated Terminal w/ fallback QR code | Zed Integrated Terminal w/ sextant QR code | | --- | --- | | <img width="1512" height="982" alt="SCR-20260731-kiyk-2" src="https://github.com/user-attachments/assets/b297c93c-24f2-43cd-b5e8-7f36ca19bf88" /> | <img width="1512" height="982" alt="SCR-20260731-kipj-2" src="https://github.com/user-attachments/assets/f913a7e9-d9e6-4b8f-bc76-6f7516d7ce90" /> | <!-- Please check the appropriate items below if they apply to your diff. --> - [x] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [x] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [x] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) --------- Co-authored-by: Phil Pluckthun <phil@kitten.sh>
# Why Zed's integrated terminal had malformed QR codes with some fonts that became more obvious with Expo's new sextant based QR code rendering. The sextant QR code rendering was disabled in expo#46148 but now Zed handles the sextant range needed for the new QR code rendering with the changes from zed-industries/zed#51072. This PR removes the check that gated against Zed rendering the sextant QR code. # How I removed the changes made by @/kitten in https://github.com/expo/expo/pull/46148/changes. # Test Plan Added the following to my `.zshrc` as the docs say ``` # expo custom build alias nexpo="/path/to/project/expo/packages/@expo/cli/bin/cli.js" alias expo-inspect="node --inspect /path/to/project/expo/packages/@expo/cli/bin/cli.js" ``` 1. Build the custom cli based on the instructions in the README.md 2. Open an Expo SDK 55+ app in Zed with Monaco font for the terminal (a font that use to show the regression) 3. Open the Zed integrated terminal 4. Run `npx expo start` 5. Observe the current release falls back to the non-sextant QR code rendering 6. Run `nexpo start` 7. Observe the QR code is now using sextant rendering | Zed Integrated Terminal w/ fallback QR code | Zed Integrated Terminal w/ sextant QR code | | --- | --- | | <img width="1512" height="982" alt="SCR-20260731-kiyk-2" src="https://github.com/user-attachments/assets/b297c93c-24f2-43cd-b5e8-7f36ca19bf88" /> | <img width="1512" height="982" alt="SCR-20260731-kipj-2" src="https://github.com/user-attachments/assets/f913a7e9-d9e6-4b8f-bc76-6f7516d7ce90" /> | # Checklist <!-- Please check the appropriate items below if they apply to your diff. --> - [x] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [x] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [x] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) --------- Co-authored-by: Phil Pluckthun <phil@kitten.sh>
The Zed integrated terminal shows a broken QR code for apps with Expo SDK 55. This PR custom-paints the terminal block and sextant glyphs used for QR codes rather than relying on font glyphs. | SDK 55 Before | SDK 55 After | SDK 55 in Ghostty | | --- | --- | --- | | <img width="581" height="536" alt="Zed Nightly" src="https://github.com/user-attachments/assets/11054401-e947-4628-918a-0088042ccbf9" /> | <img width="563" height="535" alt="Zed dev build" src="https://github.com/user-attachments/assets/22e0bed3-9927-436d-8e0b-1ad0839f7bee" /> | <img width="576" height="502" alt="Ghostty" src="https://github.com/user-attachments/assets/c5f544d8-43e5-4dad-8bcd-8a36c401e9a7" /> | ### Other notes - Expo recently merged expo/expo#46148, which causes Expo SDK 56 to fall back to the old QR code behavior when used in Zed. Once this PR merges, Expo can remove that fallback and use the compact QR code path in Zed again for Expo SDK 56+. | Current Fallback QR Code for SDK 54 and 56 | Fallback with this PR | | -- | -- | | <img width="744" height="684" alt="SCR-20260622-jwco" src="https://github.com/user-attachments/assets/414896aa-52bc-4de7-a6b9-59cd5af08de0" /> | <img width="616" height="752" alt="SCR-20260622-kfjh" src="https://github.com/user-attachments/assets/accbef7a-3365-480f-add3-08910141afa5" /> | This PR now also fixes TUIs in the integrated terminal for users using some fonts. | OpenCode w/ Monaco before change | OpenCode w/ Monaco after change | | --- | --- | | <img width="1512" height="982" alt="image" src="https://github.com/user-attachments/assets/a40f01a6-adf9-4e8b-909a-be53945eb813" /> | <img width="1512" height="982" alt="image" src="https://github.com/user-attachments/assets/b40bb22c-ef7e-466a-9bcf-46eb951087cd" /> | Finally, shade characters are now handled in the integrated terminal. | btop with tty graph before change | btop with tty graph after change | | --- | --- | | <img width="1516" height="863" alt="SCR-20260724-irpr-2" src="https://github.com/user-attachments/assets/afb77f01-6d3d-428a-b1be-7abb3d0e3d4d" /> | <img width="1516" height="876" alt="SCR-20260724-ises-2" src="https://github.com/user-attachments/assets/42262858-70da-4ba2-a5e6-96243fcfaadd" /> | #### This PR - Adds custom painting for terminal bitmap glyphs used by QR renderers: `█`, `▀`, `▄`, `▌`, `▐`, and U+1FB00..U+1FB3B sextants - Adds tests for handling sextants, blocks/half-blocks, and adjacent block merging Closes zed-industries#50158 Closes zed-industries#58830 Before you mark this PR as ready for review, make sure that you have: - [X] Added a solid test coverage and/or screenshots from doing manual testing - [X] Done a self-review taking into account security and performance aspects - [X] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - Fixed QR code rendering for Expo SDK 55 apps - Fixed rendering TUIs in the integrated terminal for some users --------- Co-authored-by: Kirill Bulatov <kirill@zed.dev>











The Zed integrated terminal shows a broken QR code for apps with Expo SDK 55. This PR custom-paints the terminal block and sextant glyphs used for QR codes rather than relying on font glyphs.
Other notes
This PR now also fixes TUIs in the integrated terminal for users using some fonts.
Finally, shade characters are now handled in the integrated terminal.
This PR
█,▀,▄,▌,▐, and U+1FB00..U+1FB3B sextantsCloses #50158
Closes #58830
Before you mark this PR as ready for review, make sure that you have:
Release Notes: