Skip to content

[MRG] Optimize 3D display (continued) #11577

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.

Sign up for GitHub

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

Closed
rth wants to merge 37 commits into from
Closed

[MRG] Optimize 3D display (continued) #11577

Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
2a75026
Optimize 3D display (#6085)
AlexandreAbraham Jul 5, 2018
b168bed
Remove irrelevant changes
rth Jul 5, 2018
a3c331d
Fix undefined variables
rth Jul 5, 2018
a5c2bd5
Check diff with respect to the original PR
rth Jul 5, 2018
e955f56
Address some review comments
rth Jul 5, 2018
e227b50
Make _juggle_axes_vec and _rotate_axes_vec private
rth Jul 5, 2018
ee53be5
Address review comments
rth Jul 14, 2018
8244c07
Make TestVoxels.test_rgb_data pass
rth Jul 14, 2018
677e79b
Review comment - use np.split + np.cumsum
rth Jul 14, 2018
d095d26
Fix broadcasting
rth Jul 14, 2018
005f6fb
PEP8
rth Jul 15, 2018
f74f3d7
Better debug message
rth Jul 15, 2018
dda0f82
Merge branch 'master' into optimize_3d
rth Jul 15, 2018
1c1e4a1
Fix for numpy 1.10
rth Jul 15, 2018
7a5c50d
Merge branch 'optimize_3d' of github.com:rth/matplotlib into optimize_3d
rth Jul 15, 2018
7c2c22e
Ignore flake8 exception and code clean-up
rth Jul 15, 2018
b81c118
Fix sphinx build
rth Jul 15, 2018
822f4f7
More review comments
rth Jul 15, 2018
004fff2
Fix PEP8
rth Jul 15, 2018
f7795e0
Address part of Eric's comments
rth Jul 16, 2018
da7f72c
Fix anntzer's comments
rth Jul 16, 2018
17657a4
Address Benjamin's review comments
rth Jul 18, 2018
e43b930
Minor fixes
rth Jul 22, 2018
7211708
Removing _juggle_axes_vec and _rotate_axes_vec
rth Jul 22, 2018
5bfa368
Remove flake8 E231 ignore for art3d.py
rth Jul 22, 2018
5faaa0c
Addressed more eric-weiser's comments
rth Jul 23, 2018
7631cfe
Convert _seg_sizes to array
rth Jul 23, 2018
ad0e783
Handle empty sequences
rth Jul 29, 2018
98d1161
Address review comments
rth Aug 18, 2018
1696d52
Experimentally revert get_colors to the previous behaviour
rth Aug 18, 2018
867819f
Merge branch 'master' into optimize_3d
rth Sep 20, 2018
92daa11
Merge branch 'master' into optimize_3d
rth Dec 17, 2018
a268fc7
Fix merge issue
rth Dec 17, 2018
946b9bc
Fix more merge conflicts
rth Dec 17, 2018
ff18f5f
Lint
rth Dec 18, 2018
76d5dc7
Fix one test failure
rth Dec 18, 2018
3e230c7
Address comments
rth Dec 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address some review comments
  • Loading branch information
@rth
rth committed Jul 5, 2018
commit e955f56a5802779226ed605caf07483456d4d966
20 changes: 10 additions & 10 deletions lib/mpl_toolkits/mplot3d/art3d.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ def path_to_3d_segment(path, zs=0, zdir='z'):
# Pre allocate memory
seg3d = np.ones((3, len(path)))

# Works either if zs is array or scalar
seg3d[2] *= zs
# Works both if zs is and array and a scalar

@WeatherGod WeatherGod Jul 18, 2018

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.

"... is an array or a scalar"

seg3d[2, :] *= zs

@rth rth Jul 15, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#6085 (comment) by @Kojoley

seg3d[2, :] = zs

Done

#6085 (comment) by @eric-wieser

If you do that, use np.empty above too

When using np.empty tests fail (I'm not quite sure why) so I kept the np.ones

@eric-wieser eric-wieser Jul 16, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't look done to me - you have *=, not =

@rth rth Jul 16, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Absolutely, missed that -- this was also the reason for np.empty not working. Fixed both.


pathsegs = path.iter_segments(simplify=False, curves=False)
for i, ((x, y), code) in enumerate(pathsegs):
Expand All @@ -181,7 +181,6 @@ def paths_to_3d_segments(paths, zs=0, zdir='z'):
"""Convert paths from a collection object to 3D segments."""

zs = np.broadcast_to(zs, len(paths))

segs = [path_to_3d_segment(path, pathz, zdir)
for path, pathz in zip(paths, zs)]
return np.asarray(segs)

@WeatherGod WeatherGod Jul 18, 2018

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.

Eh, this is technically an API change. At the very least, note that it returns a numpy array.

Expand All @@ -194,7 +193,7 @@ def path_to_3d_segment_with_codes(path, zs=0, zdir='z'):
seg3d = np.ones((3, len(path)))

# Works both if zs is an array and a scalar
seg3d[2] *= zs
seg3d[2, :] *= zs

pathsegs = path.iter_segments(simplify=False, curves=False)
codes = np.empty(len(path))
Expand Down Expand Up @@ -472,20 +471,21 @@ def set_3d_properties(self, zs, zdir):
def do_3d_projection(self, renderer):
xs, ys, zs = self._offsets3d
vxyzis = proj3d.proj_transform_clip(xs, ys, zs, renderer.M)
vzs = vxyzis[2]

fcs = (zalpha(self._facecolor3d, vxyzis[2]) if self._depthshade else
fcs = (zalpha(self._facecolor3d, vzs) if self._depthshade else
self._facecolor3d)
fcs = mcolors.to_rgba_array(fcs, self._alpha)
self.set_facecolors(fcs)

ecs = (zalpha(self._edgecolor3d, vxyzis[2]) if self._depthshade else
ecs = (zalpha(self._edgecolor3d, vzs) if self._depthshade else
self._edgecolor3d)
ecs = mcolors.to_rgba_array(ecs, self._alpha)
self.set_edgecolors(ecs)
PathCollection.set_offsets(self, vxyzis[0:2].T)

if len(vxyzis) > 0:
return min(vxyzis[2])
if vzs.size > 0:
return min(vzs)
else:
return np.nan

Expand Down Expand Up @@ -580,9 +580,9 @@ def get_vector(self, segments3d):
# Store the points in a single array for easier projection
n_segments = np.sum(self._seg_sizes)
# Put all segments in a big array
self._vec = np.vstack(segments3d)
_vec = np.vstack(segments3d)
# Add a fourth dimension for quaternions
self._vec = np.hstack([self._vec, np.ones((n_segments, 1))]).T
self._vec = np.hstack([_vec, np.ones((n_segments, 1))]).T

@eric-wieser eric-wieser Jul 29, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

More efficient as

self._vec = np.empty((4, n_segments))
self._vec[:3,:] = segments3d
self._vec[3,:] = 1

which also eliminates the need for the special case when len(segments3d) == 0


def set_verts(self, verts, closed=True):
"""Set 3D vertices."""
Expand Down