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

Improve text renderer font default #442

Merged

Conversation

frank-weindel
Copy link
Contributor

@frank-weindel frank-weindel commented Jan 18, 2023

There is an inconsistency in the handling of default fonts depending on if you invoke the TextTexture renderer and call _calculateRenderInfo() directly (as I guess might be done as a pre-render measurement) or if you invoke the TextTexture normally in an attached Lightning Element via the 'text' property.

This Lightning playground example that @michielvandergeest sent me shows this inconsistency clearly. Press the 1, 2, 3, 4 keys and notice that as the text at the top changes, the measurements for pre and post render below are not the same, even though the same exact properties (both omitting fontFace) are passed in for both of those cases.

If the fontFace property is not explicitly set, the default font actually used for text rendering differs in the two cases:

  • "Pre-renderer": A literal null is used which is automatically stringified as the font name "null", which is an invalid font name. The browser falls back to using a default font of its choosing.
  • "Post-render": The value of the defaultFontFace Stage setting is used. Which by default is "sans-serif".

The defaultFontFace setting is pulled out only at the beginning of the _getSourceLoader() method in the TextTexture. This code, as I understand it, is not hit when using the "Pre-render" method.

The solution is the move the resolution code that converts a null into the Stage defined defaultFontFace into the method that builds the CSS font string that is passed to the Canvas2D context. This way no matter how the TextTexture is invoked/rendered it uses a consistent default.

To-Do:

  • Implement
  • Test

@uguraslan uguraslan mentioned this pull request Feb 7, 2023
1 task
@uguraslan uguraslan merged commit 7227340 into rdkcentral:dev Feb 16, 2023
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