DrawBBox for displaz

[mrosen]

Here's a start at an application that allow a displaz user to interactively define a bounding box. You can set the center point and grow the box by 50% in x, y and z directions by pressing the corresponding keys. Others may find it useful as a starting point.

[c42f]

Thanks for making the time to submit this :-)

I wonder where best to put this. The test directory is probably better suited to mundane test data, whereas this is a usage example. Perhaps moving it to a new directory doc/examples would make more sense?

Would you consider using the standard PEP-8 for the code style? At the most basic, this means function names should look like make_bbox, and using four spaces per indentation level consistently. (The other python code in this repo (bindings/python) more or less uses PEP-8, I hope :-) )

I've put a bunch of line comments for things which could be a bit neater, if you have the time to do some tweaking.

[c42f]

You should be able to use the python standard library tempfile module to do this without hard coding the PLY_FILE name:

t = tempfile.NamedTemporaryFile(suffix=".ply", delete=False)

[c42f]

As a small nicety, this might be somewhat clearer as a simple for loop (map would normally be used to gather output, but here it's being used more as a foreach, which is something python lacks):

v_center = np.array(v_center)
for uvec in unit_vectors:
    vertex = v_center + transform.dot(uvec)
    f.write("%.17g %.17g %.17g\n" % tuple(vertex))

[c42f]

By the way, I'm using %.17g here, because this will exactly preserve all the binary digits of any floating point number, while keeping the string short if possible.

[c42f]

Did you see the python bindings inside the bindings/python directory? They're pretty basic, but some ply writing utilities like this one could arguably go in there rather than in this example. Just a thought really, I don't mind having this here, for the moment.

[c42f]

Minor note - writing literal blocks of text like this to file can be done neatly with the standard textwrap module (see textwrap.dedent to remove pesky indentation from a big block of text in a function).

[c42f]

I suggest that you use the default shell=False, and pass the arguments as a list ["displaz", "-script", "-rmtemp", PLY_FILE] when calling Popen. This is more annoying to type, but is far more reliable whenever you have arguments which would need to be quoted when passed to the shell, for example, empty arguments, file names with whitespace, etc.

[c42f]

Also, how about labelling your dataset? If you use the -label Box option, it will show up in the dataset view with a more readable name, rather than the temporary file name.

[c42f]

typo: Displayz

[c42f]

Thanks for the docs!

[c42f]

The indentation here is inconsistent with the other functions. Personally I prefer the standard PEP-8 style, with four spaces per indentation level.

[mrosen]

Wow. Thanks for the thorough review. I'm new to python but will address these excellent suggestions over the next few days. msr

…

On Thu, Mar 30, 2017 at 3:44 AM Chris Foster ***@***.***> wrote: ***@***.**** commented on this pull request. Thanks for making the time to submit this :-) I wonder where best to put this. The test directory is probably better suited to mundane test data, whereas this is a usage example. Perhaps moving it to a new directory doc/examples would make more sense? Would you consider using the standard PEP-8 for the code style? At the most basic, this means function names should look like make_bbox, and using four spaces per indentation level consistently. (The other python code in this repo (bindings/python) more or less uses PEP-8, I hope :-) ) I've put a bunch of line comments for things which could be a bit neater, if you have the time to do some tweaking. ------------------------------ In test/DrawBBox.py <#165 (comment)>: > + # apply transform to unit vectors and then add result to center + unit_vectors = [ + [0, 1, -1, 0], [1, 0, -1, 0], [0, -1, -1, 0], [-1, 0, -1, 0], + [0, 1, 1, 0], [1, 0, 1, 0], [0, -1, 1, 0], [-1, 0, 1, 0] + ] + xfm_unit_vectors = [transform.dot(unit) for unit in unit_vectors] + vertices = [numpy.add(v_center, xuv) for xuv in xfm_unit_vectors] + + # write out resulting vertices + map(lambda v: f.write("%s\n" % " ".join(str(elem) for elem in v)), vertices) + f.write("4 0 1 2 3\n") + f.write("4 4 5 6 7\n") + +# write out the .ply file and have Displaz render it +def UpdateDisplaz(v, T): + t = open(PLY_FILE, 'w') You should be able to use the python standard library tempfile module to do this without hard coding the PLY_FILE name: t = tempfile.NamedTemporaryFile(suffix=".ply", delete=False) ------------------------------ In test/DrawBBox.py <#165 (comment)>: > + f.write("property float y\n") + f.write("property float z\n") + f.write("element face 2\n") + f.write("property list uint int vertex_index\n") + f.write("end_header\n") + + # apply transform to unit vectors and then add result to center + unit_vectors = [ + [0, 1, -1, 0], [1, 0, -1, 0], [0, -1, -1, 0], [-1, 0, -1, 0], + [0, 1, 1, 0], [1, 0, 1, 0], [0, -1, 1, 0], [-1, 0, 1, 0] + ] + xfm_unit_vectors = [transform.dot(unit) for unit in unit_vectors] + vertices = [numpy.add(v_center, xuv) for xuv in xfm_unit_vectors] + + # write out resulting vertices + map(lambda v: f.write("%s\n" % " ".join(str(elem) for elem in v)), vertices) As a small nicety, this might be somewhat clearer as a simple for loop ( map would normally be used to gather output, but here it's being used more as a foreach, which is something python lacks): v_center = np.array(v_center)for uvec in unit_vectors: vertex = v_center + transform.dot(uvec) f.write("%.17g %.17g %.17g\n" % tuple(vertex)) ------------------------------ In test/DrawBBox.py <#165 (comment)>: > @@ -0,0 +1,88 @@ +#!/usr/bin/env python2 +import subprocess +import numpy +import sys + +# Edit this as required (windows, mac). Be able to write this. +PLY_FILE="/tmp/displaz_bbox.ply" + +# write .ply file representing a box with specfied center and transform to f +def MakeBBox(v_center, transform, f): Did you see the python bindings inside the bindings/python directory? They're pretty basic, but some ply writing utilities like this one could arguably go in there rather than in this example. Just a thought really, I don't mind having this here, for the moment. ------------------------------ In test/DrawBBox.py <#165 (comment)>: > + +# Edit this as required (windows, mac). Be able to write this. +PLY_FILE="/tmp/displaz_bbox.ply" + +# write .ply file representing a box with specfied center and transform to f +def MakeBBox(v_center, transform, f): + f.write("ply\n") + f.write("format ascii 1.0\n") + f.write("comment simple rectangular prism\n") + f.write("element vertex 8\n") + f.write("property float x\n") + f.write("property float y\n") + f.write("property float z\n") + f.write("element face 2\n") + f.write("property list uint int vertex_index\n") + f.write("end_header\n") Minor note - writing literal blocks of text like this to file can be done neatly with the standard textwrap module (see textwrap.dedent to remove pesky indentation from a big block of text in a function). ------------------------------ In test/DrawBBox.py <#165 (comment)>: > + ] + xfm_unit_vectors = [transform.dot(unit) for unit in unit_vectors] + vertices = [numpy.add(v_center, xuv) for xuv in xfm_unit_vectors] + + # write out resulting vertices + map(lambda v: f.write("%s\n" % " ".join(str(elem) for elem in v)), vertices) + f.write("4 0 1 2 3\n") + f.write("4 4 5 6 7\n") + +# write out the .ply file and have Displaz render it +def UpdateDisplaz(v, T): + t = open(PLY_FILE, 'w') + t.truncate() + MakeBBox(v, T, t) + t.close() + p = subprocess.Popen(["displaz -script -rmtemp " + PLY_FILE], stdout=subprocess.PIPE, shell=True, bufsize=1) I suggest that you use the default shell=False, and pass the arguments as a list ["displaz", "-script", "-rmtemp", PLY_FILE] when calling Popen. This is more annoying to type, but is far more reliable whenever you have arguments which would need to be quoted when passed to the shell, for example, empty arguments, file names with whitespace, etc. ------------------------------ In test/DrawBBox.py <#165 (comment)>: > + ] + xfm_unit_vectors = [transform.dot(unit) for unit in unit_vectors] + vertices = [numpy.add(v_center, xuv) for xuv in xfm_unit_vectors] + + # write out resulting vertices + map(lambda v: f.write("%s\n" % " ".join(str(elem) for elem in v)), vertices) + f.write("4 0 1 2 3\n") + f.write("4 4 5 6 7\n") + +# write out the .ply file and have Displaz render it +def UpdateDisplaz(v, T): + t = open(PLY_FILE, 'w') + t.truncate() + MakeBBox(v, T, t) + t.close() + p = subprocess.Popen(["displaz -script -rmtemp " + PLY_FILE], stdout=subprocess.PIPE, shell=True, bufsize=1) Also, how about labelling your dataset? If you use the -label Box option, it will show up in the dataset view with a more readable name, rather than the temporary file name. ------------------------------ In test/DrawBBox.py <#165 (comment)>: > + f.write("4 4 5 6 7\n") + +# write out the .ply file and have Displaz render it +def UpdateDisplaz(v, T): + t = open(PLY_FILE, 'w') + t.truncate() + MakeBBox(v, T, t) + t.close() + p = subprocess.Popen(["displaz -script -rmtemp " + PLY_FILE], stdout=subprocess.PIPE, shell=True, bufsize=1) + +def Usage(): + print "Usage: DrawBBox <input point cloud file>\n" + print " Shift Click will set the 3D cursor and put a 1 x 1 x 1 BBox there.\n" + print " Do this first.\n" + print " Then you can press x, y and z to grow it.\n" + print " Remember that these keystrokes go into the Displayz application (needs focus)" typo: Displayz ------------------------------ In test/DrawBBox.py <#165 (comment)>: > + f.write("4 4 5 6 7\n") + +# write out the .ply file and have Displaz render it +def UpdateDisplaz(v, T): + t = open(PLY_FILE, 'w') + t.truncate() + MakeBBox(v, T, t) + t.close() + p = subprocess.Popen(["displaz -script -rmtemp " + PLY_FILE], stdout=subprocess.PIPE, shell=True, bufsize=1) + +def Usage(): + print "Usage: DrawBBox <input point cloud file>\n" + print " Shift Click will set the 3D cursor and put a 1 x 1 x 1 BBox there.\n" + print " Do this first.\n" + print " Then you can press x, y and z to grow it.\n" + print " Remember that these keystrokes go into the Displayz application (needs focus)" Thanks for the docs! ------------------------------ In test/DrawBBox.py <#165 (comment)>: > + + # apply transform to unit vectors and then add result to center + unit_vectors = [ + [0, 1, -1, 0], [1, 0, -1, 0], [0, -1, -1, 0], [-1, 0, -1, 0], + [0, 1, 1, 0], [1, 0, 1, 0], [0, -1, 1, 0], [-1, 0, 1, 0] + ] + xfm_unit_vectors = [transform.dot(unit) for unit in unit_vectors] + vertices = [numpy.add(v_center, xuv) for xuv in xfm_unit_vectors] + + # write out resulting vertices + map(lambda v: f.write("%s\n" % " ".join(str(elem) for elem in v)), vertices) + f.write("4 0 1 2 3\n") + f.write("4 4 5 6 7\n") + +# write out the .ply file and have Displaz render it +def UpdateDisplaz(v, T): The indentation here is inconsistent with the other functions. Personally I prefer the standard PEP-8 style, with four spaces per indentation level. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#165 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAtUMjzkZ7mKwTGJzYfeoq3f_aWFN55eks5rq4eMgaJpZM4Mtic9> .

[c42f]

Great!

[mrosen]

Sorry for the long delay here. There were internal company issues as well as other distractions. I think the new code is PEP-8 compliant and certainly does a lot more. I hope others find this useful!