Skip to content

Reduce instances of font fallback dialog - #9734

Merged
9 commits merged into
mainfrom
dev/miniksa/font_popup
Apr 8, 2021
Merged

9 commits merged into
mainfrom
dev/miniksa/font_popup

Conversation

@miniksa

@miniksa Michael Niksa (miniksa) commented Apr 6, 2021 •

Copy link
Copy Markdown
Contributor

Reduce instances of font fallback dialog through package font loading,
basic name trimming, and revised fallback test

  • Adjusts the font dialog to only show when we attempt last-chance
    resolution from our hardcoded list of font names with a flag instead
    of with a string comparison by name

  • Adds a resolution step to trim the font name by word from the end and
    retry to attempt to resolve a proper font that just has a weight
    suffix

  • Adds a second font collection to font loading that will attempt to
    locate all TTF files sitting next to our binary, like in our package

  • Wrote my font preference in the JSON as Cascadia Code Heavy and
    watched it quietly resolve to just Cascadia Code without the dialog.

  • Put a font that isn't registered with the system into the layout
    directory for the package, set it as my desired font in Terminal, and
    watched it load just fine.

  • Try a font name with different casing and see if dialog doesn't
    pop anymore

  • Try a font with different (localized) names like MS ゴシック and
    see if dialog doesn't pop anymore

  • Check Win7 with WPF target

Closes #9375

@ghost ghost added Area-Fonts Related to the font Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Apr 6, 2021
@github-actions

This comment has been minimized.

… variables to be nearby and not package related. add some doc comments.
Comment thread src/renderer/dx/DxFontRenderData.h Resolved
@zadjii-msft

Copy link
Copy Markdown
Member

Thanks for making it a magic static - that's way better

Comment thread src/renderer/dx/DxFontRenderData.cpp Resolved
Comment thread src/renderer/dx/DxFontRenderData.cpp Resolved
Comment thread src/renderer/dx/DxFontRenderData.cpp Resolved
Comment thread src/renderer/dx/DxFontRenderData.cpp Resolved
Comment thread src/renderer/dx/DxFontRenderData.cpp Resolved
Comment thread src/renderer/dx/DxFontRenderData.cpp Resolved
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 7, 2021
@DHowett

Copy link
Copy Markdown
Member

"let me do my work," he says!

Windows 7 is now that work

@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 7, 2021
…p if we can't use the quick-convenience methods to create the font set from the files on disk.
@miniksa

Copy link
Copy Markdown
Contributor Author

"let me do my work," he says!

Windows 7 is now that work

89e9870 (#9734) should cover it but I'll probably ask you to help triple check.

@skyline75489

Copy link
Copy Markdown
Collaborator

Maybe this isn't a good time...but this will conflict with #9201. I think we might want to merge #9201 first which will simplify this PR, too.

@DHowett

Copy link
Copy Markdown
Member

Chester Liu (@skyline75489) I appreciate that, and thanks for your work in #9201! I'm inclined to merge this one first just so that we can be ready for the 1.7 release (next week; hit me up on Teams for why this is important :)) and then get reviews in 9201. I know that it complicates your life because you have some merging to do 😦 and I am sorry about that.

@DHowett Dustin L. Howett (DHowett) changed the title Reduce instances of font fallback dialog through package font loading, basic name trimming, and revised fallback test Reduce instances of font fallback dialog Apr 8, 2021
@DHowett

Copy link
Copy Markdown
Member

I tested MS ゴシック using WPF; it did not trigger the fallback code path.

@DHowett Dustin L. Howett (DHowett) added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 8, 2021
@ghost

Copy link
Copy Markdown

Hello Dustin L. Howett (@DHowett)!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I forgot that I didn't sign off.

@ghost
ghost merged commit 7f5a19b into main Apr 8, 2021
@ghost
ghost deleted the dev/miniksa/font_popup branch April 8, 2021 17:49
Dustin L. Howett (DHowett) pushed a commit that referenced this pull request Apr 12, 2021
Reduce instances of font fallback dialog through package font loading,
basic name trimming, and revised fallback test

- Adjusts the font dialog to only show when we attempt last-chance
  resolution from our hardcoded list of font names with a flag instead
  of with a string comparison by name
- Adds a resolution step to trim the font name by word from the end and
  retry to attempt to resolve a proper font that just has a weight
  suffix
- Adds a second font collection to font loading that will attempt to
  locate all TTF files sitting next to our binary, like in our package

- [x] Wrote my font preference in the JSON as `Cascadia Code Heavy` and
  watched it quietly resolve to just `Cascadia Code` without the dialog.
- [x] Put a font that isn't registered with the system into the layout
  directory for the package, set it as my desired font in Terminal, and
  watched it load just fine.
- [x] Try a font name with different casing and see if dialog doesn't
  pop anymore
- [x] Try a font with different (localized) names like MS ゴシック and
  see if dialog doesn't pop anymore
- [x] Check Win7 with WPF target

Closes #9375

(cherry picked from commit 7f5a19b)
@ghost

Copy link
Copy Markdown

🎉Windows Terminal v1.7.1033.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost

Copy link
Copy Markdown

🎉Windows Terminal Preview v1.8.1032.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Fonts Related to the font AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to find the selected font "Cascadia Mono".

4 participants