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

[no squash] Return texture filter settings to previous state #14016

Merged
merged 2 commits into from Nov 29, 2023

Conversation

sfan5
Copy link
Member

@sfan5 sfan5 commented Nov 19, 2023

original commit for reference: 72ef908

texture_clean_transparent was not returned since it's automatically enabled when needed and it's useless in other cases

@sfan5
Copy link
Member Author

sfan5 commented Nov 19, 2023

@Warr1024 please test

@Warr1024
Copy link
Contributor

Warr1024 commented Nov 19, 2023

Looks good to me.


CLARIFICATION: I have tested it and confirmed that it fixes the problem for my machine/config.

@grorp grorp self-requested a review November 20, 2023 07:09
src/client/tile.cpp Outdated Show resolved Hide resolved
@sfan5 sfan5 added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Nov 20, 2023
It was determined that this fixes scaling artifacts that can happen with bilinear,
trilinear or anisotropic filtering alone.
Since the previous commit did not bring back the relevant setting, we fix this
shortcoming by just enabling it in all cases where it is known to help.
@sfan5 sfan5 removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Nov 22, 2023
@sfan5 sfan5 changed the title Return texture filter settings to previous state [no squash] Return texture filter settings to previous state Nov 22, 2023
@sfan5 sfan5 requested a review from grorp November 22, 2023 22:25
Copy link
Member

@grorp grorp left a comment

Choose a reason for hiding this comment

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

LGTM

If we're sure enough that it won't accidentally cause more issues (I don't see how myself), I can add it as a separate commit to the PR.

Things that could go wrong:

  • There is a yet undiscovered problem with imageCleanTransparent, be it a conceptual or an implementation problem, and users can no longer disable imageCleanTransparent.

  • (Someone complains that the "the comic-like outlines" are gone.)

@SmallJoker SmallJoker added this to the 5.8.0 milestone Nov 26, 2023
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Code checked. width/height scaling looks correct. Too high texture_min_size values seem predestined for issues. Limiting that to a saner value (e.g. 1024) might make sense in the future.

LGTM, although similar to Desour (see linked issue) I somehow could not reproduce this issue.

EDIT: Clarified with Warr1024 that his "LGTM" response is based on a check in-game and not code review.

@SmallJoker SmallJoker merged commit d6a8b54 into minetest:master Nov 29, 2023
13 checks passed
@sfan5 sfan5 deleted the yxcvb branch November 29, 2023 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants