phaserjs / phaser-ce

Phaser CE is a fun, free and fast 2D game framework for making HTML5 games for desktop and mobile web browsers, supporting Canvas and WebGL rendering.
http://phaser.io
MIT License
1.34k stars 492 forks source link

Pixi.WebGLSpriteBatch: bind to textureIndex only if it's defined #708

Closed noocsharp closed 2 years ago

noocsharp commented 2 years ago

This PR

textureIndex is only defined if MultiTexture is enabled, so WebGL gives some warnings when it isn't, as shader.aTextureIndex is -1.

photonstorm commented 2 years ago

@samme I would recommend reverting this change. It will break the WebGL Sprite Batch because the vertex attribute is part of the buffer data and vertex size, regardless if multi-texturing is enabled or not.

jdnichollsc commented 2 years ago

@photonstorm ohh interesting, thanks for letting us know! what do you think is the correct way to fix this issue? https://github.com/photonstorm/phaser-ce/issues/709

photonstorm commented 2 years ago

@photonstorm ohh interesting, thanks for letting us know! what do you think is the correct way to fix this issue? #709

Reverting this PR will fix that issue. I'm not sure what this PR was meant to fix, as the attributes are needed even if multi-texturing is disabled.

noocsharp commented 2 years ago

A few warnings were popping up about textureIndex being out of bounds. (I believe it is only defined in the shaders if multi-texturing is enabled). But given the issues my attempted fix is causing, it's clearly not the right approach.

photonstorm commented 2 years ago

textureIndex is defined in both shaders within SpriteBatch (the multi one and the non-multi one) - you can see it on lines 181 and 199. Also, it's a float, so I'm not sure how it could go out of bounds, which is curious. Perhaps it wasn't the SpriteBatch shader, but another one?

noocsharp commented 2 years ago

It wasn't the value that went out of bounds, but the WebGL uniform index:

WebGL warning: vertexAttribI?Pointer: `index` (4294967295) must be < MAX_VERTEX_ATTRIBS.

You're right, it's not the SpriteBatch shader, it's in PixiFastShader.js. It appears vTextureIndex goes unused in the fragment shader when PIXI._enableMultiTextureToggle is disabled. The line that actually causes the warning is

    this.aTextureIndex = gl.getAttribLocation(program, 'aTextureIndex');

because this is where aTextureIndex becomes -1 for whatever reason. My guess is it gets optimized out because it goes unused, but I'm not very knowledgeable about shaders, so I could easily be wrong.