Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added translator support for Modulus token #136

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

vishalghige43
Copy link

PR Description

Added support for the modulus operator (%) and the assignment modulus operator (%=) in the CGL translator.

Related Issue

Resolves #116
Working on adding modulus support to the backend.

Shader Sample

This is the PerlinNoise.cgl example code for testing the modulus operator (%) and assignment modulus operator (%=):

shader PerlinNoise {

    // Perlin Noise Function
    float perlinNoise(vec2 p) {
        return fract(sin(dot(p, vec2(12.9898, 78.233))) * 43758.5453);
    }

    // Fragment Shader
    fragment {
        input vec2 vUV;
        output vec4 fragColor;

        void main() {
            float noise = perlinNoise(vUV);
            float height = noise * 10.0;
            vec3 color = vec3(height / 10.0, 1.0 - height / 10.0, 0.0);
            fragColor = vec4(color, 1.0);

            int color2 = 1200 % 10;       //MOD test
            color2 %= 10;                   //ASSIGN_MOD test
        }
    }
}

Checklist

  • Have you added the necessary tests?
  • Only modified the files mentioned in the related issue(s)?
  • Are all tests passing?

"LESS_THAN": "<",
"GREATER_THAN": ">",
"ASSIGN_ADD": "+=",
"ASSIGN_SUB": "-=",
"ASSIGN_MUL": "*=",
"ASSIGN_DIV": "/=",
"ASSIGN_MOD": "%=",
"LESS_EQUAL": "<=",
Copy link
Contributor

Choose a reason for hiding this comment

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

you have working on MOD operator so only add changes for MOD don't add Assign_MOD here someone else might working on this task . or you can assign this task also to yourself and mention in PR.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I'll make the changes and only include the modifications for the MOD operator in the PR

@@ -24,6 +24,9 @@ shader PerlinNoise {
float height = noise * 10.0;
vec3 color = vec3(height / 10.0, 1.0 - height / 10.0, 0.0);
fragColor = vec4(color, 1.0);

int color2 = 1200 % 10; //MOD test
color2 %= 10; //ASSIGN_MOD test
Copy link
Contributor

Choose a reason for hiding this comment

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

this is just a demo shader file do not change anything here . write tests only at tests/test_translator/test_codegen/

Copy link
Author

Choose a reason for hiding this comment

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

Got it! I won't make any changes to the demo shader file.
Thanks for the clarification.

Copy link
Contributor

@samthakur587 samthakur587 left a comment

Choose a reason for hiding this comment

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

add tests also for you changes

"ASSIGN_ADD",
"ASSIGN_SUB",
"ASSIGN_MUL",
"ASSIGN_DIV",
"ASSIGN_AND",
"ASSIGN_OR",
"ASSIGN_XOR",
"ASSIGN_MOD",
"BITWISE_SHIFT_RIGHT",
Copy link
Contributor

Choose a reason for hiding this comment

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

why you removed the Assign_MOD no need to remove it your task it to just add MOD support at translator frontend.

@samthakur587
Copy link
Contributor

hii @vishalghige43 can you please fix this and also solve the merge conflict .

@vishalghige43
Copy link
Author

hii @vishalghige43 can you please fix this and also solve the merge conflict .

ok,I'll complete it today.

@NripeshN NripeshN added the stale label Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add translator support for Modulus token
3 participants