Going back to this PR, I would really like to get this merged.

Below is a summary of the situations on benchmarks, API changes, and tests.
Feedback would be very appreciated.

Benchmarks

importsysfrompathlibimportPathimportcProfileimportpstatsimportioimportnumpyasnpfromskimage.measureimportmarching_cubes_lewinerfromtimeimporttime,sleepfrommatplotlib.backends.backend_qt5aggimportFigureCanvasfrommatplotlib.figureimportFigureimportmatplotlib.pyplotaspltfromPyQt5importQtWidgets,QtCorefrommpl_toolkits.mplot3dimportAxes3D# noqaprofile=FalseclassFigureWidget(QtWidgets.QWidget):def__init__(self,parent=None,canvas=None):super().__init__(parent)self.canvas=canvasself.layout=QtWidgets.QVBoxLayout()ifcanvasisnotNone:self.layout.addWidget(self.canvas)self.setLayout(self.layout)classExampleCanvas(FigureCanvas):def__init__(self):self.figure=Figure()FigureCanvas.__init__(self,self.figure)self.ax=self.figure.add_subplot(111,projection="3d")defplot(self,X=None):ifXisNone:self.draw()returnself.ax.clear()print(f"# Input dataset shape :{X.shape}")t0=time()verts,faces,normals,values=marching_cubes_lewiner(X,level=X.max()*0.5        )dt=time()-t0print(f" - marching_cubes_lewiner:{verts.shape[0]} vertices in{dt:.2f}s")ifprofile:pr=cProfile.Profile()pr.enable()t0=time()self.ax.plot_trisurf(verts[:,0],verts[:,1],faces,verts[:,2],color="r",antialiased=True        )self.draw()plt.pause(0.001)dt=time()-t0print(f" - plot_trisurf in{dt:.3f}s")t0=time()angles=list(range(0,360,72))forangleinangles:self.ax.view_init(30,angle)self.draw()plt.pause(0.001)dt=time()-t0print(f" - rotation{dt/len(angles):.2f} s/frame")ifprofile:pr.disable()s=io.StringIO()ps=pstats.Stats(pr,stream=s)ps.sort_stats('tottime')# ps.print_stats()ps.print_callers()print(s.getvalue())if__name__=="__main__":app=QtWidgets.QApplication(sys.argv)canvas=ExampleCanvas()widget=FigureWidget(canvas=canvas)canvas.plot()basedir=Path(__file__).parentapp=QtWidgets.QApplication(sys.argv)canvas=ExampleCanvas()widget=FigureWidget(canvas=canvas)canvas.plot()widget.show()classBenchmarkThread(QtCore.QThread):defrun(self):sleep(1)X=np.load(basedir/"data_3d.npy")X=np.abs(X)forstepin [4,2,1]:Xsl=X[::step, ::step, :]canvas.plot(Xsl)thread=BenchmarkThread()thread.start()sys.exit(app.exec_())

run on this 3D datasetdata_3d.npy (130 MB).

The results are below,
master

# Input dataset shape : (150, 150, 95) - marching_cubes_lewiner: 3632 vertices in 0.08s - plot_trisurf in 0.751s - rotation 0.54 s/frame# Input dataset shape : (300, 300, 95) - marching_cubes_lewiner: 8640 vertices in 0.33s - plot_trisurf in 1.729s - rotation 1.27 s/frame# Input dataset shape : (600, 600, 95) - marching_cubes_lewiner: 22174 vertices in 1.18s - plot_trisurf in 4.276s - rotation 3.40 s/frame

This PR

# Input dataset shape : (150, 150, 95) - marching_cubes_lewiner: 3632 vertices in 0.09s - plot_trisurf in 0.478s - rotation 0.28 s/frame# Input dataset shape : (300, 300, 95) - marching_cubes_lewiner: 8640 vertices in 0.34s - plot_trisurf in 0.973s - rotation 0.55 s/frame# Input dataset shape : (600, 600, 95) - marching_cubes_lewiner: 22174 vertices in 1.15s - plot_trisurf in 2.369s - rotation 1.24 s/frame

so this speeds upplot_trisurf initial drawing by 1.5-1.7x and then rendering of the rotation by 2-2.7x for different input types. It is a lot.

Handle API changes

This PR changes quite a vew functions to return numpy arrays instead of lists.

If I follow@anntzer 's suggestion (#11577 (comment)), it would mean deprecating,

• art3d.path_to_3d_segment
• art3d.paths_to_3d_segments
• art3d.path_to_3d_segment_with_codes
• art3d.paths_to_3d_segments_with_codes
• art3d.line_collection_2d_to_3d

then because we cangeproj3d.proj_transform_vec andproj3d.proj_transform_vec_clip we would also have to deprecate (since the output would change),

• proj3d.proj_transform
• proj3d.proj_transform_clip
• proj3d.transform
• proj3d.proj_trans_points
• proj3d.proj_trans_clip_points

I'm not sure what you think the ideal public API ofmplot3d should look like and whether this would go in the right direction. In any case, if you agree with this, I think these deprecations might better done is a separate PR (prior to merging this one).

Fix CI

So CI was passing fine a few commits ago, but then after I merged master, I started to get two test failures,

E           matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 1.384):E           result_images/test_mplot3d/bar3d_shaded.pngE           result_images/test_mplot3d/bar3d_shaded-expected.png[...]E           matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 0.003):E           result_images/test_mplot3d/bar3d_notshaded.pngE           result_images/test_mplot3d/bar3d_notshaded-expected.png

For instance,
bar3d_shaded-expected.png

bar3d_shaded.png

bar3d_shaded-failed-diff.png

would you have suggestions where this could be coming from?

Apart for that I think I addressed most comments, apart for possible performance improvements inhttps://github.com/matplotlib/matplotlib/pull/11577/files/ad0e783d17be4dce4d3ee6d84807d388a3bbde14#diff-fc54f86f287bf203dfa9e78102e2678b,#11577 (review) by@eric-wieser .

I think any additional performance improvements should be done in a follow up PR. Now I'm mostly concerned about the API deprecations and making CI pass, in order to make this mergable.

I don't know if we need to completely deprecate all those functions. What if instead of deprecating them, we modify them to output lists if the input was a list (and deprecate that feature as we transition to allow only numpy arrays)? Note, I am not against changing the API, because these benefits are very compelling, and the list-based approach is so outdated. I am just trying to get a feel for our options.

As for the test failure, the image difference is with the bar with no height. Perhaps there is a numerical instability somewhere that is causing the normal vector to flip?

For the bar with no height, it's possible that you reversed the order of a list of polygons somewhere, but numerical instability would also make sense.

I think in that case it would be fine to change the expected image

I don't know if we need to completely deprecate all those functions. What if instead of deprecating them, we modify them to output lists if the input was a list (and deprecate that feature as we transition to allow only numpy arrays)? Note, I am not against changing the API, because these benefits are very compelling, and the list-based approach is so outdated. I am just trying to get a feel for our options.

Most of these functions are clearly helpers that should never have been part of the public API to start with. More importantly, adding code to each and every one of them to make them check whether the input is a list and return a list in that case (and then immediately deprecating that) is just code churn; plus itstill breaks backcompat if someone was passing an array in and expecting a list out (which was the previous behavior), so it doesn't actually help anyone.

Thanks for the feedback. Made a PR to deprecate these helper functions in#13030

So CI was passing fine a few commits ago, but then after I merged master, I started to get two test failures,

I beleive@anntzer changed the test to no longer run into this issue

Rebase conflicts are likely with#16472

Sorry I don't have the availability to continue working on this. So this PR can be marked as stalled (or a more appropriate label).

If someone wants to continue it, they should certainly feel free to do so.

Given that this has been stalled for 2 years and we have had a fair amount of refactoring to the mplot3d code base I am going to close this, however it may be worth re-considering the ideas in these changes to improve performance.

Thankyou for your effort on this@rth ,@eric-wieser and@Kojoley

attn@scottshambaugh