-
Notifications
You must be signed in to change notification settings - Fork 303
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
feat(PointCloud): increase range of picking to u32 #2288
base: master
Are you sure you want to change the base?
Conversation
66c2e5d
to
49d41be
Compare
I don't know where the functional test errors are coming from, I can't replicate them locally so I'm guessing (hoping) it's a toolchain problem and not on my end |
49d41be
to
4c5e66c
Compare
4c5e66c
to
58a43d0
Compare
const data = buffer.slice(idx, idx + 3); | ||
|
||
// see PotreeProvider and the construction of unique_id | ||
const objId = (data[0] << 8) | data[1]; | ||
const index = (data[2] << 8) | data[3]; | ||
const objId = data[0]; | ||
const index = (data[1] << 16) | data[2]; |
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.
You encode each 48-bits unique_id
value as 3 uint16
. However, you read the resulting texture with a uint8
buffer (see c3dEngine#renderViewToBuffer
)...
I don't think values are preserved here.
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.
We can provide a u16 buffer ourselves
- const buffer = view.mainLoop.gfxEngine.renderViewToBuffer(
- { camera: view.camera, scene: tileLayer.object3d },
- {
- x: viewCoords.x - radius,
- y: viewCoords.y - radius,
- width: 1 + radius * 2,
- height: 1 + radius * 2,
- });
+ const zone = {
+ x: viewCoords.x - radius,
+ y: viewCoords.y - radius,
+ width: 1 + radius * 2,
+ height: 1 + radius * 2,
+ };
+ zone.buffer = new Uint16Array(4 * zone.width * zone.height);
+ const buffer = view.mainLoop.gfxEngine.renderViewToBuffer(
+ { camera: view.camera, scene: tileLayer.object3d },
+ zone,
+ );
But looking into things a bit more I'm not sure how this is gonna interact with the default fullSizeRenderTarget
used by the c3DEngine
that gets created with u8 rgba formatting.
I currently don't have an example I could use to check for this issue, as the results don't change for the example I originally used for this PR (potree 3d map).
Description
Change the picking IDs from u16 to u32.
Motivation and Context
Effectively eliminates the possibility of not having enough IDs to perform picking until GPUs start having 32GB+ of VRAM.
Tested on Firefox 123.0 (64-bit) on Ubuntu Linux 22.04.4 LTS x86_64