Gsplat better tile selection #12927
Gsplat better tile selection #12927
Conversation
…lected the previous frame Allows LOD to load correctly
|
Thank you for the pull request, @keyboardspecialist! ✅ We can confirm we have a CLA on file for you. |
| const selectionChanged = () => { | ||
| const currentSelection = tileset._selectedTiles; | ||
| if (this._previousSelectionLength !== currentSelection.length) { | ||
| this._previousSelectionLength = currentSelection.length; | ||
| return true; | ||
| } | ||
| for (let i = 0; i < currentSelection.length; i++) { | ||
| if (!currentSelection[i]._wasSelectedLastFrame) { | ||
| this._previousSelectionLength = currentSelection.length; | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| this._previousSelectionLength = currentSelection.length; | ||
| return false; | ||
| }; |
There was a problem hiding this comment.
nit - the update function is pretty long as-is, why not just extract this into a proper function instead of a lambda?
There was a problem hiding this comment.
Other comments:
- Every branch of this function sets
this._previousSelectionLength = currentSelection.length;, so rather than doing it 3x we do it just once, at the top. - "selection" refers to a 3D tile, right? This function seems like it belongs to the tileset class, not to this class. The logic itself isn't specific to splats. It also seems like, if it were done in the tileset class, there may be a more performant way to do it that doesn't involve iterating over the whole surrent selection.
(Note that 2 is mutually exclusive with my other comment above; we can only do one or the other)
There was a problem hiding this comment.
The last this._previousSelectionLength = currentSelection.length; is unnecessary anyhow, because when they are not (already) equal, the first if has already kicked in.
All this should refer to selected tiles that have splat content - but this is not handled, see other issues.
There was a problem hiding this comment.
Good point- actually, by the same logic, the second this._previousSelectionLength = currentSelection.length; is also unnecessary.
3 participants
Description
Improves tile selection change detection by checking if tiles were selected previous frame. Allows for LoD to load and render correctly.
Issue number and link
Testing plan
Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my change