GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Tweaks to adjust to new pygfx blending by almarklein · Pull Request #838 · fastplotlib/fastplotlib

Conversation

@almarklein

@almarklein almarklein commented May 27, 2025

Copy link
Copy Markdown
Collaborator

For upcoming pygfx/pygfx#1002

May not be complete, can leave open until the pygfx-pr is merged.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@almarklein
Tweaks to adjust to new pygfx blending
c16fe18
kushalkolar
kushalkolar reviewed May 27, 2025
View reviewed changes
Comment thread fastplotlib/layouts/_figure.py
self.renderer.render(self._underlay_scene, self._underlay_camera, flush=False)

# With new pygfx' blending, the depth buffer is only cleared after each flush, we need a manual depth
# clear to erase the depth values set by the underlay.

@kushalkolar kushalkolar May 27, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

what about the overlay?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@almarklein almarklein May 28, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Good point, you may want to use a different depth range in the camera, so that the fragments all have a very small depth value, so they don't get behind stuff in the scene. Or simply disable depth-tests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@almarklein almarklein May 28, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Or you can clear the depth buffer again, if you don't need it for post processing or things like that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@kushalkolar kushalkolar Jun 29, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What are depth tests? Is there a performance difference between this or clearing again? What's your recommendation, I'm not too familiar with how the depth buffer works in this context, only how it's important for knowing what objects are above what in a 3D scene. Eventually the overlay scene will probably have axes when using an orthographic projection, along with legends etc.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@almarklein almarklein Jul 3, 2025

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The depth test makes sure that objects are not occluded by objects that are behind it (but are rendered later).

The depth buffer needs to be cleared before using it, otherwise the depth test makes no sense. Since there is an underlay pass and an overlay pass, we need extra manual clears.

I added a clear before the overlay pass. That's definitely the simplest solution. The only caveat is that if users want access to the depth buffer (of the scene) they can't because the depth buffer will represent the content of the overlay. If we ever want to change that, we can look into that later. There are more upcoming changes in pyfx that will likely make that easier.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@kushalkolar kushalkolar Jul 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The only caveat is that if users want access to the depth buffer (of the scene) they can't because the depth buffer will represent the content of the overlay. If we ever want to change that, we can look into that later. There are more upcoming changes in pyfx that will likely make that easier.

do you want to make a note of it in the pygfx issue if you don't have this planned already?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@kushalkolar

kushalkolar commented Jun 29, 2025

Copy link
Copy Markdown
Member

I think this should be merged soon after we decide how to deal with the depth buffer for the overlay pass?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@almarklein

almarklein commented Jun 30, 2025

Copy link
Copy Markdown
Collaborator Author

There are some more upcoming changes. I'll likely make some more changes this week.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

almarklein added 2 commits July 3, 2025 14:54
@almarklein
disable ppaa for screenshot tests
52e8945
@almarklein
also clear for overlay
8cb599a
@almarklein
almarklein marked this pull request as ready for review July 3, 2025 13:31
@almarklein
almarklein requested a review from clewis7 as a code owner July 3, 2025 13:31
@almarklein

almarklein commented Jul 3, 2025

Copy link
Copy Markdown
Collaborator Author

Let's merge this, so we keep main working. I'll just open a new PR if new stuff arises.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar

kushalkolar commented Jul 4, 2025

Copy link
Copy Markdown
Member

Failures are due to pygfx/wgpu-py#723

And I think due to AA being disabled for tests now? Some diffs below:

diff-rect_frac_layout-rgb

diff-line_cmap_more-rgb

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@almarklein
pin imgui-bundle
9ca57b6
@almarklein

almarklein commented Jul 4, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator Author

The image tests fail because the downsample filter is changed (to go from hirez to target rez). So indeed related to SSAA. See pygfx/pygfx#1121

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@almarklein

almarklein commented Jul 4, 2025

Copy link
Copy Markdown
Collaborator Author

AA being disabled for tests now

The env var that disables PPAA is for a different kind of aa 😉, which has not been merged yet (at the time of writing): pygfx/pygfx#1117 That env var will make sure that the screenshots don't change when that PR is in effect.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions

github-actions Bot commented Jul 4, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

📚 Docs preview built and uploaded! https://www.fastplotlib.org/ver/pygfx-blending

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar

kushalkolar commented Jul 9, 2025

Copy link
Copy Markdown
Member

The image tests fail because the downsample filter is changed (to go from hirez to target rez). So indeed related to SSAA. See pygfx/pygfx#1121

So if I update the ground truth screenshots good to go!?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar kushalkolar mentioned this pull request Jul 10, 2025
Imgui pin to attempt bugfix release #876
Merged
@almarklein

almarklein commented Jul 10, 2025

Copy link
Copy Markdown
Collaborator Author

So if I update the ground truth screenshots good to go!?

I'm half wondering if I want to add a similar env var to override the pixel_ratio for screenshot tests, now that we have to update all screenshots ... I'm sorry it takes so long. Is this stopping other prs from progressing?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar
Merge branch 'main' into pygfx-blending
3cd5d37
@kushalkolar

kushalkolar commented Jul 17, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This is weird, seems like only the regenerated screenshots with imgui in the environment don't have AA, whereas the environments without imgui do:

EDIT: ok now I understand how github actions pushes generated screenshots artifacts. If an image hasn't been re-generated it still gets sent because it exists in the repo already. I should do separate dirs for imgui and non-imgui screenshots or filter what's upload to the artifact.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar added 2 commits July 17, 2025 04:15
@kushalkolar
update ground truth screenshots to not use AA
23c4732
@kushalkolar
update nb screenshot to not use AA
4a5089d
kushalkolar
kushalkolar approved these changes Jul 17, 2025
View reviewed changes

@kushalkolar kushalkolar left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

merging, now main requires pygfx@main too

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@kushalkolar
kushalkolar merged commit 11fed41 into main Jul 17, 2025
@almarklein
almarklein deleted the pygfx-blending branch August 21, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kushalkolar kushalkolar kushalkolar approved these changes
@clewis7 clewis7 Awaiting requested review from clewis7 clewis7 is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@almarklein @kushalkolar
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.