[Bug]: Fix 3D surface corruption on non-linear scales by masking invalid values by rahulrathnavel · Pull Request #31733 · matplotlib/matplotlib

rahulrathnavel · 2026-05-22T17:39:34Z

PR summary

Why is this change necessary & What problem does it solve?
Currently, creating a 3D surface plot (ax.plot_surface) with a non-linear scale (e.g., ax.set_zscale('log')) completely corrupts the surface rendering if the data contains zero or negative values. The new 3D scale transforms (introduced in #30980) were allowing mathematically invalid values to pass through to Poly3DCollection, resulting in NaN/-inf during projection and breaking the renderer.

What is the reasoning for this implementation?
Following the guidance of @scottshambaugh, this PR utilizes the new Scale.val_in_range method introduced in #31306.
Immediately after broadcasting X, Y, and Z in plot_surface, we use np.where to check if the coordinates are valid for their respective axis scales. Any invalid values are replaced with np.nan. This elegantly hooks into the existing downstream NaN filtering logic inside plot_surface, allowing the invalid polygons to be gracefully discarded without corrupting the rest of the valid surface.

AI Disclosure

I used an AI coding assistant to help analyze the 3D projection pipeline and draft the val_in_range masking logic for this PR.

PR checklist

"closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
[N/A] Plotting related features are demonstrated in an example
[N/A] New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

…alues

scottshambaugh · 2026-05-22T17:40:57Z

This needs a test, and is the issue present in other 3D artists? We would want to solve them all at once, and this check probably belongs in a helper function.

rahulrathnavel · 2026-05-22T17:53:52Z

This needs a test, and is the issue present in other 3D artists? We would want to solve them all at once, and this check probably belongs in a helper function.

Hi @scottshambaugh, that makes total sense! You are absolutely right—this is definitely affecting plot_wireframe, plot_trisurf, and likely others as well.

I will abstract the masking logic into a dedicated helper function so it can be applied cleanly across the relevant 3D artists. I'll also get a unit test added to explicitly cover the negative/zero log scale masking. I'll ping you once the refactor and tests are pushed!
so i am on the correct track right?!

scottshambaugh · 2026-05-22T18:19:10Z

You're on the right track, but per our policy please don't use AI to write your pull request description or comments except for direct translation.

rahulrathnavel · 2026-05-22T18:27:01Z

You're on the right track, but per our policy please don't use AI to write your pull request description or comments except for direct translation.

since this is my first PR ,i leaned on it for the formatting part for my text , so it sounds professional .
I have noted and will just write my own replies.
now i am working on the PR, moving the mask logic to helper function and adding the tests now , i will ping you. Thank you!

scottshambaugh

Thanks, your English is very understandable and we would rather hear your direct voice than an AI.

scottshambaugh · 2026-05-22T20:08:09Z

+
+        X = np.where(val_in_range_X(X), X, np.nan)
+        Y = np.where(val_in_range_Y(Y), Y, np.nan)
+        Z = np.where(val_in_range_Z(Z), Z, np.nan)


This is fine for now since we're working a bug fix and want to limit changes. Being able to pass numpy arrays into val_in_range will be more efficient in the future.

scottshambaugh · 2026-05-22T20:09:19Z

+    ax.set_zscale('log')
+
+    # Drawing forces evaluation and confirms the absence of crashes
+    fig.canvas.draw()


This is an inadequate test, as the plot was not crashing previously. Tests need to fail on main and pass on your branch to demonstrate the improvement

@scottshambaugh thanks 😅i will remove that double/duplicate import and update the test to explicitly assert that the invalid polygons are dropped and i will push these and ensure these updates now!

@scottshambaugh thanks 😅i will remove that double/duplicate import and update the test to explicitly assert that the invalid polygons are dropped and i will push these and ensure these updates now!

@scottshambaugh just pushed the updates!
I think some Windows CI tests timed out again, but the actual 3D logic fix green okay right?. Let me know how it looks and if we need any final fixes before merging!

@rahulrathnavel this updated test also passes when I run it on main. You need to actually run the new test yourself on both branches - it wastes our time to have to double check your work. Do not delegate this to an AI. You need to understand all changes you are proposing.

Also, please check exact values in general rather than <, it's a bit more protective.

scottshambaugh · 2026-05-22T21:35:55Z

is the issue present in other 3D artists? We would want to solve them all at once, and this check probably belongs in a helper function.

You've only touched 3 of the axes3d plotting methods, and this really should be applied at the artists3d level instead. I appreciate your enthusiasm and work so far, and if you want to take one more try that's fine. But with the time pressure to get this in 3.11 and the errors in not addressing my comments I would not recommend choosing this as your first issue to solve.

rahulrathnavel · 2026-05-22T21:44:01Z

is the issue present in other 3D artists? We would want to solve them all at once, and this check probably belongs in a helper function.

You've only touched 3 of the axes3d plotting methods, and this really should be applied at the artists3d level instead. I appreciate your enthusiasm and work so far, and if you want to take one more try that's fine. But with the time pressure to get this in 3.11 and the errors in your implementation not addressing my comments so far I would not recommend choosing this as your first issue to solve.

@scottshambaugh, thank you for the honest feedback! I completely understand.
I will gladly step back so you or the core team can implement it.

Fix 3D surface corruption with non-linear scales by masking invalid v…

cb8042d

…alues

github-actions Bot added the topic: mplot3d label May 22, 2026

rahulrathnavel added 2 commits May 22, 2026 23:31

Fix TypeError by vectorizing val_in_range array checks

e175f8c

Fix W293 trailing whitespace on blank line

a4f7f33

rahulrathnavel added 2 commits May 23, 2026 01:31

Refactor scale masking into helper function and add tests

6503bdc

Fix W293 trailing whitespace across refactored files

3aab848

scottshambaugh reviewed May 22, 2026

View reviewed changes

rahulrathnavel added 2 commits May 23, 2026 01:48

Update 3D log scale test to assert dropped polygons

3cab332

Fix W293 trailing whitespace in 3D log scale test

acc5892

rahulrathnavel closed this May 22, 2026

rahulrathnavel deleted the fix-3d-log-negative-surface branch May 22, 2026 21:44

Uh oh!

Conversation

rahulrathnavel commented May 22, 2026

PR summary

AI Disclosure

PR checklist

Uh oh!

scottshambaugh commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rahulrathnavel commented May 22, 2026

Uh oh!

scottshambaugh commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rahulrathnavel commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottshambaugh left a comment

Choose a reason for hiding this comment

Uh oh!

scottshambaugh May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scottshambaugh May 22, 2026

Choose a reason for hiding this comment

Uh oh!

rahulrathnavel May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rahulrathnavel May 22, 2026

Choose a reason for hiding this comment

Uh oh!

scottshambaugh May 22, 2026

Choose a reason for hiding this comment

Uh oh!

scottshambaugh commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rahulrathnavel commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

scottshambaugh commented May 22, 2026 •

edited

Loading

scottshambaugh commented May 22, 2026 •

edited

Loading

rahulrathnavel commented May 22, 2026 •

edited

Loading

rahulrathnavel May 22, 2026 •

edited

Loading

scottshambaugh commented May 22, 2026 •

edited

Loading