See: T38441 for details, this diff is for review only
Details
Diff Detail
Event Timeline
initial review.
| io_export_paper_model.py | ||
|---|---|---|
| 66 | no need for try here | |
| 70 | whats blist ? | |
| 81 | this could be a regular function | |
| 114 | our API should be able to do this. | |
| 159 | image names can be up to 64 characters now, does this size need updating? | |
| 174 | Best use pythons tempfile - http://docs.python.org/3/library/tempfile.html | |
| 214 | prefer filepath.lower().endswith((".svg", ".png")) | |
| 994 | Much more efficient to simply do present.clear() | |
| 1798 | I think this is a bit confusing, why not import bmesh and call bmesh.new ? | |
Replies to the comments in code. (I'll also fix the remaining ones I don't reply to.)
| io_export_paper_model.py | ||
|---|---|---|
| 70 | a type similar to list, implemented using a search tree. It can insert at any position in O(log n). https://pypi.python.org/pypi/blist/1.3.4 | |
| 114 | sure, as v1.to_3d().cross(v2.to_3d()).z | |
| 994 | I don't think that would work. There may be other unrelated items in the list present. Each vertex may have many instances in the flat net. | |
| 1798 | this way seemed to look more like Python to me, but I should better just conform with the API. | |
I commited the changes to https://raw2.github.com/addam/Export-Paper-Model-from-Blender/master/io_export_paper_model.py
It still doesn't use tempfile, that will require a bit deeper changes.
| io_export_paper_model.py | ||
|---|---|---|
| 1798 | Or perhaps I'll just remove this function for now. It's commented out from the UI. | |
Committed 2d cross product to mathutils.
| io_export_paper_model.py | ||
|---|---|---|
| 114 | Update, v1.cross(v2) now works for 2d vectors. https://developer.blender.org/rB06b6cd83459713ef5c00f705f6cdf1481ed24179 | |
I made quite a lot of changes in the code and hopefully I understood a bit how Phabricator works, so I'm trying to update this.
The code should now be easier to read, although it still does not fully comply with PEP8.
I fixed a few issues both in the unfolding core and in the SVG output.
The script is compatible with Blender 2.70---2.72b.
Updates based on campbellbarton's comments:
- style changes (a lot of them)
- UI: 3D toolbox: tabbed interface, split panel into two
- API update: 2d cross product. Removed the cross_product function
- added __slots__ to all classes that are more than singletons
- use tempfile module for embedding images, instead of random names
- linked images: use relative paths and convert to forward slashes in SVG
Other improvements and new features:
- Outer line is a contiguous SVG path (suitable for cutting plotters)
- new operator: Clear All Seams
- Island.join/is_below: robustness against various kinds of disgusting geometry
- explicit check: touching vs. crossing boundaries
- changed boundary_sorted -> boundary (hopefully a speedup)
- rename 'Make Unfoldable' -> 'Unfold' everywhere
- improved main_faces selection
(...and many fixes of various importance)
Trying to make a complete diff (again), with two more commits:
- code cleanup; leaving broken output of markers
- fix: converting dimensions throughout the process
Good; now this diff is again ready for code review.
(sorry for the mess, I'm still learning how to use Arcanist)
This can go into contrib.
@Adam Dominec (emu). I have added you to addons project, so you can commit to this repository.