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

Editor interpreting unsigned uvec shader uniform parameters as signed #92064

Open
xkisu opened this issue May 18, 2024 · 4 comments · May be fixed by #89529
Open

Editor interpreting unsigned uvec shader uniform parameters as signed #92064

xkisu opened this issue May 18, 2024 · 4 comments · May be fixed by #89529

Comments

@xkisu
Copy link

xkisu commented May 18, 2024

Tested versions

Reproducible in v4.2.1-stable

System information

Godot v4.2.1.stable - macOS 14.4.1 - Vulkan (Forward+) - integrated Apple M1 - Apple M1 (8 Threads)

Issue description

The editor is refusing to let me specify integers over the 32-bit signed max for unsigned shader vector parameters. When I try to specify one over the signed max (i.e. an ARGB hexidecimal color) the editor reverts it to the signed 32-bit max. The max of an unsigned 32-bit integer field should be 4294967295, but as you can see with the steps below it's capped at the signed 32-bit limit of 2147483647.

I'm not sure if this is a wider problem with Godot using the signed Vector4i type for uvec4 shader types (which is a problem of it's own), or if this is just an issue with how the editor is interpreting the parameter limits.
image

image

Steps to reproduce

  1. Create a Shader with a uvec2, uvec3, or uvec4 parameter (example using my shader):
// color_ramp.gd
shader_type canvas_item;

uniform uvec4 a_span; // [spanX, y, color0, color1]

varying vec4 v_rampColor;

mediump vec4 unpack_color_int(uint color) {
	// Format is ARGB
	uint r = (color >> 16u) & 0xffu;
	uint g = (color >> 8u) & 0xffu;
	uint b = (color >> 0u) & 0xffu;
	uint a = (color >> 24u) & 0xffu;

    return vec4(float(r), float(g), float(b), float(a)) / 255.0f;
}

void vertex() {
    v_rampColor = unpack_color_int((VERTEX_ID & 1) == 0 ? a_span.z : a_span.w);
}

void fragment() {
    COLOR = v_rampColor;
}
  1. Attach the shader to a canvas item.
  2. Attempt to set the Z or W parameters to a uint32 hex value, i.e. 4278255360 (0xff00ff00).
    image
  3. Notice that after hitting enter, the value reverts to the signed 32-bit integer max.
    image

Minimal reproduction project (MRP)

unsigned_shader_parameter_issue.zip

@xkisu
Copy link
Author

xkisu commented May 18, 2024

I just realized this does work correctly if I pass the signed decimal conversion equivalent to the fields (don't know why I didn't think to try that before), but I still feel like it should be possible to enter the unsigned values directly for an unsigned field - it feels like erroneous behaviour to have to use signed integers to set an unsigned field.
image

@AThousandShips

This comment was marked as outdated.

@AThousandShips AThousandShips closed this as not planned Won't fix, can't repro, duplicate, stale May 18, 2024
@xkisu
Copy link
Author

xkisu commented May 18, 2024

Thank you for reporting, consolidating in:

Which has been solved on latest branch

See there for more details, if you think something was missed about this and it's not the same issue, please comment here and it can be reopened

It looks like #89436 does fix it for uint parameters, but doesn't for uvec* parameter and the merged changes added a todo comment for them. Would be good to also fix it for uvec parameters otherwise they experience the same behaviour.

// TODO: Handle vector pairs?

@AThousandShips
Copy link
Member

True forgot that detail, then this will be fixed by:

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

Successfully merging a pull request may close this issue.

2 participants