Skip to content

-Quality flag: strict threshold comparison plus float-precision loss produce two related, silently incorrect behaviors #351

Description

@noisethanks

This report covers two consequences of a single design pattern in the -Quality pipeline: m_quality > g_HIGHQULITY_THRESHOLD is a strict inequality against a threshold constant (0.7), and the value being compared is silently truncated to float precision somewhere between CLI parsing and the comparison itself.

  1. -Quality 0.7 doesn't reach the high-quality branch

g_HIGHQULITY_THRESHOLD is 0.7, and the comparison is >, not >=. A user who explicitly passes -Quality 0.7, the exact threshold, reasonably expects to land on the high-quality path. They don't. 0.7 > 0.7 is false regardless of precision.

  1. Float truncation means there's no reliable way to guarantee landing above 0.7 either

Separately, and more seriously: the value that actually reaches this comparison has been silently truncated to float precision, so even values a user deliberately chooses to sit above 0.7 aren't safe.

Root cause:

applications/_plugins/common/cmdline.cpp:411 parses -Quality into a float, assigned to CompressOptions.fquality but declared double (see textureio.h:73).
compress.cpp (lines 223, 249, 256, 278, 584) casts back down via (CODECFLOAT)options->fquality, where CODECFLOAT is typedef float (codec.h:41), before calling SetParameter.
That truncated value is what lands in m_Quality (codec_bc7.cpp:170).

We measured the exact crossover empirically on build_cli_off (stock CPU BC7 codec, no modifications):

Q Wall time Result
0.7000001 2.80s Exhaustive (correct)
0.70000001 1.92s Fast path (incorrect)
0.700000001 1.86s Fast path

float(0.7) ≈ 0.699999988..., below true 0.7. Values close enough to 0.7 round to that same float and silently fail the > comparison, regardless of how clearly the user's intent was to exceed the threshold.

This isn't limited to hand-typed edge values. Any pipeline that computes -Quality programmatically (LOD-based tiers, GUI sliders, config-driven builds) can produce values that silently fall on the wrong side of a threshold with no indication anything went wrong and there's no "safe margin" a caller can add to reliably guarantee the intended branch.

Suggested fix:

For (1): consider >= if the intent is that 0.7 itself should qualify, or document the strict-inequality behavior explicitly if not.
For (2): the truncation happens across several call sites (compress.cpp casts to CODECFLOAT in at least 5 places). Widening CODECFLOAT to double would fix this at the root, but we recognize that type is used broadly across the tree and widening it isn't a small change. I'm flagging the mechanism rather than proposing a specific patch.

Happy to share more detail on the reproduction, or discuss either fix direction further.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions