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

Fix unpacking of msdf markup params #4600

Merged
merged 1 commit into from
Sep 22, 2022
Merged

Conversation

NewboO
Copy link
Contributor

@NewboO NewboO commented Aug 30, 2022

Fixes: #4595

At the moment, unpacking function of text markup parameters isn't the exact inverse of packing, especially on shadow offset.

Simple example of the issue is to keep default settings for shadow (ie. white text with a black shadow at an offset of (0, 0)) and toggling markup without actually using any tag. It gives something like this (slightly visible on black background, obvious on white blackground):
image

This is because an offset of (0, 0) is packed as 127 + 127 * 256 = 32639 but unpacked as ((32639 % 256) / 256 * 2 - 1, (32639 / 256 / 256) * 2 - 1) = (-0.008, -0.004).

This corrected function gives the expected rendering:
image

Color unpacking error is barely visible but comes from the same kind of difference (packed by multiplying by 255, unpacked by dividing by 256).

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@yaustar
Copy link
Contributor

yaustar commented Aug 30, 2022

Is it related to this issue? #4595

@NewboO
Copy link
Contributor Author

NewboO commented Aug 30, 2022

Indeed, forgot to check if this was reported.

@yaustar
Copy link
Contributor

yaustar commented Aug 30, 2022

That's an awesome find, thank you!

@yaustar yaustar added the area: ui UI related issue label Aug 30, 2022
@yaustar yaustar requested a review from a team August 30, 2022 15:57
@yaustar
Copy link
Contributor

yaustar commented Aug 30, 2022

@slimbuck @mvaligursky @GSterbrant Would you be able to check the PR please?

Copy link
Member

@slimbuck slimbuck left a comment

Choose a reason for hiding this comment

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

Approving without testing. Thanks for this contribution!!

@mvaligursky mvaligursky added the release: next minor Ticket marked for the next minor release label Sep 21, 2022
@mvaligursky mvaligursky merged commit 540840f into playcanvas:main Sep 22, 2022
@NewboO NewboO deleted the patch-1 branch September 23, 2022 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ui UI related issue release: next minor Ticket marked for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling markup on a text element adds shadow even when offset is 0, 0
5 participants