Skip to content
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

Materials: Consider to use mediump for all shaders #14570

Closed
4 of 13 tasks
Mugen87 opened this issue Jul 27, 2018 · 19 comments
Closed
4 of 13 tasks

Materials: Consider to use mediump for all shaders #14570

Mugen87 opened this issue Jul 27, 2018 · 19 comments

Comments

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 27, 2018

Description of the problem

Right now, the default precision for shader progams in three.js is highp. Issues like #14137 showed that this approach causes rendering problems especially on (older) mobile devices . Using mediump would solve these issues and also improve the performance of the shader execution on mobile, see #14137 (comment).

The problem is that certain materials like MeshStandardMaterial can not produce correct visual output with mediump right now, see #14137 (comment). Changing certain parts of the shader souce code is necessary to solve this issue.

Three.js version
  • Dev
  • r94
  • ...
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
Hardware Requirements (graphics card, VR Device, ...)
@greggman
Copy link
Contributor

mediump won't solve all issues AFAIK. I'm pretty sure (but could be wrong) if you check lighting using mediump you'll find things don't render correctly? At least that was my experience with my own shaders. For Phong shading passing the vertex viewPosition to the fragment shader in mediump means there's actually not enough resolution to do the math and what you see is not remotely correct.

It's not checkable on desktop because even putting mediump on desktop uses highp.

Of course maybe it's better than black. One solution is to use highp if it's supported, something like

#if GL_FRAGMENT_PRECISION_HIGH
   precision highp float;
#else 
  precision mediump float;
#endif

or other contortions so at least you don't get black?

@mrdoob
Copy link
Owner

mrdoob commented Jul 28, 2018

One solution is to use highp if it's supported

In the issue that @Mugen87 referenced, the devices supported highp but it had glsl bugs when using highp that the conformance test didn't cover.

@mrdoob
Copy link
Owner

mrdoob commented Jul 28, 2018

/cc @WestLangley @bhouston

@greggman
Copy link
Contributor

If three defaults to mediump you'll basically get incorrect rendering on all phones because at the moment all phones (99%) will actually use mediump if you ask for it. At mediump the lighting will be wrong.

If it's just one bad phone then I'd suggest you ask the WebGL people to add a test and then blacklist that phone until they fix it (or get them to make that phone not report it can support highp).

@greggman
Copy link
Contributor

Example:

highp correct lighting

highp-correct-lighting

mediump incorrect lighting (same scene)

mediump-incorrect-lighting

@Supernuija
Copy link

I can confirm what @Mugen87 said about using mediump as default would be a great idea!
This is also an issue with Adreno 306 gpu phones and interestingly a newer of the same phone model, Samsung J5(2016) renders all materials black using highp and the older 2014 version with same Adreno 306 gpu renders them correctly. Also I did find that in an older Sony Xperia Z2 pad, highp rendered correctly with Chrome but not with Firefox.
This seems to be an endless swamp, if three.js defaults to highp instead of mediump. No one knows how many users have either drivers, gpu:s or browser versions that will not support correct material rendering. With this simple setting many problems would be solved instantly IMHO.

@Supernuija
Copy link

It seems that current three.js check for device graphic capabilities gives false positive value on some hardware, which leads to this material rendering black issue. New discovery was, that in that vasturiano/globe.gl#44 site mentioned above, some coloured objects had solid colour, but the textured night globe was pitch black. This might be due to some specific material definition that allow some colours to render still even when this bug prevents some other materials to render correctly.

I think that the only way to prevent this kind of bug to occur would be some kind of simple pre-check, where known colour object would be drawn on specific location (like the white cube) and then checked if the result on that pixel area is coloured black which would lead the default precision to be changed to mediump. How it is done codingwise, is up to someone who knows better.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 10, 2021

I don't think the engine should implement such checks. If a device says it supports highp, the engine should rely on this support. Otherwise the device/driver needs a fix.

The idea of this issue is to investigate whether it is possible to add mediump support to materials. If this works, apps can configure Material.precision in order to force mediump on certain devices. This is currently not possible because of the above rendering issues.

@Supernuija
Copy link

Knowing closely how eager all mobile device manufacturers are to implement this kind of fixes on any older mobile devices, I would still try to ensure that three.js displays all content properly despite the mobile device hw/sw developers.

If we would live in a perfect world, these kind of things would always be fixed. As long as the manufacturer's main business driver is to force users constantly buy new devices (designed obsolescense), any kind of fixes on older hardware drivers is practically none, especially in mobile devices. In order to guarantee functionality on as many devices as possible by default, no matter how badly others have done their homework, it would be strongly recommended that framework would probe in this case what is really possible and what's not.

@Supernuija
Copy link

I think the option to set mediump by default and update all the required shader code so the lighting doesn't break (#14570 (comment)) by @mrdoob was a good idea.

Unfortunately I am not experienced enough to do that, but I hope someone who knows the area better finds this thread and gives it a go. I am more than willing to do the tests when needed with actual mobile hw.

@Supernuija
Copy link

Just found out that Adreno 330 has this same render black issue using highp, but only with Firefox, Chrome renders correctly.

@LeviPesin
Copy link
Contributor

Maybe we can create a list of issues that appear when using mediump, like #25071 (maybe based on #22003) and then solve them one-by-one?

@mrdoob
Copy link
Owner

mrdoob commented Dec 6, 2022

I'm thinking this is something that is not that relevant/needed nowadays...

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 12, 2022

Yeah, I think the amount of devices which only supports mediump is not relevant anymore.

However, there is the valid use case that applications request mediump on mobile devices and things start to break. Maybe we can use the same strategy like for solving #25071: Identifying the code which is not compatible with mediump and explicitly define highp for the respective scope/variable.

@mrdoob
Copy link
Owner

mrdoob commented Dec 12, 2022

That sound good 👍

@LeviPesin
Copy link
Contributor

Related: #8353

@krei-se
Copy link

krei-se commented Nov 18, 2023

I have a device with this issue and would love a fix for this.

Adreno Chrome <-- hehe.

@mrdoob
Copy link
Owner

mrdoob commented Nov 28, 2023

I don't think we'll end up doing this...

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Nov 28, 2023

Agreed. I think at this point we can close the issue and hope that devices catch up.

@mrdoob mrdoob closed this as completed Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants