FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Set material npot to true for quadPanelEl for better quality when layer disabled by vincentfretin · Pull Request #5706 · aframevr/aframe · GitHub

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

Merged
dmarcos merged 1 commit into
aframevr: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 into
aframevr:masterfrom
vincentfretin:npotquad

Conversation

Copy link
Copy Markdown
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

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

with layer enabled

mrxz commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
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 commented Apr 25, 2025

Copy link
Copy Markdown
Member

Should we set minFilter instead?

Copy link
Copy Markdown
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 commented Apr 25, 2025

Copy link
Copy Markdown
Contributor

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 commented Apr 25, 2025

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
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 commented Apr 28, 2025

Copy link
Copy Markdown
Member

Thanks

dmarcos merged commit e947d42 into aframevr:master Apr 28, 2025
vincentfretin deleted the npotquad branch April 29, 2025 07:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL