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

Shader chunk system can exceed 8 varyings #71

Closed
willeastcott opened this issue Nov 8, 2014 · 9 comments
Closed

Shader chunk system can exceed 8 varyings #71

willeastcott opened this issue Nov 8, 2014 · 9 comments
Labels
area: graphics Graphics related issue bug

Comments

@willeastcott
Copy link
Contributor

Check out this project:

https://playcanvas.com/will/physicsperformance

The shader on the ground plane's material creates a varying for each of the 4 maps (diffuse, normal, specular, opacity). This takes the number of varyings over 8 and the program fails to link on an iPhone, for example, that can support no more than 8 varyings.

@guycalledfrank
Copy link
Contributor

I see. Is there any elegant solution to this? When developing new shaders I used an unfinished "layer" system for a moment, where each layer could have only 1 UV set, and we could have all kinds of maps assigned to one layer. I also wanted to make an ability to add multiple layers with different blends over each other. But it would break old mapTransforms in old projects.
As a workaround, I decided to convert mapTransforms to layers by analyzing if there are equal transforms at the moment of shader creation and grouping them into 1 layer (and 1 varying).
But the downside of such idea was that once the shader is compiled, you can't dynamically change mapTransform of one texture without affecting all other textures that shared the original transform at generation time. Or you should recompile the shader when changing mapTransform a few times at runtime. In the end I just dropped the idea and returned all map transforms back.
That's it...

  • can we drop mapTransforms and replace it with something better performance-wise? (I guess no, projects will break)
  • should we allow users to create UV transforms in UI and assign them to maps? We may also allow users to use position as UV and other funky stuff. But maybe it would be too complex and unusual for users?
  • if we're just leaving mapTransforms as they are now, we then have to silently couple multiple equal transforms in one, as I tried. Should we compile multiple shaders instead of just one in case if user decides to dynamically change transforms after original shader generation? Should we recompile the shader each time any transform changed (not great)? Should we transforms maps in PS by uniform when no varyings available? This will make it a dependent read though...

@guycalledfrank
Copy link
Contributor

Actually, since we recompile & cache on any serious material change, the best option for now would be to check transform equality of different maps, and recompile if it's changed.

@willeastcott
Copy link
Contributor Author

Here's another suggestion: if you run out of varyings, disable normal mapping and print a warning to the JS console. At least you are then almost certain to have enough varyings since you can drop the binormal and tangent. And at least the program will link and the mesh will render (albeit not quite as prettily). I'm not saying I like this option, but it's better than what we have currently.

can we drop mapTransforms and replace it with something better performance-wise?

What did you have in mind?

But maybe it would be too complex and unusual for users?

It depends on the interface. I can't quite visualize what it would look like.

Actually, since we recompile & cache on any serious material change, the best option for now would be to check transform equality of different maps, and recompile if it's changed.

Yeah, this is probably the way to go for now.

By the way....I did just look at the GLSL generated for the floor plane in the project. It has 9 varyings:

  • vPositionW
  • vNormalW
  • vTangentW
  • vBinormalW
  • vUv0
  • vUvDiffuse
  • vUvNormal
  • vUvOpacity
  • vUvSpecular

It looks like vUv0 isn't needed. You could use a temporary instead. I guess at the moment, the linker isn't able to drop vUv0 because it's kind of being used.

@guycalledfrank
Copy link
Contributor

OK, I'll fix it. So the total list of stuff to add is:

  • use 1 varying where transforms have equal value;
  • remove vUv0/1 when these aren't used;
  • disable normal/parallax mapping when we exceed varyings and print a warning.

@willeastcott
Copy link
Contributor Author

I would say it would be best to remove uUv0/1 where possible first (as the top priority because it's easy to fix).

Ultimately, it's a question of the engine doing something sensible in every case when there are only 8 varyings possible. A failed program link must always be avoided.

@frankxw
Copy link

frankxw commented Apr 1, 2015

I just ran into this recently on an iPad 2 (pretty old at this point). Shader results in too many varyings. If you look at base.frag, you guys recently commented out vVertexColor. Could you also comment out the other varyings not being used? Specifically vUv1 and vNormalV. There's a comment at the top about the compiler removing unneeded stuff, but I don't think this always applies to varyings.

@willeastcott
Copy link
Contributor Author

@guycalledfrank I just wanted to completely understand the current status of this in the engine. Am I right in saying this is still technically an issue? What are the circumstances where the number of generated varyings can still exceed 8?

@Maksims
Copy link
Contributor

Maksims commented Feb 24, 2017

@guycalledfrank is this still an issue, or have been addressed? This ticket been stalled, and shall be at least updated or requires a closure.

@willeastcott
Copy link
Contributor Author

Closing due to age. I have not seen any shader compilation/linking issues relating to a lack of varying in the last few years.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue bug
Projects
None yet
Development

No branches or pull requests

4 participants