Guekka / bethutil

Common utilities for working with bethesda assets (dds, nif, bsa, hkx...)
https://guekka.github.io/bethutil/
Mozilla Public License 2.0
4 stars 2 forks source link

fix(tex): fix formats and their selection based on alpha #49

Closed mklokocka closed 4 months ago

mklokocka commented 4 months ago

The algorithm for selection which format to output for texture had two issues:

1) Bad compression formats set for older games. BC5 is not supported at all. BC1 should be used in situations where the input has no alpha. Using BC5 is possible, but increases the file size for no reason. Another reason is that introducing alpha to a previously RGB image affects and breaks i.e. normal maps, for which alpha governs the specularity.

2) The selection of the correct format to used based on alpha didn't work - it checked the input format, not whether it has alpha or not (the alpha is opaque).

This PR solves both of those issues.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.36%. Comparing base (2835714) to head (5d2ce06).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/Guekka/bethutil/pull/49/graphs/tree.svg?width=650&height=150&src=pr&token=llth93S2Em&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar)](https://app.codecov.io/gh/Guekka/bethutil/pull/49?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar) ```diff @@ Coverage Diff @@ ## main #49 +/- ## ========================================== + Coverage 80.28% 80.36% +0.08% ========================================== Files 38 38 Lines 1684 1691 +7 ========================================== + Hits 1352 1359 +7 Misses 332 332 ``` | [Files](https://app.codecov.io/gh/Guekka/bethutil/pull/49?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar) | Coverage Δ | | |---|---|---| | [src/tex/formats.cpp](https://app.codecov.io/gh/Guekka/bethutil/pull/49?src=pr&el=tree&filepath=src%2Ftex%2Fformats.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar#diff-c3JjL3RleC9mb3JtYXRzLmNwcA==) | `100.00% <100.00%> (ø)` | | | [src/tex/optimize.cpp](https://app.codecov.io/gh/Guekka/bethutil/pull/49?src=pr&el=tree&filepath=src%2Ftex%2Foptimize.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar#diff-c3JjL3RleC9vcHRpbWl6ZS5jcHA=) | `71.90% <100.00%> (+1.46%)` | :arrow_up: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/Guekka/bethutil/pull/49?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/Guekka/bethutil/pull/49?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar). Last update [2835714...5d2ce06](https://app.codecov.io/gh/Guekka/bethutil/pull/49?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Edgar).
Guekka commented 4 months ago

Also, minor thing but please link your commits to the FNV issue (add #44 at the end)