Skip to content

Fix: UI graphics bug [MTT-6786] #848

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

Merged
merged 4 commits into from
Jul 6, 2023
Merged

Conversation

jilfranco-unity
Copy link
Contributor

@jilfranco-unity jilfranco-unity commented Jul 5, 2023

Description

A workaround fix for the UI graphics bug that I tracked down with the help of @fernando-cortez

The bug is due to a regression caused by this commit in the unity trunk. In short, the issue is caused by a math issue when taking MipMap textures sizes into account when calculating the texel size for the UVs of the UI sprite meshes. The workaround fix is to make sure one of the quality levels allows for full resolution MipMaps (since anything less than full res causes the meshes to generate with the wrong UV scaling). In our case, I set the High quality setting on mobile to allow for full resolution textures, increased from half resolution.

There is a small performance/memory hit to be had by allowing full res textures on mobile, but it's a small price to pay to fix the visual bug and unblock the release. We'll just revert this change once the regression is fixed in the future. :)

Issue Number(s)

MTT-6786

Contribution checklist

  • [n/a] Tests have been added for boss room and/or utilities pack
  • Release notes have been added to the project changelog file and/or package changelog file
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • JIRA ticket ID is in the PR title or at least one commit message
  • Include the ticket ID number within the body message of the PR to create a hyperlink
  • [n/a] An Index entry has been added in readme.md if applicable

@jilfranco-unity jilfranco-unity added 0-URGENT Blocker for a release and needs to be merged ASAP 2-Easy This PR is trivial and can be reviewed quickly 1-Needs Review PR needs attention from the assignee and reviewers labels Jul 5, 2023
@SamuelBellomo SamuelBellomo requested review from fernando-cortez and removed request for SamuelBellomo July 5, 2023 19:49
@fernando-cortez fernando-cortez self-assigned this Jul 5, 2023
@jilfranco-unity jilfranco-unity requested a review from a team as a code owner July 5, 2023 20:18
@RikuTheFuffs
Copy link

Hey Jil, is this the PR/commit Fernando mentioned in this thread? In this case, should we still merge this PR, or wait until Fernando runs his experiments?

@jilfranco-unity jilfranco-unity merged commit 18343f6 into develop Jul 6, 2023
@jilfranco-unity jilfranco-unity deleted the fix/ui-graphics-bug branch July 6, 2023 17:58
@jilfranco-unity
Copy link
Contributor Author

jilfranco-unity commented Jul 6, 2023

Hey Jil, is this the PR/commit Fernando mentioned in this thread? In this case, should we still merge this PR, or wait until Fernando runs his experiments?

This was the PR that was discussed in that thread and during our meeting this morning, yes :) @RikuTheFuffs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0-URGENT Blocker for a release and needs to be merged ASAP 1-Needs Review PR needs attention from the assignee and reviewers 2-Easy This PR is trivial and can be reviewed quickly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants