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

[Bug]: CanvasText class used by <m-label> has some shortcomings and undesirable behavior. #104

Open
TheCodeTherapy opened this issue Aug 16, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@TheCodeTherapy
Copy link
Contributor

Describe the bug

There are some details we should consider about the CanvasText class used to draw text on behalf of the <m-label> tag:

the printAtWordWrap function:

As it is at the moment, the CanvasText function uses the printAtWordWrap method to draw the text with line-wrapping when width and height attributes were provided for the <m-label> tag.

Because printAtWordWrap immediately draws the text to the context, we cannot use line count and line height measurements performed inside this method to identify if the text will fit the provided height attribute, and do something about it in case it won't.

One alternative would be to transform this method into a computeLineWrapping method that would return the lines to be drawn and the height necessary for all the lines to fit the <m-label> with the provided font-size, width, and content attributes.

With that information, we would be able to choose between:

  • expanding the <m-label> mesh height to fit all the text to be drawn considering the provided padding, font-size, and content attributes (which would have the undesirable side-effect of having a non-deterministic height for the <m-label> tag, and would also require some refactor of the Label element), or;

  • creating the canvas with the necessary dimensions to draw all the text and "squish it" into the provided <m-label> mesh's height (which would have the undesirable side-effect of being distorted/squished, but already presumably better than missing text that was supposed to be drawn).

The padding usage:

We only have one padding attribute instead of separating it into horizontalPadding and verticalPadding. That reduces usage flexibility, but it also comes with some bugs.

Right now, if the user chooses a center alignment for the <m-label>, and provides a padding value greater than zero, the padding is wrongly applied to the left of the text, preventing it from being centered, and if a right alignment is chosen, the text usually gets rendered with a chunk cut out of the label given wrong padding influence.

Usage of the CanvasText class in other packages:

As I'm currently implementing a way to draw textures with text (for text chat and identification purposes) above the character on the 3d-web-client-core package of the 3d-web-experience repository, and such package already has mml-web as a dependency, we could benefit from exposing the CanvasText class on mml-web's index.ts so it can be imported and used for such purpose.

That would avoid duplicated code and improve maintainability, as the improvements delivered to this class on the mml-web package would also benefit any other package using it.

Expected behavior

The <m-label> class should offer more control over padding, and proper alignment in combination with the padding attribute(s). It should also prevent missing text when the font-size attribute combined with the content and height would cause the text to overflow the mesh's available space available to draw.

How to reproduce the bug

Using the <m-label> tag with the combination of attributes listed in the bug description can illustrate the described undesirable behavior.

Link to the code that reproduces this bug

No response

Packages

mml-web

Package versions

0.4.0

Extra details

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 🔖 Ready
Development

No branches or pull requests

1 participant