Skip to content

Conversation

@exeldro
Copy link
Contributor

@exeldro exeldro commented Mar 1, 2024

Description

Add support for #if and #elif to CF parser
It supports without comparator and with the following comparators: ==, >=, <=, !=, <, >

Motivation and Context

This was not supported yet and I wanted to use it in a shader. And it helps when trying to convert existing shaders to be used in OBS.

How Has This Been Tested?

On windows 11 by having a shader with this code in it:

#define test 1234

#if test == 0

#elif 0

#elif test > 1234

#elif test < 1234

#elif test == 1234

#endif

Types of changes

  • Tweak (non-breaking change to improve existing functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

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

While the code seems quite robust (and was able to handle quite a few malformed preprocessor instructions), it seems to fail over one specific variation of correct code:

float4 PSDrawBare(VertInOut vert_in) : TARGET
{
#define test 1234

#if test == 0
    float vector = 4.0;
#elif 0
    float vector = 3.0;
#elif test > 1234
    float vector = 2.0;
#elif test < 1234
    float vector = 1.0;
#elif test == 1234
    float vector = 5.0;
#endif

	return image.Sample(def_sampler, vert_in.uv);
}

The parser breaks after the #elif 0 preprocessor line, as it attempts to parse a "sub-block" (via cf_preprocess_subblock) but because the code already "jumped" over the newline after 0, it finds the non-preprocessor token float and skips the entire line.

And indeed if the line were to say #elif 1 instead, the parser skips the line, finds #elif test > 1234, somehow jumps back to the #elif 1, throws another error, skips the current line, finds #elif test < 1234 and then throws the outside of "#if/#ifdef/#ifndef block" error, so the internal state of the parser seems to get messed up.


Overall I think this would be fine to add, as it's clearly "missing functionality" from the parser/preprocessor (it is an open TODO after all).

I shouldn't forget to mention that the use of macros to create some variation of "uber-shaders" is discouraged with modern APIs which instead expect you to compose fragment and vertex shader out of smaller "function objects" that you compile and load only once, rather than creating several "fat" variants of the same shader influenced by preprocessor conditionals.

We get shader compilation caching "for free" with Metal on macOS and have implemented it for Direct3D11, but at some point we need to move to precompiled shaders (including proper shader "libraries") with the expectation that plugins also have to do the same, but that point hasn't been reached yet.

@exeldro
Copy link
Contributor Author

exeldro commented Jan 20, 2026

cur_token was indeed wrong for things like #if 0, #if 1, #elif 0 and #elif 1
Rebased and fixed the issue

@exeldro exeldro requested a review from PatTheMav January 20, 2026 16:52
Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

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

Only bug I could find was fixed by most recent push, seems to work fine otherwise.

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

Labels

Enhancement Improvement to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants