-
-
Notifications
You must be signed in to change notification settings - Fork 723
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.
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
support also partial cubemap/fallback images: backgroundColor for missing face(s) #570
Conversation
I've continued this branch with a major extension of The tile and fallback face files will only be produced when they actually contain image data. I also added some generally useful options. Some options are just passed through in the .json file to the viewer. Added options regarding partial panoramas:
Added options regarding cylindrical input:
Added general options:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good. The only major change I'd like is to define missing cubemap faces in the configuration instead of checking by trial and error.
src/js/pannellum.js
Outdated
console.log(config.strings.fileAccessError.replace('%s', e.target.src)+'; will use background instead'); | ||
onLoad(); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prevents helpful error messages when URLs are incorrect. Missing faces should be defined as null
in the configuration, and this should then be checked instead of trying and failing to load the face.
utils/multires/generate.py
Outdated
origHeight = str(origHeight) | ||
origWidth = str(origWidth) | ||
origFilename = os.path.join(os.getcwd(), args.inputFile) | ||
extension = '.jpg' | ||
if args.png: | ||
extension = '.png' | ||
colorList = eval(args.backgroundColor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling eval
is unsafe; ast.literal_eval
should be used instead.
utils/multires/generate.py
Outdated
|
||
# Create output directory | ||
os.makedirs(args.output) | ||
if not os.path.exists(args.output): | ||
os.makedirs(args.output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was intended to fail if the output directory already exists. A check could be added to print a descriptive error message instead of a stack trace, but I don't think the script should continue if the output directory already exists.
src/js/libpannellum.js
Outdated
@@ -101,6 +101,37 @@ function Renderer(container) { | |||
pose = undefined; | |||
|
|||
var s; | |||
var faceMissing = false; | |||
var cubeImgWidth = undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The = undefined
part is unnecessary; var cubeImgWidth;
is functionally identical.
src/js/libpannellum.js
Outdated
if (imageType == 'cubemap') { | ||
for (s = 0; s < 6; s++) { | ||
if (image[s].width > 0) { | ||
if (cubeImgWidth == undefined) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strict comparison, ===
, should be used.
src/js/libpannellum.js
Outdated
if (loaded == 5 && this.width == 0) // support partial fallback/cubemap image | ||
this.width = fallbackImgSize; | ||
if (this.width > 0) { | ||
if (fallbackImgSize == undefined) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strict comparison, ===
, should be used.
…descriptive error message
Thanks for your swift feedback; |
for convenience, no need to delete output directory every time while testing
Great. Thanks! |
…sing face(s) (mpetroff#570) * support also partial cubemap/fallback images: backgroundColor for missing face(s) * extend multires/generate.py to cylindrical input and partial panoramas * fixed a few typos in new help strings of generate.py * Missing cubemap faces now need to be defined as null in the configuration * made generate.py strict again on existing output directory, printing descriptive error message * replaced unsafe eval by ast.literal_eval in generate.py * a couple of minor improvements regarding 'undefined' in libpannellum.js * made generate.py allow existing output directory in debug mode for convenience, no need to delete output directory every time while testing
Similarly to the recently added support for missing tiles in multires panoramas,
this PR adds support for missing faces in cubemap (which includes fallback) panoramas.
Any areas for which no data is available are shown with the configurable
backgroundColor
.I updated the documentation of
backgroundColor
accordingly.This PR also includes a couple of minor improvements of related code.
This is in a sense a continuation of PR #563.