search

yt-project / yt_idv

Interactive volume rendering for yt
Other
9 stars 6 forks source link

main no longer rendering selected field? #25

Closed chrishavlin closed 3 years ago

chrishavlin commented 3 years ago

Currently on main when you run amr_volume_rendering.py you get what I think is a rendering of one of the coordinates even though the density field is specified:

snap_0001_main

If I revert back to before the octree merge, I get the density field as expected

snap_0000

matthewturk commented 3 years ago

Definitely a bug, definitely my fault, definitely will try to fix asap. Whoops.

On Fri, Aug 6, 2021, 7:49 AM chrishavlin @.***> wrote:

Currently on main when you run amr_volume_rendering.py you get what I think is a rendering of one of the coordinates even though the density field is specified:

[image: snap_0001_main] https://user-images.githubusercontent.com/22038879/128512527-a1d48870-37e1-4604-9b0a-7ddf22e9527a.png

If I revert back to before the octree merge, I get the density field as expected

[image: snap_0000] https://user-images.githubusercontent.com/22038879/128512580-57806413-d53d-43dd-94a3-891ec5e3e825.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/yt-project/yt_idv/issues/25, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAVXO5N5WMDKWG4AQ4GWSTT3PK5ZANCNFSM5BV3HHEQ .

matthewturk commented 3 years ago

So I'm looking over the changed files and I'm not sure where it could be coming from.

https://github.com/yt-project/yt_idv/pull/21/files

The possibilities I see are in using the textureoffset call instead of texture, but looking at that, it seems like I don't know why it would go to x instead. But can you try reverting that specific shader? And, does it do this with all the types of shaders?

chrishavlin commented 3 years ago

So it seems that it is related to the changes to in the max_intensity.frag.glsl. After reverting those 2 lines of textureOffset changes, the amr_volume_rendering.py example runs as expected. But one clue I hadn't noticed before -- the following message gets printed to the terminal (using the latest version on main)

b"0:172(56): error: parameter `in offset' must be a constant expression\n0:172(20): error: type mismatch\n0:173(61): error: parameter `in offset' must be a constant expression\n0:173(21): error: type mismatch\n0:176(7): warning: `map_sample' used uninitialized\n0:176(61): warning: `tex_sample' used uninitialized\n0:177(20): warning: `tex_sample' used uninitialized\n0:179(14): warning: `map_sample' used uninitialized\n"
matthewturk commented 3 years ago

Ah! Ok. That's the problem. It's not compiling, and when it doesn't, it uses a default shader that uses x.

I don't know why it doesn't like our specification of offset. Maybe that needs to be made constant?

On Fri, Aug 6, 2021, 1:33 PM chrishavlin @.***> wrote:

So it seems that it is related to the changes to in the max_intensity.frag.glsl. After reverting those 2 lines of textureOffset changes, the amr_volume_rendering.py example runs as expected. But one clue I hadn't noticed before -- the following message gets printed to the terminal (using the latest version on main)

b"0:172(56): error: parameter in offset' must be a constant expression\n0:172(20): error: type mismatch\n0:173(61): error: parameterin offset' must be a constant expression\n0:173(21): error: type mismatch\n0:176(7): warning: map_sample' used uninitialized\n0:176(61): warning:tex_sample' used uninitialized\n0:177(20): warning: tex_sample' used uninitialized\n0:179(14): warning:map_sample' used uninitialized\n"

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/yt-project/yt_idv/issues/25#issuecomment-894444572, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAVXO77WOBB7HMQSUO66DTT3QTHXANCNFSM5BV3HHEQ .

chrishavlin commented 3 years ago

Maybe that needs to be made constant?

Ya, I think so -- khronos says " The offset value must be a constant expression." but I haven't figured out the right way to do it...

matthewturk commented 3 years ago

Funnily enough, it works for me! I'll see if I can break it. The final answer here:

https://stackoverflow.com/questions/25440114/argument-4-to-function-texelfetchoffset-must-be-a-constant-expression

seems to touch on what we might be seeing -- I'm on NVIDIA, and I think you might not be. Anyway, we can't rely on this behavior; I thought having it be a flat uniform would be sufficient, but I guess not. What we'll need to do is use the offset and the known dimensions of the texture (which we can get via textureSize) to figure out the new coordinate dimensions. I think it might be something like:

texture(ds_tex, (tex_curr_pos * textureSize(ds_tex) + texture_offset)/textureSize(ds_tex)).rgb

but there might be some off-by-ones or something.