Skip to content

Update lens_distortion.wgsl - #25218

Open
JupiterSky11 wants to merge 4 commits into
bevyengine:mainfrom
JupiterSky11:patch-1
Open

Update lens_distortion.wgsl#25218
JupiterSky11 wants to merge 4 commits into
bevyengine:mainfrom
JupiterSky11:patch-1

Conversation

@JupiterSky11

Copy link
Copy Markdown

Summary of Changes

Fixed the visual creasing between quadrants by changing the perpendicular value of the dot product (zero) to be at 45 degrees from the corner rather than 90. Also changed the direction calculation to use the built-in normalize function.

Objective

Correct the visual creasing in the lens distortion shader.

Solution

The distortion is warped by the dot product between the closest corner and the absolute position of the current texle position. This means that zero influence will be perpendicular to the corner. The angle between the corner and the axis is 45 degrees, which is only half of the way to the 90 degrees we need.
To correct this, I subtract TAU (the maximum value of the dot product in this situation), then multiply the value by 0.5. This multiplication shifts the original point of zero up from 90 degrees away from the corner to 45 degrees to the corner. I then add TAU back to make the maximum value line up with where it originally was.

Testing

I placed the edge of the screen aligned with a straight element to examine the curve visually. With this fix, the axis crease no longer appeared.

LensDistortionShaderFix

Fixed the visual creasing between quadrants by changing the perpendicular value of the dot product (zero) to be at 45 degrees from the corner rather than 90.  Also changed the `direction` calculation to use the built-in `normalize` function.
@github-actions

Copy link
Copy Markdown
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly ✨

@kfc35 kfc35 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Jul 30, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering Jul 30, 2026
@kfc35 kfc35 added S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Shaders This code uses GPU shader languages labels Jul 30, 2026

@alice-i-cecile alice-i-cecile left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fix looks solid, but we should include your very helpful information from your PR description in the comments here, so future readers can follow along <3

@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 30, 2026
@JupiterSky11

Copy link
Copy Markdown
Author

Thanks @alice-i-cecile ! I realized this broke some usages of the "multiplier" variable, so I fixed that too. I'll comment it up and introduce the changes in another commit.

Fixed some bugs, added comments explaining the goals of the shader and the mechanics of how some of it works.
@JupiterSky11

Copy link
Copy Markdown
Author

Let me know if there's any linguistic errors, confusing wording, or anything else I should fix! I believe this fits with the original intended functionality well enough.

@JupiterSky11

Copy link
Copy Markdown
Author

I have noticed a few weird little hairs on this shader that didn't really need to be there. The divide by zero check in the original, the if statement, and the clamp at the end... none of them seem to do anything, and branching code on a GPU is something you should avoid unless absolutely necessary.

Unless anyone here knows if these are targeting platform dependent bugs or some other arcane silliness, I'm going to keep stripping them away. I'm also going to check out the other shaders introduced in the effects stack to see if there's more clean up to do, but I'll put that in another pull request if it happens.

Removed some extraneous code that has no affect on the output.
@JupiterSky11

Copy link
Copy Markdown
Author

So that's what that button does. Thanks github for explaining that so well...

@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Aug 2, 2026
let direction = normalize(uv_multiplied);

// Correct for the uv multiplier to prevent scaling the image along with the distortion.
let direction_adjusted = direction / multiplier;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

multiplier can be zero, it would NaN everything

/// is stronger on one axis than the other. When a component of multiplier is set to 0.0,
/// no distortion effect is applied.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oops, guess I misinterpreted the multiplier effect then. I'm gonna have some tinkering to do. Not sure why anyone would want that, but the more functionality the merrier.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior D-Shaders This code uses GPU shader languages D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Uncontroversial This work is generally agreed upon

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

4 participants