Texture: Remove RGBFormat.

[Mugen87]

Related issue: -

Description

This PR removes THREE.RGBFormat. There are three major reasons for this step:

• THREE.RGBFormat is slow. Not all devices natively support RGB texture formats which requires an emulation in software. Several resources in the WebGL space recommend to always use RGBA.
• sRGB encoded color textures have to be in RGBA8 otherwise mipmap generation via WebGL is not possible. That makes the usage of RGB8 impractical.
• WebGPU does not support 3 channel formats. By aligning WebGL and WebGPU we automatically improve the portability of apps or components like loaders.

Devs who are affected by this change should use RGBAFormat instead.

[mrdoob]

Thanks!

[gkjohnson]

Is RGBIntegerFormat affected by the same issues? Does it need to be removed, as well?

[Mugen87]

Good catch! Yes, it should also be removed.

[luruke]

Hi @Mugen87 ,

THREE.RGBFormat is slow. Not all devices natively support RGB texture formats which requires an emulation in software. Several resources in the WebGL space recommend to always use RGBA.

Do you mind share some info? You know what device/driver/gpu are not supporting natively RGB?

[Mugen87]

E.g. all devices using Metal support no 3 channel formats.

[Makio64]

I know it was a necessary step for the futur but it would be great in the futur to add a deprecated flag one version before this kind of big change, some libs dependent on it just exploded following the update like 'postprocessing' : pmndrs/postprocessing#338

[mrdoob]

We can still add a deprecation warning. Would you like to create a PR with it?

[Makio64]

We can still add a deprecation warning. Would you like to create a PR with it?

I wonder if somethings like this would do the job with treeshaking :

const RGBFormat = (function(){ 
console.warn('RGBFormat is now deprecated for performance reason, see https://github.com/mrdoob/three.js/pull/23228')
return 1022 
})()

//...
export{..., RGBFormat, ... }

[mrdoob]

I don't think that does what you want.

[Usnul]

Hey guys, this is pretty old, but I have just come across it. Personally I see this as a fairly big negative. I understand that implementations that exist (WebGL) might be slow on some platforms for RGB, but I feel this step was taken without enough consideration.

Here's an example: I have a normal texture, it's RGB, naturally. I now have to invest 33% more space to load it into GPU memory. Let's say that's okay, how about if I use a basis texture? A compressed format that is - those things will work as RGB. So, we're creating a situation where using a complex code path, such as compressed textures - I can have my RGB, but in a normal flow I can't. That just doesn't make sense to me.

Warning the user that RGB is not optimal I think is fine, plaster it all over docs, add a comment in constants.js stating as much. But why remove the functionality?

@Mugen87 stated:

E.g. all devices using Metal support no 3 channel formats.

I can't help but ask - so what? What about all the devices running GL and Vulkan? What of DirectX? Did three.js become a library that targets primarily Apple devices without me being aware of it? That would be okay too, I just fail to find sufficient reasoning for such a drastic (by my estimation) move.

[Mugen87]

Sorry, but I think everything was already said regarding this topic and I see no reasons for reviving this discussion.

[mrdoob]

Warning the user that RGB is not optimal I think is fine, plaster it all over docs, add a comment in constants.js stating as much. But why remove the functionality?

Because it's a performance footgun and people shouldn't have spend brain cycles on it.

[Usnul]

@mrdoob I understand the logic, and I do see that it would be difficult to warn users in a way that would both discourage newbies as well as inform more advanced users.

@Mugen87

Sorry, but I think everything was already said regarding this topic and I see no reasons for reviving this discussion.

I appreciate that, I did follow the links around the issue, this is where I came ultimately, if there is more public discussion/argumentation on the topic - please send me a link or post it here. I'm not here for a flame war, I just see this as a decision taken too lightly, I accept that there might be a lot more information that I simply did not see yet.

Note for those who, like me, might be unfamiliar with Metal API, here's a part from the spec that talks about formats:

[ringoz]

I am unable to use RGB9E5 format for my RGBE texture because of this change.
How do I specify gl.RGB format now?

[ringoz]

R11F_G11F_B10F is also a nice packed format which requires GL_RGB.
These formats are effectively banned from three.js now.
This PR is a performance footgun for me and the only reason to use local patching.

[ringoz]

The description of this PR is misleading.
Firstly, slow are 24 bits, not 3 channels.
Secondly, WebGPU surely does support 3 channel formats, from the spec:
"rgb9e5ufloat",
"rg11b10ufloat",

[ringoz]

what should've been done is renaming of RGBFormat to RGBPackedFormat
if you want people to not have spend brain cycles.

[Mugen87]

The PR focused on the common RGB format usage. It is true that there are specialized packed RGB formats but on the other hand they are used in production not very often. TBH, I have never seen them in the last seven years in the WebGL space.

Instead of introducing a new THREE constant, would you be happy if you could do this:

texture.format = 'RGB';
texture.internalFormat = 'RGB9_E5';

Making texture.format = 'RGB'; work requires a single additional line in WebGLUtils.convert(). You can try this out yourself by adding return gl[ p ] at the end of convert().

[ringoz]

Yeah, this is a great idea.

[donmccurdy]

@Mugen87 If we wanted to add R11F_G11F_B10F support to KTX2Loader for more compact IBL data, would you recommend using the .format = 'RGB' workaround above, with .internalFormat = 'R11F_G11F_B10F'? Or something else?

[Mugen87]

If we wanted to add R11F_G11F_B10F support to KTX2Loader for more compact IBL data, would you recommend using the .format = 'RGB' workaround above, with .internalFormat = 'R11F_G11F_B10F'?

Yes.