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

Add iDate builtin to Shadertoy utility #428

Merged
merged 8 commits into from
Nov 21, 2023
Merged

Conversation

Vipitis
Copy link
Contributor

@Vipitis Vipitis commented Nov 18, 2023

starting my list from pygfx/shadertoy#15 with the the iDate builtin.

this seems straight forward, since time is already used in _update I was expecting this to be an easy add.
the milliseconds via the perf_timer that is already there seem like a janky solution.
the format seems to be correct, and it's also correctly updated. But none of the shaders I tried work as expected.

Tasks:

  • add iDate to glsl builtins
  • add logic to ._update (isn't working correctly)
  • add iDate to wgsl builtins
  • fix offset (currently seconds are in iDate.y, but should be in iDate.w) found a solution that feels incorrect Addressed correctly and docstring added for guidance
  • [ ] add the ability to freeze date for .snapshot (maybe ?) not with this PR
  • add an example/test

@Vipitis
Copy link
Contributor Author

Vipitis commented Nov 21, 2023

Hey, I believe I need a bit of help with a particular issue. iDate is a vec4 with 32bit floats. the best reference I found is this: https://www.shadertoy.com/view/ldKGRR (this shadertoy doesn't run due to an unrelated issue with I believe the syntax...), but another example that does work which I have been using to test this is here: https://www.shadertoy.com/view/MdVcRd.

The straight forward idea was to just add a new object to the _uniform_data UniformArray. However the behavior was unexpected (months value was the only value that was changing with seconds). So I though there was some kind of offset with the binary data. I tried to understand how it works and spend some time with the debugger. This is the mem object (without padding). It's 64 bytes of which only 56 hold data. There is two additional 32 bit blocks. Not sure why.

shader._uniform_data.mem.hex(" ", 4)
'00000000 00000000 00000000 00000000 00004844 0000e143 0000803f 88c55540 d6a09a3f 01000000 00e0fc44 00002041 0000a841 9788b945 00000000 00000000'
 mouse-f-4,                          resolution-f-3,            time-f-1,t_d-f-1, frame-I-1,date-f-4,                          pad?,    pad?;

In python everything looks correct, the offset makes sense, the views are correct etc. But I am not sure how it is handled on the c side.

Since the order of fields in the UniformArray matches the data of ShadertoyInput struct, perhaps the struct declaration of int is incorrect (glsl and wgsl).
So I came up with a horrible fix to the problem ( most recent commit), by adding the padding infront of the date. Shaders now work as expected.
Looking at the memory with padding looks like this:

shader._uniform_data.mem.hex(" ", 4)
'00000000 00000000 00000000 00000000 00004844 0000e143 0000803f 8a538d41 4695e43f 03000000 00000000 00000000 00e0fc44 00002041 0000a841 2612d545'
 mouse-f-4,                          resolution-f-3,            time-f-1,t_d-f-1, frame-I-1,pad-I-1,pad-I-1, date-f-4;                                   

I don't think that is a good solution and It feels like I might be breaking something else.

@almarklein
Copy link
Collaborator

almarklein commented Nov 21, 2023

Yes, alignment of fields in a uniform array is tricky 😄 See e.g. https://www.w3.org/TR/WGSL/#alignment-and-size

It's usually best to put larger fields first. In fact, I think that if you put date directly after mouse, you'd get vec4, vec4, vec3, and then a bunch of 32-bit scalars, and I suspect that no padding is needed.

@Vipitis
Copy link
Contributor Author

Vipitis commented Nov 21, 2023

I added an example and it also passed all tests and examples. (mem_tests fail, but that seems to be unrelated).
adding date options to the .snapshot() method to ensure determinism can't be easily done here, but I am not forgetting about that.

@Vipitis Vipitis marked this pull request as ready for review November 21, 2023 17:03
@Vipitis Vipitis requested a review from Korijn as a code owner November 21, 2023 17:03
Korijn
Korijn previously approved these changes Nov 21, 2023
@Korijn
Copy link
Collaborator

Korijn commented Nov 21, 2023

Seems like there is a lint failure, otherwise this looks good to me! 👍

almarklein
almarklein previously approved these changes Nov 21, 2023
@Vipitis Vipitis dismissed stale reviews from almarklein and Korijn via 9336b7b November 21, 2023 21:22
@Vipitis
Copy link
Contributor Author

Vipitis commented Nov 21, 2023

looks like #noqa doesn't mind for long strings. This essentially was indentation inconsistencies im the shadercode

@Korijn Korijn merged commit 031a766 into pygfx:main Nov 21, 2023
19 checks passed
@Vipitis Vipitis deleted the shadertoy-idates branch November 21, 2023 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants