Skip to content

Fix GPU shader correctness and warnings surfaced by DXC warning flags - #2319

Open
num3ric wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
num3ric:fix/gpu-shader-unused-var-warnings
Open

Fix GPU shader correctness and warnings surfaced by DXC warning flags#2319
num3ric wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
num3ric:fix/gpu-shader-unused-var-warnings

Conversation

@num3ric

@num3ric num3ric commented May 29, 2026

Copy link
Copy Markdown
Contributor
  • GpuShaderUtils: add intCast() helper that emits a C-style cast on HLSL (silences -Wfloat-conversion) and a functional cast on GLSL/MSL/OSL.
  • FixedFunctionOpGPU: use intCast() for the three implicit float->int conversions in the ACES2 hue-weight setup and cusp-table search.
  • FixedFunctionOpGPU: drop the unused _Add_SinCos_Shader calls in Gamut_Compress_Fwd/Inv_Shader (gamut-compress stays in JMh space, so cos_hr/sin_hr were dead).
  • GradingBSplineCurve: in the HUE-FX inverse path (curveIdx == 7), offset knStartY by knStart to match the CPU evalCurveRevHue.
  • GradingRGBCurveOpGPU: inverse eval no longer declares identity_x (it's never used in the inverse), and the in/out qualifier branch is folded into a single emitter — matches the CPU evalCurveRev signature.
  • DxApp: when verbose (test_gpu_exec --dx -v), pass -Wall -Wextra -Wconversion to DXC and print DXC_OUT_ERRORS even on success, so generated-shader warnings are visible during debugging.

  * GpuShaderUtils: add intCast() helper that emits a C-style cast on HLSL (silences -Wfloat-conversion) and a functional cast on GLSL/MSL/OSL.
  * FixedFunctionOpGPU: use intCast() for the three implicit float->int conversions in the ACES2 hue-weight setup and cusp-table search.
  * FixedFunctionOpGPU: drop the unused _Add_SinCos_Shader calls in Gamut_Compress_Fwd/Inv_Shader (gamut-compress stays in JMh space, so cos_hr/sin_hr were
  dead).
  * GradingBSplineCurve: in the HUE-FX inverse path (curveIdx == 7), offset knStartY by knStart to match the CPU evalCurveRevHue.
  * GradingRGBCurveOpGPU: inverse eval no longer declares identity_x (it's never used in the inverse), and the in/out qualifier branch is folded into a
  single emitter — matches the CPU evalCurveRev signature.
  * DxApp: when verbose (test_gpu_exec --dx -v), pass -Wall -Wextra -Wconversion to DXC and print DXC_OUT_ERRORS even on success, so generated-shader
  warnings are visible during debugging.

Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com>
@num3ric

num3ric commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

See attached file for warning logs (without fixes included here): dxc_warnings_full.log

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed exact head 1d41e95d8738fdccc3d4bac4104c665c5bcbc519 against exact PR base 57a94a8003c00be506a41f54a080d9fefaba6d22 and current main 5a808fb57a94c7229640a97835c420c9a1fbd1fe. Although GitHub marks the branch behind, it merges cleanly; I tested the resulting current-main tree a28f55e4e14a070f2a82f779bb73c0de038a9be9.

The shader changes are internally consistent. The HLSL-only C-style integer cast preserves existing truncation while avoiding DXC's functional-cast warning; other shader languages retain their required functional syntax. The HUE-FX inverse lower bound now matches the CPU path's knot offset, the inverse RGB-curve helper safely drops an argument that was redundant because its empty-curve path returns x, and the removed sine/cosine setup was dead in the standalone JMh gamut-compress paths.

Exact base, head, and current merge all build successfully. The same 50 shader/curve/fixed-function focused tests pass on all three. The full CPU runs are also baseline-equivalent: base and head each have 1144 passes / 4 failures, and current merge has 1149 passes / the same 4 failures (two unavailable-monitor failures and two pre-existing AppleClang floating-point tolerance failures). Live exact-head CI is fully green: 21 check-runs plus EasyCLA and ReadTheDocs, with no pending, failed, or action_required workflow. I found no blocking issue.

AI disclosure: I used OpenAI Codex to inspect the exact revisions and repository policy, build the exact base/head/current-main merge oracle, run and compare the focused and full CPU suites, verify live checks and review history, and draft this review. I verified the evidence and conclusion.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants