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.

Sign up for GitHub

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

Fix potential unsafe swizzle optimizations #298

Merged
merged 1 commit into from
Apr 17, 2023
Merged

Fix potential unsafe swizzle optimizations #298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/rewriter.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,17 +247,18 @@ let private simplifyVec (constr: Ident) args =
| _ -> allArgs

// vec3(a.x, b.xy) => vec3(a.x, b)
let rec dropLastSwizzle = function
let rec dropLastSwizzle n = function
| [Dot (expr, field) as last] when isFieldSwizzle field ->
match [for c in field -> swizzleIndex c] with
| [0] | [0; 1] | [0; 1; 2] | [0; 1; 2; 3] -> [expr]
| [0] when n = 1 -> [expr]
| [0; 1] | [0; 1; 2] | [0; 1; 2; 3] -> [expr]
| _ -> [last]
| e1 :: rest -> e1 :: dropLastSwizzle rest
| e1 :: rest -> e1 :: dropLastSwizzle (n-1) rest
| x -> x

let args = combineSwizzles args |> List.map useInts
let args = if args.Length = vecSize then mergeAllEquals args args else args
let args = dropLastSwizzle args
let args = dropLastSwizzle vecSize args
FunCall (Var constr, args)

let private simplifyExpr (didInline: bool ref) env = function
Expand Down
11 changes: 10 additions & 1 deletion tests/unit/vectors.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,16 @@ float withExtraComponents() {
vec2 v2 = vec2(1);
vec3 v3 = vec3(1, 2, v2.x);
vec4 v4 = vec4(v2, v3.xy);
return v2.x + v3.x + v4.x;
vec3 v5 = vec3(1, v3.rg);
Comment on lines 25 to +26
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I think that a.xy can become a whenever it's the last argument and there's more than one argument.

Again, assuming I understand you correctly -- that is also properly handled by this PR, and tested via these cases here?

Copy link
Owner

Choose a reason for hiding this comment

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

alright, I think we can merge

Thank!

return v2.x + v3.x + v4.x + v5.x;
}

float withExtraComponentsUnsafe(in vec3 a) {
vec3 v1 = vec3(a.x);
vec3 v2 = vec3(1.0, a.x);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative could be to do the transformation only when there are at least two arguments in the constructor call?

If I understand you, that would apply to calls like this, which are AIUI unsafe to transform (this is equivalent to vec3(1.0, a.xx) not vec3(1.0, a.xy)).

Copy link
Owner

Choose a reason for hiding this comment

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

vec3(1.0, a) is equivalent to vec3(1.0, a.xy)

So I think that a.xy can become a whenever it's the last argument and there's more than one argument.

vec2 v3 = vec3(1.0, a.y);
vec3 v4 = vec3(1.0, 2.0, a.y);
return v1.x + v2.x + v3.x + v4.x;
}

struct S{
Expand Down
10 changes: 9 additions & 1 deletion tests/unit/vectors.frag.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ float withExtraComponents()
vec2 v2=vec2(1);
vec3 v3=vec3(1,2,v2);
vec4 v4=vec4(v2,v3);
return v2.x+v3.x+v4.x;
vec3 v5=vec3(1,v3);
return v2.x+v3.x+v4.x+v5.x;
}
float withExtraComponentsUnsafe(vec3 a)
{
vec3 v1=vec3(a.x),v2=vec3(1,a.x);
vec2 v3=vec3(1,a.y);
vec3 v4=vec3(1,2,a.y);
return v1.x+v2.x+v3.x+v4.x;
}struct S{vec2 p1;vec2 cp1;vec2 cp2;vec2 p2;};
vec2 calc(S points,float t)
{
Expand Down