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 more uniforms to .snapshot() #37

Merged
merged 12 commits into from
Jan 16, 2025
Merged

Add more uniforms to .snapshot() #37

merged 12 commits into from
Jan 16, 2025

Conversation

Vipitis
Copy link
Collaborator

@Vipitis Vipitis commented Sep 25, 2024

Also fixes #34 by keeping the fractional part.

by skipping the ._update() method for offscreen=True variant we can easily set most of the uniforms in the snapshot method. Should make this very close to deterministic.

note: whole offscreen and snapshot situation needs a rework, as there are recent improvements in wgpu-py that should make it easier. But will look into that at a later date, the current implementation will do.

@Vipitis Vipitis requested a review from Korijn October 14, 2024 08:56
@Vipitis Vipitis mentioned this pull request Jan 10, 2025
@Vipitis
Copy link
Collaborator Author

Vipitis commented Jan 10, 2025

looks like some other stuff broke too... will have a look later

@Vipitis
Copy link
Collaborator Author

Vipitis commented Jan 15, 2025

little lost on the CI thing for screenshots here. it seems to not collect any tests ( 0 out of 1) for this workload... It should be 22 collected and 1 selected instead.
Which is the odd part to me. I saw multiple changes screenshot testing and CI for wgpu-py and tried to make a few of these changes - but not really getting it working right now.
I tried this on a remote linux machine and made sure it's using llvmpipe and it did pass and generate the screenshot just fine - so I am unsure what the differences to Github CI are (I didn't use the CI runner image).

remaining more things to try:

  • example testutils as an actual module (but test examples works right now) ref: Fixes to testing wgpu-py#604
  • difference in the workflow script between ci.yaml and screenshot.yaml: screenshots has an additional apt repository (but wgpu-py also has it):

In light of #31 and the potential to remake this whole thing in the future, I had hoped to find a small patch to make this

@Korijn
Copy link
Contributor

Korijn commented Jan 15, 2025

You could try printing the full stdout & stderr of the subprocess in _determine_can_use_wgpu_lib to see if there's any clues there

@Vipitis
Copy link
Collaborator Author

Vipitis commented Jan 15, 2025

so... it turns out it was this alternative drivers that caused a panic somehow (your review comment about it vanished). Which is why there are differences between the two workflows.

So wgpu-py removed this from ci.yaml in pygfx/wgpu-py#350 but never from the screenshots.yaml (it's still there) which is how we inherited it. And it must have became a problem recently.

will add the docs and this should be good

@Korijn Korijn merged commit 4ca11da into main Jan 16, 2025
10 checks passed
@Korijn Korijn deleted the inp-idate branch January 16, 2025 06:01
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.

iDate.w milliseconds out of sync
2 participants