search

mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.67k stars 35.37k forks source link

Loaders: Ensure all loaders properly label and convert color textures and colors to sRGB / Linear correctly #23283

Closed gkjohnson closed 1 year ago

gkjohnson commented 2 years ago

Is your feature request related to a problem? Please describe.

From what I can tell these are the loader examples that still are not setting renderer.outputEncoding = sRGBEncoding on the page meaning they don't appropriately set the model color spaces for linear lighting calculations on load where I think the color space should be known at parse time:

Material Colors & Textures

Vertex Colors

I was taking a look at ColladaLoader and OBJLoader, as well. It doesn't look like there's very explicit documentation on color spaces for these formats and they're more specification by convention. Given that our demos don't adjust the default output color space for the renderer it seems like the assumption is that the textures and colors are sRGB. Is there any evidence or example models that show this isn't always the right thing to do?

Describe the solution you'd like

All loaders look correct when setting WebGLRenderer.outputEncoding to sRGBEncoding by appropriately labeling all color textures and converting colors to Linear before setting materials.

Describe alternatives you've considered

--

Additional context

23272

23280

Mugen87 commented 2 years ago

I was taking a look at ColladaLoader and OBJLoader, as well. It doesn't look like there's very explicit documentation on color spaces for these formats and they're more specification by convention.

There is a note in the Collada spec that says color space information can added to a <hint> attribute:

If this element or a higher precedence element is not present then use a common format R8G8B8A8 with linear color gradient, not sRGB.

However, if we want sRGB to be the default at some point, it makes sense to update ColladaLoader, too.

gkjohnson commented 2 years ago

Something isn't lining up, then. The Collada "Elf" model doesn't specify a <format> tag for any of its textures meaning if that's true then we're displaying our Collada models in the examples incorrectly but the model really doesn't look right if the textures are assumed to be linear:

outputEncoding=LinearEncoding outputEncoding=sRGBEncoding
image image

So is the right image above correct? Or is the elf Collada model incorrect? Does Blender not label exported Collada textures correctly? Or is there something else going on?

Mugen87 commented 2 years ago

The left image is right. If you just set outputEncoding to sRGBEncoding, the sRGB workflow is incomplete.

Mugen87 commented 2 years ago

Meaning we have to update the loader and hack in sRGB support.

Mugen87 commented 2 years ago

I wonder if it is really a good idea to force sRGB with 3D formats that do not properly support it 🤔 .

gkjohnson commented 2 years ago

If you just set outputEncoding to sRGBEncoding, the sRGB workflow is incomplete.

Can you explain this? It should never be correct to display the canvas as Linear on the page. Is the Collada format assuming a linear output color to the display? If interpreting Collada textures as linear colors only looks correct if you are not transforming them before outputting to an sRGB display then I don't understand how they can correctly be considered linear.

Mugen87 commented 2 years ago

AFAICT, the Collada spec does not really care about color spaces. Most times applications just load the Collada, parse the colors/load textures and display the values as they are.

You are right with your assumption that the textures are not linear. When you inspect the color profiles of Collada textures, they are usually sRGB (which is contradictory regarding the spec).

If we want to display sRGB colors, we have to make assumptions when loading the Collada asset which are not necessarily true. For example the standard does not explain in what color space material, light or vertex colors are.

Because of this, I would not touch loaders with no clear color space specification. If we default outputEncoding to sRGBEncoding at some point, I suggest to set LinearEncoding to all Collada examples.

Devs who want a proper color space workflow should work with glTF.

gkjohnson commented 2 years ago

I'm certainly not advocating against preferring glTF when possible but the point of this issue is to get the three.js loaders to behave consistently in the correct labeling and conversion of colors. If it's demonstrable that Collada files are specifying colors and textures in sRGB then they should be loaded such that they behave consistently with the three.js renderer settings. As it is you cannot just load a glTF model and Collada model in the same scene and have them look correct. I see there was an issue on the forum related to the editor in this regard, as well.

When it comes to some of these specs that are ambiguous about color spaces (OBJ, Collada) my assumption has always been that colors and color textures were specified as sRGB just because of the limitations of Linear colors when converting to sRGB for display and the age of the formats. In my experience OBJ files are the same. If it really is ambiguous in practice and there are use cases, example files, or exporters out there using linear textures and colors then maybe a Loader flag can be added to set the color space to load as. Otherwise it sRGB colors seems like the more likely assumption.

mrdoob commented 2 years ago

I agree with @gkjohnson. I think it's fair to assume that the colors and textures in obj and collada files are in srgb (unless specified somehow).

And yes, we should aim to set WebGLRenderer.outputEncoding to sRGBEncoding by default. It'll be easier to assume that anything that it's passed to the renderer is in linear. Easier to spot bugs that way.

gkjohnson commented 2 years ago

It occurs to me that Vertex colors would need to be converted, as well, which might not be such an easily "undoable" change for anyone relying on them being sRGB right now since three.js relies on them being linear. I imagine this would affect both ColladaLoader and PLYLoader, at least. Not sure if we have any vertex colored test models, though.

scurest commented 2 years ago

FWIW, you may want to know that Blender (at least recent versions) exports material colors (eg. Kd) as linear (obj, dae), but vertex colors as sRGB (dae). I don't think this is by intent (this is how it stores it internally, so probably just no one ever thought about it) but it is how it works. I don't know about other formats.

(Personally I agree with gkjohnson that sRGB should be the default assumption.)

gkjohnson commented 2 years ago

FWIW, you may want to know that Blender (at least recent versions) exports material colors (eg. Kd) as linear (obj, dae)

Oh interesting... Is it documented anywhere? Here's a Blender help post from 2019 that seems to discuss the issue and identifies that the output color space changed in Blender 2.8:

https://blenderartists.org/t/rgb-values-wrong-when-exporting-as-obj/1179228/9

The answers from the post are a bit disappointing with the solutions basically amounting to "edit the files directly so they conform to your engine". So much for standards 😁. Either way it doesn't sound like there's a strong rationale for their output color space there.

It would be good to look at what other environments and reference files (if they exist?) do in terms of interpreting color spaces for these formats but unfortunately I don't have the bandwidth to put a list like that together.

gkjohnson commented 2 years ago

At this point I've updated all the loaders I'm familiar with. It's possible that externally maintained loaders (IFC) should wait to upgrade until something like #23392 is figured out. This should get most all of the canonical three.js loaders loading into a consistent color space so it should be easier to transition / catch issues with any future color space management changes.

@looeee would you be able to convert FBXLoader to convert all material and light Color instances and vertex color to Linear on load? And label all color texture maps as sRGB? According to this documentation all colors in FBX are expected to be sRGB (credit to WestLangley for the link).

And @takahirox maybe MMDLoader should do the same so everything works with renderer.outputEncoding = sRGBEncoding?

Other than that I'm unfamiliar with the remaining loaders I listed (Vox, VRML, VTK, PCD, GCode) so I'm not sure if they need to be changed.

Takieddin commented 1 year ago

the issue is still with Dracoloader, any way to it

donmccurdy commented 1 year ago

The .drc files (created by the Draco library) don't have textures or materials, so I think you must be talking about a model with vertex colors? I don't know any rule about what color space its vertex colors must be in, so custom conversions may sometimes be necessary. But since the Draco library only accepts PLY and OBJ files as input, and we already do conversions for those formats...

https://github.com/mrdoob/three.js/blob/7fc4d1e93360fc31aa783466cb2c5b6f3c6361f6/examples/jsm/loaders/OBJLoader.js#L534-L538

https://github.com/mrdoob/three.js/blob/7fc4d1e93360fc31aa783466cb2c5b6f3c6361f6/examples/jsm/loaders/PLYLoader.js#L477-L481

... I think it would be appropriate for us to do the same conversion in DRACOLoader if we are loading a .drc file. The conversion is not necessary when using DRACOLoader to decode the contents of a Draco-compressed glTF file with the dracoLoader.decodeDracoFile(...) method. In general I would recommend using a Draco-compressed .glb rather than using .drc, because the .glb provides a full material specification and other features you would otherwise need to rebuild with .drc.

@Takieddin would you want to provide an example .drc file, and/or open a PR for the conversion?

Takieddin commented 1 year ago

@donmccurdy yes I am referring to vertex color, here is a pointcloud .ply file as a raw data and .drc file the compressed version I uploaded the two on threejs editor .drc on left and .ply on the right image

I would like to know if using glb would be better for my case the problem I think that it needs to be converted to mesh first
worth mentioning that I uploaded the same two file to the plyuploader example template and commented the line //renderer.outputEncoding = THREE.sRGBEncoding; and the colors appeared darker
.drc on the right and .ply on the left image

Takieddin commented 1 year ago

@donmccurdy here are the files loot.zip

donmccurdy commented 1 year ago

@Takieddin thanks! I've opened a PR:

One goal of this issue is for all loaders to assign the correct color space information, where "correctness" involves the definitions and assumptions given on the color management guide. You'll want to keep this renderer output setting enabled, unless you're (a) not rendering to a display, or (b) have some other color management workflow defined.

renderer.outputEncoding = sRGBEncoding
mrdoob commented 1 year ago

Amazing work everyone! 🥹

nightgryphon commented 1 year ago

I've got trouble with mandatory unconditional color conversion sRGB->Linear while loading OBJ+MTL model exported by Blender to AFRAME. Colors exported by Blender seems to be linear so conversion distort them.

Please is it possible to add an option to specify/override the color space assumed for OBJ+MTL while loading or an option to turn off the color conversion so all relying projects can achieve correct color management?

The code causing trouble is examples/jsm/loaders/MTLLoader.js method createMaterial_( materialName ) which perform unconditional color conversion

case 'kd':
        // Diffuse color (color under white light) using RGB values
        params.color = new Color().fromArray( value ).convertSRGBToLinear();

...etc...
donmccurdy commented 1 year ago

Confirmed (in Blender 3.5) that the exported MTL file writes the diffuse color in sRGB-Linear, not sRGB. I haven't checked what it does for vertex colors yet.

I don't believe there's any canonical answer for what OBJ/MTL should use, unfortunately, so this might easily vary from exporter to exporter. We didn't edit any OBJ/MTL files when doing these updates, so presumably our own OBJ/MTL files use sRGB as originally expected.

Possible workaround would be to create an option like:

const loader = new OBJLoader();
loader.sourceColorSpace = THREE.LinearSRGBColorSpace;
loader.load( 'path/to/scene.obj', ... );

This defines the source color space, the target is (always) the current three.js working color space. I certainly wouldn't want to provide this option for file formats that do have a well-established color space convention. But I'm open to providing the option for OBJ/MTL, if others agree.

nightgryphon commented 1 year ago

I've also found recent Blender can export PBR extensions in MTL so it also can be perfect to get an extra option to use MeshPhysicalMaterial instead of MeshPhongMaterial https://benhouston3d.com/blog/extended-wavefront-obj-mtl-for-pbr/

If an option (let call it "usePBR") set than MeshPhysicalMaterial created instead of MeshPhongMaterial and MTL parameters are treated as described at above document. Actually most of parameters are reused but there are a few extras.

donmccurdy commented 1 year ago

MTLLoader could be made to use MeshPhysicalMaterial or MeshStandardMaterial automatically if PBR properties are detected in the source file. Should be no need for users to opt-in on that, it can be automatic.

Perhaps it would be better to discuss this in the thread below, though:

nightgryphon commented 1 year ago

Automatic detection and switch to non-MeshPhongMaterial in general sounds good BUT it will unpredictable affect code assuming MeshPhongMaterial for OBJ loaded models. So there has to be an option to override automatic detection and force the exact material model to use.

Right now i'm trying to implement custom MTLLoader and faced exactly this issue with automatic material model switch. My existing code use envMap and envMapIntensity for OBJ models and switching some materials to non-PhongMaterial significantly change SOME RANDOM MODELS which trigger automatic model change while others stay as intended. This can confuse a lot. So there should be the way to exactly control what is going on while load OBJ.

donmccurdy commented 1 year ago

I would recommend continuing discussion in the thread linked here; support for PBR materials in OBJ is not particularly to this color management thread.