Skip to content

Set material npot to true for quadPanelEl for better quality when layer disabled#5706

Merged
dmarcos merged 1 commit intoaframevr:masterfrom
vincentfretin:npotquad
Apr 28, 2025
Merged

Set material npot to true for quadPanelEl for better quality when layer disabled#5706
dmarcos merged 1 commit intoaframevr:masterfrom
vincentfretin:npotquad

Conversation

@vincentfretin
Copy link
Contributor

Description:

Set material npot (not power of two) to true for quadPanelEl for better quality when layer is disabled.

Changes proposed:

  • Setting npot to true similar to what we have for a-videosphere and a-sky

Tested on the https://aframe.io/aframe/examples/test/layer/ example

layer disabled, without npot
npotfalse

layer disabled, with npot: true (not necessary visible on the jpg here, but trust me it changes the quality)
npottrue

with layer enabled
quadlayer

@mrxz
Copy link
Contributor

mrxz commented Apr 24, 2025

The npot property shouldn't meaningfully change the quality. In fact, I'd even consider removing it since WebGL 1 support has been dropped. So quite surprised to see it used this way.

I haven't tested it myself, so I do take your word that there's a perceivable difference. My guess is that it's caused by the minFilter being set to gl.LINEAR instead of gl.LINEAR_MIPMAP_LINEAR (material.js#L36). This introduces more aliasing, especially at larger distances or at off angles. In this viewing configuration that probably results in a (slightly) sharpened image which helps with the outlines.

Though I wouldn't recommend using it this way. At the very least it would be cleaner to directly set the minFilter instead of using npot: true for the same effect. A better approach would be to set the texture lod bias, but Three.js sadly doesn't exposes this property.

@vincentfretin
Copy link
Contributor Author

Yes that's exactly the lines

texture.wrapS = THREE.ClampToEdgeWrapping;
texture.wrapT = THREE.ClampToEdgeWrapping;
texture.magFilter = THREE.LinearFilter;
texture.minFilter = THREE.LinearFilter;

that improves the sharpness. I have those lines on a custom component that creates a sphere geometry and material for 360 image.

@dmarcos
Copy link
Member

dmarcos commented Apr 25, 2025

Should we set minFilter instead?

@vincentfretin
Copy link
Contributor Author

Except for repeat, offset, anisotropy, setting other texture properties is currently not exposed to the material component schema.
I don't know what that npot: true was doing in the past with WebGL1, today on WebGL2 it's just setting those 4 texture properties.
That may not be a good name now to mean just setting linear filter, but that's a property that is used in the wild, so I guess we'll keep it as backward compatibility.

@mrxz
Copy link
Contributor

mrxz commented Apr 25, 2025

WebGL 1 placed various restrictions on non-power-of-two (npot) textures. Including the wrapping and filtering modes that can be used with such textures. With WebGL 2 there are no restrictions for npot textures.

I'd say we should start exposing the filtering methods on the material component. Indirectly setting it through the npot property is unintuitive. Not to mention that it's unclear if npot is used for legacy reasons (WebGL 1 support), to enable clamping or to use linear filtering/avoid mipmaps.

@dmarcos
Copy link
Member

dmarcos commented Apr 25, 2025

I’m good with exposing the new properties and deprecating npot

@vincentfretin
Copy link
Contributor Author

Okay. Feel free to still merge this and we can work on that later. I don't plan to work on it right now.

@dmarcos
Copy link
Member

dmarcos commented Apr 28, 2025

Thanks

@dmarcos dmarcos merged commit e947d42 into aframevr:master Apr 28, 2025
3 checks passed
@vincentfretin vincentfretin deleted the npotquad branch April 29, 2025 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants