Skip to content
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.

Sign up for GitHub

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

CVAT-3D - Part1 #3738

Closed
wants to merge 3 commits into from
Closed

CVAT-3D - Part1 #3738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5377a7e
CVAT-3D Includes frame manipulation activation by mouse in select mod…
Sep 28, 2021
659cba2
CVAT3D-Part1
Sep 28, 2021
8f31b81
Merge branch 'develop' of https://github.com/openvinotoolkit/cvat int…
Sep 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion cvat-canvas3d/src/typescript/canvas3dModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,14 @@ export interface ShapeProperties {
outlineColor: string;
selectedOpacity: number;
colorBy: string;
resetZoom: boolean;
canvasBackgroundColor: string;
Comment on lines +70 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand how opacity, outlined, colorBy relates to shapes. But I don't really understand why resetZoom and canvasBackgroundColor are shape properties.

}

export enum UpdateReasons {
IMAGE_CHANGED = 'image_changed',
OBJECTS_UPDATED = 'objects_updated',
UPDATE_CANVAS_BACKGROUND = 'update_canvas_background',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see any notifications with UPDATE_CANVAS_BACKGROUND. How is it used?

DRAW = 'draw',
SELECT = 'select',
CANCEL = 'cancel',
Expand All @@ -80,6 +83,7 @@ export enum UpdateReasons {
SHAPE_ACTIVATED = 'shape_activated',
GROUP = 'group',
FITTED_CANVAS = 'fitted_canvas',
UPDATE_OPACITY = 'update_opacity',
}

export enum Mode {
Expand Down Expand Up @@ -172,6 +176,8 @@ export class Canvas3dModelImpl extends MasterImpl implements Canvas3dModel {
outlineColor: '#000000',
selectedOpacity: 60,
colorBy: 'Label',
resetZoom: true,
canvasBackgroundColor: '#000000',
},
};
}
Expand Down Expand Up @@ -324,13 +330,18 @@ export class Canvas3dModelImpl extends MasterImpl implements Canvas3dModel {
}

public configureShapes(shapeProperties: ShapeProperties): void {
const { opacity, selectedOpacity } = this.data.shapeProperties;
this.data.drawData.enabled = false;
this.data.mode = Mode.IDLE;
this.cancel();
this.data.shapeProperties = {
...shapeProperties,
};
this.notify(UpdateReasons.OBJECTS_UPDATED);
if (opacity !== shapeProperties.opacity || selectedOpacity !== shapeProperties.selectedOpacity) {
this.notify(UpdateReasons.UPDATE_OPACITY);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need separated notify for opacity?
What if in one canvas request we push opacity, selectedOpacity for example with outlineColor?

} else {
this.notify(UpdateReasons.OBJECTS_UPDATED);
}
}

public fit(): void {
Expand Down
89 changes: 56 additions & 33 deletions cvat-canvas3d/src/typescript/canvas3dView.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,16 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
const canvasSideView = this.views.side.renderer.domElement;
const canvasFrontView = this.views.front.renderer.domElement;

canvasPerspectiveView.addEventListener('mouseleave', (event: MouseEvent): void => {
if (this.mode === Mode.DRAW) {
event.preventDefault();
this.cancelDraw();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need stop drawing when mouse gets out of the canvas?

this.setHelperVisibility(false);
this.mode = Mode.IDLE;
this.dispatchEvent(new CustomEvent('canvas.canceled'));
}
});

canvasPerspectiveView.addEventListener('contextmenu', (e: MouseEvent): void => {
if (this.controller.focused.clientID !== null) {
this.dispatchEvent(
Expand Down Expand Up @@ -448,12 +458,6 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
viewType.controls.mouseButtons.right = CameraControls.ACTION.NONE;
} else {
viewType.controls = new CameraControls(viewType.camera, viewType.renderer.domElement);
viewType.controls.mouseButtons.left = CameraControls.ACTION.NONE;
viewType.controls.mouseButtons.right = CameraControls.ACTION.NONE;
viewType.controls.mouseButtons.wheel = CameraControls.ACTION.NONE;
viewType.controls.touches.one = CameraControls.ACTION.NONE;
viewType.controls.touches.two = CameraControls.ACTION.NONE;
viewType.controls.touches.three = CameraControls.ACTION.NONE;
}
viewType.controls.minDistance = CONST.MIN_DISTANCE;
viewType.controls.maxDistance = CONST.MAX_DISTANCE;
Expand All @@ -479,7 +483,7 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
{ passive: false },
);
});

this.dispatchEvent(new CustomEvent('canvas.settings'));
model.subscribe(this);
}

