Maniphest T67889

Object Importer Error
Closed, Resolved
Actions

Assigned To
Bastien Montagne (mont29)
Authored By
Brad Hawkins (ConsUme)
Jul 29 2019, 6:22 PM
Tags
  • Add-ons (Community)
  • Import/Export
Subscribers
Bastien Montagne (mont29)
Brad Hawkins (ConsUme)
Philipp Oeser (lichtwerk)
Sybren A. Stüvel (sybren)

Description

System Information
Operating system: Windows-10-10.0.17134 64 Bits
Graphics card: Intel(R) HD Graphics 620 Intel 4.5.0 - Build 23.20.16.4973

Blender Version
Broken: version: 2.80 (sub 75), branch: master, commit date: 2019-07-24 14:22, hash: rB507ffee6e1f4
Worked: (optional)

Short description of error
Certain Obj files will not import and throws an error
Traceback (most recent call last):

File "C:\Users\bh630\Downloads\blender-2.80rc3-windows64\blender-2.80rc3-windows64\2.80\scripts\addons\io_scene_obj\__init__.py", line 145, in execute
  return import_obj.load(context, **keywords)
File "C:\Users\bh630\Downloads\blender-2.80rc3-windows64\blender-2.80rc3-windows64\2.80\scripts\addons\io_scene_obj\import_obj.py", line 1215, in load
  use_image_search, float_func)
File "C:\Users\bh630\Downloads\blender-2.80rc3-windows64\blender-2.80rc3-windows64\2.80\scripts\addons\io_scene_obj\import_obj.py", line 338, in create_materials
  context_mat_wrap.roughness = 1.0 - (sqrt(float_func(line_split[1])) / 30)

ValueError: math domain error

location: <unknown location>:-1

Exact steps for others to reproduce the error
Use Release candidate 3 and just use the Object:

3DS - The Legend of Zelda Ocarina of Time 3D - Gerudo Mask.zip7 KBDownload

Revisions and Commits

rBA Blender Add-ons
rBA27381001d7b9 Fix T67889: Object Importer Error.

Related Objects

Mentioned In
rBdb0782e9460e: Py node shader wrapper: Add clamping to official min/max values.

Event Timeline

Brad Hawkins (ConsUme) created this task.Jul 29 2019, 6:22 PM
Philipp Oeser (lichtwerk) lowered the priority of this task from 90 to 50.Jul 30 2019, 9:37 AM
Philipp Oeser (lichtwerk) edited projects, added Add-ons (Community), Import/Export; removed BF Blender.
Philipp Oeser (lichtwerk) added a subscriber: Philipp Oeser (lichtwerk).

Confirmed, checking...

Philipp Oeser (lichtwerk) added a subscriber: Bastien Montagne (mont29).Jul 30 2019, 10:13 AM

Looks like the material file specifies a negative specular exponent... [this should not be allowed imho]

# Blender MTL File: 'None'
# Material Count: 1

newmtl material-01029822
Ns -3.921569

I guess we should just clamp this to min 0.0, but will check with others...

Also: seeing this has actually been exported from blender: checked, and we actually allow negative specular exponent [if roughness is above 1.0 -- which we seem to allow as well...] to be exported [should not be allowed I guess, see above...]

# XXX Totally empirical conversion, trying to adapt it
#     (from 1.0 - 0.0 Principled BSDF range to 0.0 - 900.0 OBJ specular exponent range)...
spec = (1.0 - mat_wrap.roughness) * 30
spec *= spec
fw('Ns %.6f\n' % spec)

Not sure if this could/should be clamped in our material (node) system, or on export, will check with others as well...

CC @Bastien Montagne (mont29)

Sybren A. Stüvel (sybren) added a subscriber: Sybren A. Stüvel (sybren).Jul 30 2019, 10:24 AM
In T67889#739236, @Philipp Oeser (lichtwerk) wrote:
spec = (1.0 - mat_wrap.roughness) * 30
spec *= spec
fw('Ns %.6f\n' % spec)

This code can only produce negative specularity if the roughness is a complex number. Squaring the number using spec *= spec makes it absolute, hence nonnegative.

IMO we should handle out-of-range values gracefully, either rejecting the material altogether or clamping the values.

Philipp Oeser (lichtwerk) added a comment.EditedJul 30 2019, 10:27 AM
In T67889#739274, @Sybren A. Stüvel (sybren) wrote:
In T67889#739236, @Philipp Oeser (lichtwerk) wrote:
spec = (1.0 - mat_wrap.roughness) * 30
spec *= spec
fw('Ns %.6f\n' % spec)

This code can only produce negative specularity if the roughness is a complex number. Squaring the number using spec *= spec makes it absolute, hence nonnegative.

IMO we should handle out-of-range values gracefully, either rejecting the material altogether or clamping the values.

oops, true, wasnt paying attention that it gets squared... I wonder how this got exported from blender then (I mean how it got negative)?

Bastien Montagne (mont29) claimed this task.Jul 30 2019, 10:38 AM

Exported from Blender 2.75.... That's an old geezer, who knows what it was doing at that time? ;)

Seriously, yes, we should clamp those values on import to a valid range, just safer. Probably even in the material wrapper thingy, such that all IO scripts could benefit from it.

Bastien Montagne (mont29) changed the task status from Unknown Status to Resolved by committing rBA27381001d7b9: Fix T67889: Object Importer Error..Jul 30 2019, 11:24 AM
Bastien Montagne (mont29) mentioned this in rBdb0782e9460e: Py node shader wrapper: Add clamping to official min/max values..
Bastien Montagne (mont29) added a commit: rBA27381001d7b9: Fix T67889: Object Importer Error..