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

2d canvas font code panics on default system font on HL2 #27166

Closed
jdm opened this issue Jul 3, 2020 · 5 comments
Closed

2d canvas font code panics on default system font on HL2 #27166

jdm opened this issue Jul 3, 2020 · 5 comments
Assignees
Projects

Comments

@jdm
Copy link
Member

@jdm jdm commented Jul 3, 2020

From a log on a user's device:

RUST: ERROR - canvas::canvas_data - error getting font handle for style Font { font_family: FontFamily { families: FontFamilyList([FamilyName(FamilyName { name: Atom('Helvetica' type=dynamic), syntax: Identifiers }), FamilyName(FamilyName { name: Atom('Arial' type=inline), syntax: Identifiers }), Generic(SansSerif)]), is_system_font: false }, font_style: Normal, font_variant_caps: Normal, font_weight: FontWeight(700.0), font_size: FontSize { size: NonNegative(18.0 px), keyword_info: KeywordInfo { kw: None, factor: 1.0, offset: 0.0 px } }, font_stretch: FontStretch(NonNegative(Percentage(1.0))), hash: 7779769241958551862 }: no font found
\00thread 'CanvasThread' panicked at 'error getting font handle for default system font: NotFound', components\canvas\canvas_data.rs:1432:10
@jdm
Copy link
Member Author

@jdm jdm commented Jul 3, 2020

We'll need to make this optional and just ignore text operations when we can't find fonts to avoid panics like this; we should also figure out how to load the default fallback font on UWP as well.

@atouchet atouchet added this to To do in HoloLens Jul 3, 2020
@jdm
Copy link
Member Author

@jdm jdm commented Jul 6, 2020

Using https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Tutorial/Drawing_text, I see the "Hello world" text on desktop macOS and Windows nightlies, and a blank canvas in the UWP nightly. I'm going to try and fix that.

@jdm jdm self-assigned this Jul 6, 2020
@jdm
Copy link
Member Author

@jdm jdm commented Jul 6, 2020

Ok, so the path we follow gets us to https://github.com/servo/font-kit/blob/efb64bcd3fdaf21a46a0edb086a4f157264a54ea/src/source.rs#L146 before we encounter any errors. We have a FontHandle that corresponds to the serif generic font (which is Times New Roman on windows), and the corresponding path we have is C:\WINDOWS\FONTS\TIMES.TTF.

Font::from_handle takes us to https://github.com/servo/font-kit/blob/3f5c183ecfbbba9af388307e495031c6c0b7cf47/src/loader.rs#L69, where we take the path branch and end up in https://github.com/servo/font-kit/blob/3f5c183ecfbbba9af388307e495031c6c0b7cf47/src/loader.rs#L62. At this point, we:

  • open the path without any problems
  • call Loader::from_file with the new opened font file
  • this takes us here, where GetFinalPathNameByHandleW returns 0

It's not really clear to me why we open a file from a path, then try to determine the path to that file handle. I also need to figure out what the winapi says the actual underlying error is at this point, since the docs claim that's a valid function to call from UWP apps.

@jdm
Copy link
Member Author

@jdm jdm commented Jul 6, 2020

Ok, not super surprisingly, the underlying error is Io(Os { code: 5, kind: PermissionDenied, message: "Access is denied." }). It's interesting that we're allowed to open that file path but not not go from the file handle to the path.

@jdm
Copy link
Member Author

@jdm jdm commented Jul 6, 2020

#27184 will make the crash go away. I've submitted servo/font-kit#156 to fix the underlying problem, which will allow the actual text to appear when we update to a version of font-kit that includes that change.

@jdm jdm moved this from To do to In progress in HoloLens Jul 6, 2020
@bors-servo bors-servo closed this in 4a0cd44 Jul 7, 2020
@atouchet atouchet moved this from In progress to Done in HoloLens Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
HoloLens
  
Done
Linked pull requests

Successfully merging a pull request may close this issue.

1 participant
You can’t perform that action at this time.