Expand Down Expand Up @@ -750,12 +754,27 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
return cuboid;
}

private setupObjects(): void {
private setupObjects(selected?: boolean): void {
const { clientID } = this.model.data.activeElement;
if (this.views.perspective.scene.children[0]) {
this.clearSceneObjects();
if (!selected) {
this.clearSceneObjects();
}
this.resetColor();
this.setHelperVisibility(false);
for (let i = 0; i < this.model.data.objects.length; i++) {
const object = this.model.data.objects[i];
if (selected) {
const target = this.views.perspective.scene.getObjectByName(clientID);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clientID is not changed in the loop, why do we get target identifier in the loop?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If target is const, than line:

```this.views.perspective.scene.children[0].remove(target);`` tries to remove the same element a lot of times.
Please, correct me if I am wrong

if (`${object.clientID}` !== clientID) {
continue;
} else {
this.views.perspective.scene.children[0].remove(target);
this.views.side.scene.children[0].children = [];
this.views.front.scene.children[0].children = [];
this.views.top.scene.children[0].children = [];
Comment on lines +773 to +775
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to clear side/front/top scenes a lot of times if only one object can be there at the same time?

}
}
this.setupObject(object, true);
}
this.action.loading = false;
Expand All @@ -773,6 +792,14 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
this.views.perspective.renderer.domElement.dispatchEvent(event);
}

private cancelDraw(): void {
this.controller.drawData.enabled = false;
this.controller.drawData.redraw = undefined;
Object.keys(this.views).forEach((view: string): void => {
this.views[view as keyof Views].scene.children[0].remove(this.cube[view as keyof Views]);
});
}

public notify(model: Canvas3dModel & Master, reason: UpdateReasons): void {
if (reason === UpdateReasons.IMAGE_CHANGED) {
if (!model.data.image) return;
Expand All @@ -789,9 +816,21 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
loader.load(objectURL, this.addScene.bind(this));
URL.revokeObjectURL(objectURL);
this.dispatchEvent(new CustomEvent('canvas.setup'));
} else if (reason === UpdateReasons.UPDATE_CANVAS_BACKGROUND) {
const { canvasBackgroundColor } = this.model.data.shapeProperties;
this.views.top.scene.background = new THREE.Color(canvasBackgroundColor);
this.views.perspective.scene.background = new THREE.Color(canvasBackgroundColor);
this.views.side.scene.background = new THREE.Color(canvasBackgroundColor);
this.views.front.scene.background = new THREE.Color(canvasBackgroundColor);
Comment on lines +820 to +824
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try to test this code?

} else if (reason === UpdateReasons.UPDATE_OPACITY) {
this.resetColor();
} else if (reason === UpdateReasons.SHAPE_ACTIVATED) {
const { clientID } = this.model.data.activeElement;
this.setupObjects();
if (clientID !== 'null') {
this.setupObjects(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to pass the argument here (based on this.model.data.activeElement)? In this.setupObjects we also get access to this.model.data.activeElement (right in the first line)

} else {
this.setupObjects();
}
if (clientID !== 'null') {
this.setDefaultZoom();
}
Expand Down Expand Up @@ -820,34 +859,14 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
}),
);
this.model.data.activeElement.clientID = 'null';
if (this.model.mode === Mode.DRAG_CANVAS) {
const { controls } = this.views.perspective;
controls.mouseButtons.left = CameraControls.ACTION.ROTATE;
controls.mouseButtons.right = CameraControls.ACTION.TRUCK;
controls.mouseButtons.wheel = CameraControls.ACTION.DOLLY;
controls.touches.one = CameraControls.ACTION.TOUCH_ROTATE;
controls.touches.two = CameraControls.ACTION.TOUCH_DOLLY_TRUCK;
controls.touches.three = CameraControls.ACTION.TOUCH_TRUCK;
}
this.setupObjects();
} else if (reason === UpdateReasons.CANCEL) {
if (this.mode === Mode.DRAW) {
this.controller.drawData.enabled = false;
this.controller.drawData.redraw = undefined;
Object.keys(this.views).forEach((view: string): void => {
this.views[view as keyof Views].scene.children[0].remove(this.cube[view as keyof Views]);
});
this.cancelDraw();
}
this.model.data.groupData.grouped = [];
this.setHelperVisibility(false);
this.model.mode = Mode.IDLE;
const { controls } = this.views.perspective;
controls.mouseButtons.left = CameraControls.ACTION.NONE;
controls.mouseButtons.right = CameraControls.ACTION.NONE;
controls.mouseButtons.wheel = CameraControls.ACTION.NONE;
controls.touches.one = CameraControls.ACTION.NONE;
controls.touches.two = CameraControls.ACTION.NONE;
controls.touches.three = CameraControls.ACTION.NONE;
this.dispatchEvent(new CustomEvent('canvas.canceled'));
} else if (reason === UpdateReasons.FITTED_CANVAS) {
this.dispatchEvent(new CustomEvent('canvas.fit'));
Expand Down Expand Up @@ -1154,7 +1173,6 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
return;
}
if (!this.action.selectable) return;
this.resetColor();
const object = this.views.perspective.scene.getObjectByName(clientID);
if (object === undefined) return;
this.model.data.focusData.clientID = clientID;
Expand All @@ -1168,18 +1186,23 @@ export class Canvas3dViewImpl implements Canvas3dView, Listener {
}),
);
} else if (this.model.data.focusData.clientID !== null) {
this.resetColor();
this.model.data.focusData.clientID = null;
}
}
};

private resetColor(): void {
const { opacity, selectedOpacity } = this.model.data.shapeProperties;
this.model.data.objects.forEach((object: any): void => {
const { clientID } = object;
const target = this.views.perspective.scene.getObjectByName(String(clientID));
if (target) {
((target as THREE.Mesh).material as THREE.MeshBasicMaterial).color.set((target as any).originalColor);
if (this.model.data.activeElement.clientID === String(clientID)) {
((target as THREE.Mesh).material as THREE.MeshBasicMaterial).opacity = selectedOpacity / 100;
} else {
((target as THREE.Mesh).material as THREE.MeshBasicMaterial).opacity = opacity / 100;
}
}
});
}
Expand Down
8 changes: 7 additions & 1 deletion cvat-ui/src/components/annotation-page/canvas/canvas-wrapper3D.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ interface Props {
automaticBordering: boolean;
showObjectsTextAlways: boolean;
frame: number;
resetZoom : boolean;
canvasBackgroundColor: string;
}

interface ViewSize {
Expand Down Expand Up @@ -188,6 +190,8 @@ const CanvasWrapperComponent = (props: Props): ReactElement => {
onShapeDrawn,
onCreateAnnotations,
frameFetching,
resetZoom,
canvasBackgroundColor,
} = props;
const { canvasInstance } = props as { canvasInstance: Canvas3d };

Expand Down Expand Up @@ -374,7 +378,7 @@ const CanvasWrapperComponent = (props: Props): ReactElement => {

useEffect(() => {
updateShapesView();
}, [opacity, outlined, outlineColor, selectedOpacity, colorBy]);
}, [opacity, outlined, outlineColor, selectedOpacity, colorBy, resetZoom, canvasBackgroundColor]);

useEffect(() => {
const canvasInstanceDOM = canvasInstance.html() as ViewsDOM;
Expand All @@ -386,6 +390,7 @@ const CanvasWrapperComponent = (props: Props): ReactElement => {
canvasInstanceDOM.perspective.addEventListener('click', onCanvasClick);
canvasInstanceDOM.perspective.addEventListener('canvas.fit', onResize);
canvasInstanceDOM.perspective.addEventListener('canvas.groupped', onCanvasObjectsGroupped);
canvasInstanceDOM.perspective.addEventListener('canvas.settings', onCanvasShapeDrawn);
window.addEventListener('resize', onResize);

return () => {
Expand All @@ -396,6 +401,7 @@ const CanvasWrapperComponent = (props: Props): ReactElement => {
canvasInstanceDOM.perspective.removeEventListener('click', onCanvasClick);
canvasInstanceDOM.perspective.removeEventListener('canvas.fit', onResize);
canvasInstanceDOM.perspective.removeEventListener('canvas.groupped', onCanvasObjectsGroupped);
canvasInstanceDOM.perspective.removeEventListener('canvas.settings', onCanvasShapeDrawn);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to call onCanvasShapeDrawn (which is literally should be called after an object is drawn on 3D canvas) after constructor() of Canvas3DView? Could you explain logic?

window.removeEventListener('resize', onResize);
};
}, [frameData, annotations, activeLabelID, contextMenuVisibility]);
Expand Down