Repository navigation
Remove duplicate LCD enum checks from Plot shader tests - #25
Merged
Merged
Conversation
The shared LCD contract suite already owns all five enum and uniform mappings. Keep Plot's distinct shader integration checks while removing the repeated mapping case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The shader-reference suite repeats the same five LCD enum values and shader-uniform helper calls already checked by
vnm_msdf_text/tests/lcd_contract_tests.cpp. Keeping a second copy adds maintenance without a distinct regression boundary.Changes
Remove only the duplicate enum/uniform case and its invocation. Keep the shader-specific source checks, including filter integration, channel ordering, sampling and opaque-background rules. Add a short ownership comment pointing to the contract suite. No production behavior or test registration changes.
This addresses the narrow, immediately actionable part of test-audit finding F08. Replacing the remaining source-spelling oracles with rendering behavior is deferred; this change does not claim to solve that separate issue.
Validation
Compiled and ran the remaining shader-reference executable with native g++ and warnings enabled. Compiled and ran the unchanged shared
lcd_contract_testsowner: both contract cases pass. The companion Plot/MSDF shader source check also passes. Qt/GPU rendering and full project CMake builds were not run. Qt is unavailable here, and these selected CPU tests require neither Qt nor a GPU.Current work checked
Compared current master and the latest merged PR diff. No open PR was present in this repository at the duplicate-work check. The recent atlas-sizing change does not remove this enum-only duplicate.
Independent source review found no concrete blockers in the bounded removal or the retained contract ownership.