[WIP] feat: add expand/compress buttons #29
Conversation
zhixiangteoh
added
type.Enhancement
sovoid
changed the title
Add resize handle to toolbar
The idea is to attach the handle to the controller and then calculate the euclidean distance between the original handle position and the position of the handle (attached to the controller); some way to determine the sign of resizing (increase/decrease) has yet to be found. Some additional changes to driver `multiple-layers.js` particularly in that `handleToolbarIntersections` now handles engaging the move and resize interactions. `handleSelectEnd` is still where move and resize are disengaged.
|
you might need to rebase this branch with master |
Two points `handleLeftPoint` and `handleRightPoint` are created every time the resize handle is engaged. Then these points are used to determine whether to expand or compress. This way of doing it seems to be pretty buggy—3D euclidean distance between `currPosition` (position that the controller is pointing at) and the left and right points are not always reflected properly; you can be pointing to the right but the distance to the right point is somehow longer than the distance to the left. Open to new ways to determine the sign of resizing. '#' will be ignored, and an empty message aborts the commit. # # On branch feat-resizing # Your branch is up to date with 'origin/feat-resizing'. # # Changes to be committed: # modified: apps/simple-video-layers/multiple-layers.js # modified: util/webxr/MediaLayerManager/GlassLayer.js # modified: util/webxr/Toolbar.js #
| if ( | ||
| layerObj.glassLayer && | ||
| controller.userData.engagedResize && | ||
| layerObj |
There was a problem hiding this comment.
add this check before layerObj.glassLayer
| layerObj | ||
| ) { | ||
| layerObj.toolbar.disengageResize(controller); | ||
| controller.userData.engagedResize = false; |
There was a problem hiding this comment.
since we are passing controller here, let us modify this env variable inside the disengageResize
There was a problem hiding this comment.
@cabanier do you have any suggestions on this?
There was a problem hiding this comment.
Yes, it seems that this logic should move to disengageResize.
Maybe have an engaged attribute on controller.userdata?
There was a problem hiding this comment.
Hmm my idea was that since in handleSelectEnd we access controller.userData.engagedResize directly, as in:
if (layerObj.glassLayer && controller.userData.engagedResize && layerObj) {
// ^^^^^^^^^^^^^^^^^^^^^^^
layerObj.toolbar.disengageResize(controller);
controller.userData.engagedResize = false;
}we should explicitly modify this field in the driver itself.
| } | ||
| if (controller.userData.engagedResize && layerObj) { | ||
| layerObj.toolbar.disengageResize(controller); | ||
| controller.userData.engagedResize = false; |
There was a problem hiding this comment.
Looks like this condition is duplicated
| fluidResize() { | ||
| if ( | ||
| !this.resizeHandleClone || | ||
| !this.resizeHandleClone.userData.engageResizePosition |
There was a problem hiding this comment.
this condition is a bit weird. We can just combine them into one condition using optional chaining
| // console.log("dist to left", distanceLtoCurr); | ||
| // console.log("dist to right", distanceRtoCurr); | ||
| // console.log(distanceRtoCurr < distanceLtoCurr ? "expand" : "compress"); | ||
| const sign = distanceRtoCurr < distanceLtoCurr ? 1 : -1; |
There was a problem hiding this comment.
Lets use better variable names here since it is not explicit what these variables mean just by the name? Can you explain what they are doing here in a comment?
| this.handleRightPoint.getWorldPosition(handleRightPosition); | ||
|
|
||
| // console.log("handle left", handleLeftPosition); | ||
| // console.log("handle right", handleRightPosition); |
Description
This PR modifies the toolbar to expand and compress quad video layers by a fixed amount (factor of 1.25).
Note: Although the C/E buttons have no effect on equirect background layer (because equirect layer makes no sense of
widthorheightattributes), the toolbar for equirect layer video is the same one as for quad layer videos, which is not an ideal UI. Functionality-wise it does not affect any of the existing features nor introduce any new bugs.