-
-
Notifications
You must be signed in to change notification settings - Fork 35.6k
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
Geometry attributes compression example #18208
Conversation
…/Minimap.js" and commit the added files only
Can you please explain a little bit the approach of your PR? What do you understand by geometry attributes compression? |
Oh, sorry for not providing enough description! In this PR, the geometry attributes (position, normal and uv) are compressed by some of the compression algorithms provided in the GeometryCompressionUtils.js (whose name is temporary and may be changed according to you).
So after I did the implementation of these compression methods, I think this can be helpful for others who also needs geometry attributes compresion (maybe solving a very complex and large scene). If you want more explanation or codes improvement, I'll be glad to provide! |
I bet @zeux will like to see this 😁 |
I would consider using 16-bit components for octahedron normal encoding because the vertex attribute strides that aren't 4 byte aligned can have a performance penalty on some WebGL implementations. That gives 3x savings (12->4 bytes) at no perceptible quality loss. 8-byte octahedral encoding can have visible quantization errors in reflections on highly tessellated geometry, although of course if you are crafting a compressed vertex manually you can put the 2 bytes in an interleaved vertex somewhere to avoid the 4-byte alignment issue. Alternatively you may want to pack normal & tangent together in a 4 byte per vertex stream with 8-bit octahedron encoding and two components for each. Similarly, using 3 uint16 components can sometimes lead to performance cliffs on some WebGL implementations. So the performance-optimized layout is unfortunately slightly more memory hungry than memory-optimized. gltfpack pads everything to 4 bytes, although this also due to glTF alignment requirements (that exist for similar reasons). |
Learned a lot from @zeux 's comment! So can you please provide some more information on what webgl implementations will be potentially affected/harmed from vertex data compression? As I am currently dealing with large scenes and geometry compressions right now, thank you and appreciated! Also, perhaps I should add a "bytes" option which accept "2" or "4" (stands for uint8 or uint16) for octahedron normal encoding? Or any suggestion on just using uint16 instead of uint8? |
I haven't done a specific survey on various implementations, but a couple more concrete examples (note that again, the issue isn't with compressed data per se, it's with vertex attribute data that's not aligned to a 4 byte boundary):
It might make sense to extend the sample to provide both uint8 and uint16 normal storage - uint16 storage wouldn't suffer from alignment issues and, perhaps as importantly, could demonstrate the possible difference in quality between 8-bit and 16-bit octahedron encoding. |
…oding reference info.
added some info about compression functions
Thanks! |
I renamed the example to webgl_buffergeometry_compression. |
mesh.material.map = data.texture ? texture : null; | ||
|
||
var normalEncode = ""; | ||
if (data.oct1bytesEncode) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LeonYuanYao This should probably be changed to a dropdown in the example GUI since they're all mutually exclusive options. It's a bit confusing at the moment because all of the normal packing options can be checked but it will only use the last one.
} | ||
|
||
|
||
function computeGPUMemory(mesh) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's already an estimateBytesUsed
function in BufferGeometryUtils if there's interest in using that.
|
||
export namespace GeometryCompressionUtils { | ||
|
||
export function compressNormals( mesh: THREE.Mesh, encodeMethod: String ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions should have : void
return values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you 👍 , will improve the codes in the next PR.
Sorry for the late review but I saw a few things that felt comment worthy! I do like this addition, though -- thanks! I've also had success in the past just compressing UVs and normals to UInt8 or UInt16 values (assuming the UVs are [0.0, 1.0]) which is a pretty nice savings and doesn't require a new shader. A few comments on the functions:
|
Very nice and useful comments! Thanks. Since this PR is merged already, I would consider making these improvements into a new PR in one or two weeks (it is chinese new year vacation now). And for the question of Raycasting, yes I think the traditional CPU-based raycasting may not work properly. It should be noticed when using position attribute compression. However it can be solved by using GPU-based raycasting, I don't know if it is supported by three.js? This could be another new feature to be considered. |
Happy New Year! 🐀 |
|
||
if (currentCos > bestCos) { | ||
best = oct; | ||
bestCos = currentCos; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the value of bestCos
is never read, see https://lgtm.com/projects/g/mrdoob/three.js/snapshot/8ff9714090676d3f766f88d95e8c841b4250d541/files/examples/jsm/utils/GeometryCompressionUtils.js#x3ada8d6e7c54300f:1. It seems octEncodeBest()
can be refactored/simplified to avoid this lgtm issue.
var tempx = x; | ||
var tempy = y; | ||
tempx = (1 - Math.abs(y)) * (x >= 0 ? 1 : -1); | ||
tempy = (1 - Math.abs(x)) * (y >= 0 ? 1 : -1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another lgtm issue: tempx
and tempy
are always assigned values here so you can write the code like so:
var tempx = ( 1 - Math.abs( y ) ) * ( x >= 0 ? 1 : - 1 );
var tempy = ( 1 - Math.abs( x ) ) * ( y >= 0 ? 1 : - 1 );
and delete var tempx = x;
and var tempy = y;
.
Created an example on geometry attributes compression util with a customed material 'PackedPhongMaterial', which can display how much gpu memory can be reduced by applying these compression algrithms. Any suggestions are welcomed and appreciated!